Bug 233349 – Web Inspector: Console execution context can become an unexpected selection on refresh/navigation

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Patrick Angle]

<rdar://68229876>

Steps to (flakily) reproduce:
1. Go to https://books.apple.com/us/audiobook/winning-unabridged/id1561431722
2. Inspect the page.
3. Switch to the console tab *(explained below)
4. Confirm the console works with 1+1 or other similarly simple input.
5. Refresh the page.
6. Notice that sometime the work Auto disappears from the execution context selector, and the entire control turns blue indicating a non-Auto selection.

* This is /much/ harder to reproduce on the Elements tab, for example because there is currently a very short 10ms timeout during which the current frame's execution context will be eligible for re-selection post-load. Increasing the timeout in QuickConsole.prototype._handleFrameExecutionContextsCleared to 100ms makes this much easier to reproduce. This makes for two somewhat unfortunate bugs here:
1. If the frame of the active execution context commits a provisional load, our code attempts to reselect the new context for that frame, which results in clearing the `Auto` bit for selected context, which means the context no longer will follow the selected DOM node.
2. Because of the very short timeframe for the above to take place before bailing (currently 10ms) there are many situations where other large payloads over the protocol, like a complex DOM tree, will cause the time to expire before we receive an event telling us there is a new execution context for the frame, which means we end up not following the frame anyways... which leads to...
3. Which can lead to us having a selected execution context that may no longer be valid since the failsafe in QuickConsole.prototype._handleFrameExecutionContextsCleared doesn't really fail safely, since it doesn't provide a new execution context to be active and instead seems to rely on the hope that the context will still work for future invocations, which I believe is how many users are getting into the state of a blank execution context picker (with a working dropdown menu with actual contexts that are selectable) and are unable to evaluate anything in the console.

[Patrick Angle]

Created attachment 444767 [details]
Patch v1.0

[Devin Rousso]

Comment on attachment 444767 [details]
Patch v1.0

View in context: https://bugs.webkit.org/attachment.cgi?id=444767&action=review

r=me, nice catch :)

> Source/WebInspectorUI/UserInterface/Views/QuickConsole.js:400
>                  this._restoreSelectedExecutionContextForFrame = null;

Style: I'd add a newline after this

> Source/WebInspectorUI/UserInterface/Views/QuickConsole.js:401
> +                this._useExecutionContextOfInspectedNode = InspectorBackend.hasDomain("DOM");

NIT: We should probably make a `_canUseExecutionContextOfInspectedNode()` or something since this is done so often.

[Patrick Angle]

Created attachment 445638 [details]
Patch v1.1 - Address review nits

[EWS]

Committed r286412 (244758@main): <https://commits.webkit.org/244758@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 445638 [details].