Skip to content

devtools: Do not cache environment actor values#44765

Merged
eerii merged 2 commits into
servo:mainfrom
atbrakhi:fix-stale-values
May 7, 2026
Merged

devtools: Do not cache environment actor values#44765
eerii merged 2 commits into
servo:mainfrom
atbrakhi:fix-stale-values

Conversation

@atbrakhi
Copy link
Copy Markdown
Member

@atbrakhi atbrakhi commented May 6, 2026

Currently Environment Objects are cached in createEnvironmentActor. That meant DevTools could keep showing an old value for a variable after execution had moved on to a different value.

In this change, we keep caching the environment actor name, but make sure the current state is being reflected by refreshing the environment actor data each time.

This keeps actor id stable and is also closer to Firefox behavior.

Testing: Current tests are passing
Fixes: part of #36027

@atbrakhi atbrakhi requested a review from eerii May 6, 2026 13:52
@atbrakhi
Copy link
Copy Markdown
Member Author

atbrakhi commented May 6, 2026

Depends on #44763

Now:
image

Before:
image
Note that to reproduce, you might need to step through the lines several times!

Currently Environment Objects are cached in `createEnvironmentActor`.
That meant DevTools could keep showing an old value for a variable
after execution had moved on to a different value.

In this change, we keep caching the environment actor name, but make
sure the current state is being reflected by refreshing the environment
actor data each time.

This keeps the actor id stable and is also closer to Firefox behavior.

Signed-off-by: atbrakhi <[email protected]>
@atbrakhi atbrakhi force-pushed the fix-stale-values branch from 24c5268 to 8c9aef0 Compare May 6, 2026 15:31
@atbrakhi atbrakhi changed the title devtools: Do not cache environment actors devtools: Do not cache environment actor values May 6, 2026
@atbrakhi
Copy link
Copy Markdown
Member Author

atbrakhi commented May 6, 2026

@eerii this should be ok to review now. We are caching actor names now and rebuild the values. we also update/register based on that.

@atbrakhi atbrakhi marked this pull request as ready for review May 6, 2026 15:32
@atbrakhi atbrakhi requested a review from gterzian as a code owner May 6, 2026 15:32
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label May 6, 2026
Comment thread components/devtools/actors/environment.rs
Co-authored-by: eerii <[email protected]>
Signed-off-by: atbrakhi <[email protected]>
Copy link
Copy Markdown
Member

@eerii eerii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! :)

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label May 7, 2026
@eerii eerii enabled auto-merge May 7, 2026 09:15
@eerii eerii added this pull request to the merge queue May 7, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label May 7, 2026
Merged via the queue into servo:main with commit eb9a5fb May 7, 2026
30 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants