1511214 - Support global evaluations in the console

Status: RESOLVED INCOMPLETE

(Core Graveyard :: Web Replay, enhancement, P5)

Description

[Brian Hackett [Laid off!]]

Attachment: patch

Right now, doing console evaluations when paused at a scripted location works fine; the evaluation is done in the context of the topmost frame.  When there is no frame, as happens when paused at a checkpoint or when still recording, a "Cannot evaluate while replaying without a frame" error message is produced instead.

The attached patch adds support for evaluating code when there is no frame on the stack.  We don't use the server's evalWindow as the environment for the eval, as this window is in the wrong process.  Instead, we use the current global in the child process --- defined here as the global which most recently had any scripts compiled for it --- as the evaluation's environment.

Comment 1

[Brian Hackett [Laid off!]]

Attachment: Part 1 - Support global evaluations in ReplayDebugger.

Add implementations for ReplayDebugger.Object.{executeInGlobal,asEnvironment}, and add a new replay-specific method, ReplayDebugger.replayCurrentGlobal(), to get the currently active global (or undefined) in the replaying process.

Comment 2

[Brian Hackett [Laid off!]]

Attachment: Part 2 - Support global evaluations in the web console while replaying.

Add support for global evaluations in the console, with some handling for the case where there is no global in the replaying process to evaluate against (we can run into this issue when e.g. paused at the beginning of the recording).

Comment 3

[Logan Smyth [:loganfsmyth]]

Comment on attachment 9028819 [details] [diff] [review]
Part 1 - Support global evaluations in ReplayDebugger.

Review of attachment 9028819 [details] [diff] [review]:
-----------------------------------------------------------------

::: devtools/server/actors/replay/replay.js
@@ +175,5 @@
>  
>    addScript(script);
>    addScriptSource(script.source);
>  
> +  gCurrentGlobal = script.global;

I'm not familiar enough with the debugger's handling of globals right now. Are there contexts where there will be multiple globals here that we need to worry about? If we have a same-origin iframe for instance, does that global also show up here?

Comment 4

[Brian Hackett [Laid off!]]

(In reply to Logan Smyth [:loganfsmyth] from comment #3)
> Comment on attachment 9028819 [details] [diff] [review]
> Part 1 - Support global evaluations in ReplayDebugger.
> 
> Review of attachment 9028819 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: devtools/server/actors/replay/replay.js
> @@ +175,5 @@
> >  
> >    addScript(script);
> >    addScriptSource(script.source);
> >  
> > +  gCurrentGlobal = script.global;
> 
> I'm not familiar enough with the debugger's handling of globals right now.
> Are there contexts where there will be multiple globals here that we need to
> worry about? If we have a same-origin iframe for instance, does that global
> also show up here?

Yes, that global would show up here.

Comment 5

[Logan Smyth [:loganfsmyth]]

(In reply to Brian Hackett (:bhackett) from comment #4)
> (In reply to Logan Smyth [:loganfsmyth] from comment #3)
> > Comment on attachment 9028819 [details] [diff] [review]
> > Part 1 - Support global evaluations in ReplayDebugger.
> > 
> > Review of attachment 9028819 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: devtools/server/actors/replay/replay.js
> > @@ +175,5 @@
> > >  
> > >    addScript(script);
> > >    addScriptSource(script.source);
> > >  
> > > +  gCurrentGlobal = script.global;
> > 
> > I'm not familiar enough with the debugger's handling of globals right now.
> > Are there contexts where there will be multiple globals here that we need to
> > worry about? If we have a same-origin iframe for instance, does that global
> > also show up here?
> 
> Yes, that global would show up here.

Is that something we should be concerned about? If the nested iframe loads a script after the main page, we'll end up with the wrong global for execution, I assume? If that's just a limitation for now that's probably alright, but it seems like ideally we'd have a plan to eventually handle it properly.

Comment 6

[Brian Hackett [Laid off!]]

(In reply to Logan Smyth [:loganfsmyth] from comment #5)
> (In reply to Brian Hackett (:bhackett) from comment #4)
> > (In reply to Logan Smyth [:loganfsmyth] from comment #3)
> > > Comment on attachment 9028819 [details] [diff] [review]
> > > Part 1 - Support global evaluations in ReplayDebugger.
> > > 
> > > Review of attachment 9028819 [details] [diff] [review]:
> > > -----------------------------------------------------------------
> > > 
> > > ::: devtools/server/actors/replay/replay.js
> > > @@ +175,5 @@
> > > >  
> > > >    addScript(script);
> > > >    addScriptSource(script.source);
> > > >  
> > > > +  gCurrentGlobal = script.global;
> > > 
> > > I'm not familiar enough with the debugger's handling of globals right now.
> > > Are there contexts where there will be multiple globals here that we need to
> > > worry about? If we have a same-origin iframe for instance, does that global
> > > also show up here?
> > 
> > Yes, that global would show up here.
> 
> Is that something we should be concerned about? If the nested iframe loads a
> script after the main page, we'll end up with the wrong global for
> execution, I assume? If that's just a limitation for now that's probably
> alright, but it seems like ideally we'd have a plan to eventually handle it
> properly.

Hmm, I guess I don't have a good understanding of what the semantics of the eval window are.  Looking at this more closely, it looks like the eval window defaults to the top level window, and the cd() function can be used to change that.  Since we don't support the cd() function yet (it requires adding new bindings to the evaluate, which isn't supported, and will require other changes as well to work), it looks like we should be using the top level window all the time for now.

Do you know about a good way to identify the top level window correctly in a content process?  I could use the first window which scripts were created for, but this seems like a hack and might not behave correctly in all situations.

Comment 7

[Logan Smyth [:loganfsmyth]]

(In reply to Brian Hackett (:bhackett) from comment #6)
> (In reply to Logan Smyth [:loganfsmyth] from comment #5)
> > (In reply to Brian Hackett (:bhackett) from comment #4)
> > > (In reply to Logan Smyth [:loganfsmyth] from comment #3)
> > > > Comment on attachment 9028819 [details] [diff] [review]
> > > > Part 1 - Support global evaluations in ReplayDebugger.
> > > > 
> > > > Review of attachment 9028819 [details] [diff] [review]:
> > > > -----------------------------------------------------------------
> > > > 
> > > > ::: devtools/server/actors/replay/replay.js
> > > > @@ +175,5 @@
> > > > >  
> > > > >    addScript(script);
> > > > >    addScriptSource(script.source);
> > > > >  
> > > > > +  gCurrentGlobal = script.global;
> > > > 
> > > > I'm not familiar enough with the debugger's handling of globals right now.
> > > > Are there contexts where there will be multiple globals here that we need to
> > > > worry about? If we have a same-origin iframe for instance, does that global
> > > > also show up here?
> > > 
> > > Yes, that global would show up here.
> > 
> > Is that something we should be concerned about? If the nested iframe loads a
> > script after the main page, we'll end up with the wrong global for
> > execution, I assume? If that's just a limitation for now that's probably
> > alright, but it seems like ideally we'd have a plan to eventually handle it
> > properly.
> 
> Hmm, I guess I don't have a good understanding of what the semantics of the
> eval window are.  Looking at this more closely, it looks like the eval
> window defaults to the top level window, and the cd() function can be used
> to change that.  Since we don't support the cd() function yet (it requires
> adding new bindings to the evaluate, which isn't supported, and will require
> other changes as well to work), it looks like we should be using the top
> level window all the time for now.
> 
> Do you know about a good way to identify the top level window correctly in a
> content process?  I could use the first window which scripts were created
> for, but this seems like a hack and might not behave correctly in all
> situations.

I think I'd have to look over the existing logic that the console has. It seems like they have a lot of logic to determine what window to use.

If we'd like to land this ASAP, I think it's fine in the short term, but it does seem like things will stop working the way you expect as soon as a page has an iframe, which I'd imagine is a good many pages for ads and things.