Title: [202566] trunk
Revision
202566
Author
[email protected]
Date
2016-06-28 08:12:10 -0700 (Tue, 28 Jun 2016)

Log Message

Web Inspector: QuickConsole should update its selection when RuntimeManager.defaultExecutionContextIdentifier changes
https://bugs.webkit.org/show_bug.cgi?id=159183

Reviewed by Timothy Hatcher.

Source/WebInspectorUI:

Currently, the UI listens for state changes in the Frame tree to decide when to reset
the selected execution context back to its default value. This is no good, because this
should happen even if we have no UI (i.e., testing models only). The UI should simply
display changes to the model rather than driving them based on other model changes.

Move the logic to reset the execution context into RuntimeManager. When the selected
context changes, an event is fired which causes the QuickConsole to rebuild its path components.

* UserInterface/Controllers/RuntimeManager.js:
(WebInspector.RuntimeManager):
(WebInspector.RuntimeManager.prototype.set defaultExecutionContextIdentifier):
Fire an event when the execution context actually changes.

(WebInspector.RuntimeManager.prototype._frameExecutionContextsCleared):
Added. If the selected context was cleared, reset back to default.

* UserInterface/Models/Frame.js:
(WebInspector.Frame.prototype.clearExecutionContexts):
Include the contexts that were cleared so clients can match against them.

* UserInterface/Views/QuickConsole.js:
(WebInspector.QuickConsole):
Use the proper constant name. Both the old and new names evaluated to `undefined`.
No need to keep track of the selected path component, it will always match the
defaultExecutionContextIdentifier in RuntimeManager.

(WebInspector.QuickConsole.prototype.get selectedExecutionContextIdentifier):
(WebInspector.QuickConsole.prototype.set selectedExecutionContextIdentifier):
Forward to RuntimeManager. This name is less awkward for the UI code that manages selections.

(WebInspector.QuickConsole.prototype._executionContextPathComponentsToDisplay):
Special-case for the main frame execution context.

(WebInspector.QuickConsole.prototype._framePageExecutionContextsChanged):
Remove indirection.

(WebInspector.QuickConsole.prototype._frameExecutionContextsCleared):
Fix the guard to handle undefined execution contexts in the case where it represents the main frame (undefined).

(WebInspector.QuickConsole.prototype._defaultExecutionContextChanged):
Rebuild when the model changes.

(WebInspector.QuickConsole.prototype._pathComponentSelected): Simplify.
(WebInspector.QuickConsole.prototype.get executionContextIdentifier):
(WebInspector.QuickConsole.prototype._removeExecutionContextPathComponentForFrame):
Move the fallback selection behavior into RuntimeManager.

(WebInspector.QuickConsole.prototype._updateExecutionContextPathComponentForFrame): Deleted.
This has been dead code ever since we removed iOS 6 legacy support.

LayoutTests:

Add a new test case for reverting to the top-level execution context when the selected context is destroyed.

* inspector/runtime/change-execution-context-identifier-expected.txt:
* inspector/runtime/change-execution-context-identifier.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (202565 => 202566)


--- trunk/LayoutTests/ChangeLog	2016-06-28 15:10:50 UTC (rev 202565)
+++ trunk/LayoutTests/ChangeLog	2016-06-28 15:12:10 UTC (rev 202566)
@@ -1,3 +1,15 @@
+2016-06-28  Brian Burg  <[email protected]>
+
+        Web Inspector: QuickConsole should update its selection when RuntimeManager.defaultExecutionContextIdentifier changes
+        https://bugs.webkit.org/show_bug.cgi?id=159183
+
+        Reviewed by Timothy Hatcher.
+
+        Add a new test case for reverting to the top-level execution context when the selected context is destroyed.
+
+        * inspector/runtime/change-execution-context-identifier-expected.txt:
+        * inspector/runtime/change-execution-context-identifier.html:
+
 2016-06-28  Adam Bergkvist  <[email protected]>
 
         WebRTC: Robustify 'this' type check in RTCPeerConnection JS built-ins

Modified: trunk/LayoutTests/inspector/runtime/change-execution-context-identifier-expected.txt (202565 => 202566)


--- trunk/LayoutTests/inspector/runtime/change-execution-context-identifier-expected.txt	2016-06-28 15:10:50 UTC (rev 202565)
+++ trunk/LayoutTests/inspector/runtime/change-execution-context-identifier-expected.txt	2016-06-28 15:12:10 UTC (rev 202566)
@@ -1,4 +1,3 @@
-
 Test that RuntimeManager.evaluateInInspectedWindow respects the selected execution context.
 
 
@@ -19,3 +18,8 @@
 Passphrase in selected frame: coldwater
 PASS: The passphrase should match the phrase defined in the main frame.
 
+-- Running test case: ScriptExecutionContextRemoveSubframe
+PASS: The test page should only have one sub-frame.
+PASS: The test page should now have no sub-frames.
+PASS: When a selected non-top-level execution context is removed, the default execution context should revert to the top-level context.
+

Modified: trunk/LayoutTests/inspector/runtime/change-execution-context-identifier.html (202565 => 202566)


--- trunk/LayoutTests/inspector/runtime/change-execution-context-identifier.html	2016-06-28 15:10:50 UTC (rev 202565)
+++ trunk/LayoutTests/inspector/runtime/change-execution-context-identifier.html	2016-06-28 15:12:10 UTC (rev 202566)
@@ -36,7 +36,7 @@
         description: "Test that evaluateInInspectedWindow works when defaultExecutionContextIdentifier is a subframe context.",
         test: (resolve, reject) => {
             let mainFrame = WebInspector.frameResourceManager.mainFrame;
-            let subframes = WebInspector.frameResourceManager.frames.filter((frame) => frame != mainFrame);
+            let subframes = WebInspector.frameResourceManager.frames.filter((frame) => frame !== mainFrame);
             InspectorTest.expectThat(subframes.length === 1, "The test page should only have one sub-frame.");
 
             WebInspector.runtimeManager.defaultExecutionContextIdentifier = subframes[0].pageExecutionContext.id;
@@ -63,12 +63,35 @@
         }
     });
 
+    suite.addTestCase({
+        name: "ScriptExecutionContextRemoveSubframe",
+        description: "Test that evaluateInInspectedWindow works when the defaultExecutionContextIdentifier is destroyed and reverts to the top-level context.",
+        test: (resolve, reject) => {
+            let mainFrame = WebInspector.frameResourceManager.mainFrame;
+            let subframes = WebInspector.frameResourceManager.frames.filter((frame) => frame !== mainFrame);
+            InspectorTest.expectThat(subframes.length === 1, "The test page should only have one sub-frame.");
+
+            // Set the execution context to the subframe so we can switch away from it when the frame is detached.
+            WebInspector.runtimeManager.defaultExecutionContextIdentifier = subframes[0].pageExecutionContext.id;
+
+            // Force-override the contextId, otherwise we won't be able to access the iframe's DOM element when evaluating in the iframe execution context.
+            let _expression_ = `document.getElementById("subframe").remove();`;
+            let objectGroup = "test";
+            let contextId = WebInspector.RuntimeManager.TopLevelExecutionContextIdentifier;
+            RuntimeAgent.evaluate.invoke({_expression_, objectGroup, contextId}, () => {
+                InspectorTest.expectThat(WebInspector.frameResourceManager.frames.length === 1, "The test page should now have no sub-frames.");
+                InspectorTest.expectThat(WebInspector.runtimeManager.defaultExecutionContextIdentifier === WebInspector.RuntimeManager.TopLevelExecutionContextIdentifier, "When a selected non-top-level execution context is removed, the default execution context should revert to the top-level context.");
+                resolve();
+            });
+        }
+    });
+
     suite.runTestCasesAndFinish();
 }
 </script>
 </head>
 <body>
-<iframe src="" _onload_="runTest()"></iframe>
+<iframe id="subframe" src="" _onload_="runTest()"></iframe>
 <p>Test that RuntimeManager.evaluateInInspectedWindow respects the selected execution context.</p>
 </body>
 </html>

Modified: trunk/Source/WebInspectorUI/ChangeLog (202565 => 202566)


--- trunk/Source/WebInspectorUI/ChangeLog	2016-06-28 15:10:50 UTC (rev 202565)
+++ trunk/Source/WebInspectorUI/ChangeLog	2016-06-28 15:12:10 UTC (rev 202566)
@@ -1,3 +1,60 @@
+2016-06-28  Brian Burg  <[email protected]>
+
+        Web Inspector: QuickConsole should update its selection when RuntimeManager.defaultExecutionContextIdentifier changes
+        https://bugs.webkit.org/show_bug.cgi?id=159183
+
+        Reviewed by Timothy Hatcher.
+
+        Currently, the UI listens for state changes in the Frame tree to decide when to reset
+        the selected execution context back to its default value. This is no good, because this
+        should happen even if we have no UI (i.e., testing models only). The UI should simply
+        display changes to the model rather than driving them based on other model changes.
+
+        Move the logic to reset the execution context into RuntimeManager. When the selected
+        context changes, an event is fired which causes the QuickConsole to rebuild its path components.
+
+        * UserInterface/Controllers/RuntimeManager.js:
+        (WebInspector.RuntimeManager):
+        (WebInspector.RuntimeManager.prototype.set defaultExecutionContextIdentifier):
+        Fire an event when the execution context actually changes.
+
+        (WebInspector.RuntimeManager.prototype._frameExecutionContextsCleared):
+        Added. If the selected context was cleared, reset back to default.
+
+        * UserInterface/Models/Frame.js:
+        (WebInspector.Frame.prototype.clearExecutionContexts):
+        Include the contexts that were cleared so clients can match against them.
+
+        * UserInterface/Views/QuickConsole.js:
+        (WebInspector.QuickConsole):
+        Use the proper constant name. Both the old and new names evaluated to `undefined`.
+        No need to keep track of the selected path component, it will always match the
+        defaultExecutionContextIdentifier in RuntimeManager.
+
+        (WebInspector.QuickConsole.prototype.get selectedExecutionContextIdentifier):
+        (WebInspector.QuickConsole.prototype.set selectedExecutionContextIdentifier):
+        Forward to RuntimeManager. This name is less awkward for the UI code that manages selections.
+
+        (WebInspector.QuickConsole.prototype._executionContextPathComponentsToDisplay):
+        Special-case for the main frame execution context.
+
+        (WebInspector.QuickConsole.prototype._framePageExecutionContextsChanged):
+        Remove indirection.
+
+        (WebInspector.QuickConsole.prototype._frameExecutionContextsCleared):
+        Fix the guard to handle undefined execution contexts in the case where it represents the main frame (undefined).
+
+        (WebInspector.QuickConsole.prototype._defaultExecutionContextChanged):
+        Rebuild when the model changes.
+
+        (WebInspector.QuickConsole.prototype._pathComponentSelected): Simplify.
+        (WebInspector.QuickConsole.prototype.get executionContextIdentifier):
+        (WebInspector.QuickConsole.prototype._removeExecutionContextPathComponentForFrame):
+        Move the fallback selection behavior into RuntimeManager.
+
+        (WebInspector.QuickConsole.prototype._updateExecutionContextPathComponentForFrame): Deleted.
+        This has been dead code ever since we removed iOS 6 legacy support.
+
 2016-06-27  Joseph Pecoraro  <[email protected]>
 
         Web Inspector: When modifying sessionStorage, localStorage gets updated

Modified: trunk/Source/WebInspectorUI/UserInterface/Controllers/RuntimeManager.js (202565 => 202566)


--- trunk/Source/WebInspectorUI/UserInterface/Controllers/RuntimeManager.js	2016-06-28 15:10:50 UTC (rev 202565)
+++ trunk/Source/WebInspectorUI/UserInterface/Controllers/RuntimeManager.js	2016-06-28 15:12:10 UTC (rev 202566)
@@ -31,6 +31,8 @@
 
         // Enable the RuntimeAgent to receive notification of execution contexts.
         RuntimeAgent.enable();
+
+        WebInspector.Frame.addEventListener(WebInspector.Frame.Event.ExecutionContextsCleared, this._frameExecutionContextsCleared, this);
     }
 
     // Public
@@ -124,13 +126,32 @@
     }
 
     get defaultExecutionContextIdentifier() { return this._defaultExecutionContextIdentifier; }
-    set defaultExecutionContextIdentifier(value) { this._defaultExecutionContextIdentifier = value; }
+    set defaultExecutionContextIdentifier(value)
+    {
+        if (this._defaultExecutionContextIdentifier === value)
+            return;
+
+        this._defaultExecutionContextIdentifier = value;
+        this.dispatchEventToListeners(WebInspector.RuntimeManager.Event.DefaultExecutionContextChanged);
+    }
+
+    // Private
+
+    _frameExecutionContextsCleared(event)
+    {
+        let contexts = event.data.contexts || [];
+
+        let currentContextWasDestroyed = contexts.some((context) => context.id === this._defaultExecutionContextIdentifier);
+        if (currentContextWasDestroyed)
+            this.defaultExecutionContextIdentifier = WebInspector.RuntimeManager.TopLevelExecutionContextIdentifier;
+    }
 };
 
 WebInspector.RuntimeManager.TopLevelExecutionContextIdentifier = undefined;
 
 WebInspector.RuntimeManager.Event = {
-    DidEvaluate: "runtime-manager-did-evaluate"
+    DidEvaluate: Symbol("runtime-manager-did-evaluate"),
+    DefaultExecutionContextChanged: Symbol("runtime-manager-default-execution-context-changed"),
 };
 
 WebInspector.RuntimeManager.ConsoleObjectGroup = "console";

Modified: trunk/Source/WebInspectorUI/UserInterface/Models/Frame.js (202565 => 202566)


--- trunk/Source/WebInspectorUI/UserInterface/Models/Frame.js	2016-06-28 15:10:50 UTC (rev 202565)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/Frame.js	2016-06-28 15:12:10 UTC (rev 202566)
@@ -206,8 +206,9 @@
     clearExecutionContexts(committingProvisionalLoad)
     {
         if (this._executionContextList.contexts.length) {
+            let contexts = this._executionContextList.contexts.slice();
             this._executionContextList.clear();
-            this.dispatchEventToListeners(WebInspector.Frame.Event.ExecutionContextsCleared, {committingProvisionalLoad:!!committingProvisionalLoad});
+            this.dispatchEventToListeners(WebInspector.Frame.Event.ExecutionContextsCleared, {committingProvisionalLoad:!!committingProvisionalLoad, contexts});
         }
     }
 

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/QuickConsole.js (202565 => 202566)


--- trunk/Source/WebInspectorUI/UserInterface/Views/QuickConsole.js	2016-06-28 15:10:50 UTC (rev 202565)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/QuickConsole.js	2016-06-28 15:12:10 UTC (rev 202566)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013, 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2013, 2015, 2016 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -31,9 +31,8 @@
 
         this._toggleOrFocusKeyboardShortcut = new WebInspector.KeyboardShortcut(null, WebInspector.KeyboardShortcut.Key.Escape, this._toggleOrFocus.bind(this));
 
-        var mainFrameExecutionContext = new WebInspector.ExecutionContext(WebInspector.QuickConsole.MainFrameContextExecutionIdentifier, WebInspector.UIString("Main Frame"), true, null);
+        let mainFrameExecutionContext = new WebInspector.ExecutionContext(WebInspector.RuntimeManager.TopLevelContextExecutionIdentifier, WebInspector.UIString("Main Frame"), true, null);
         this._mainFrameExecutionContextPathComponent = this._createExecutionContextPathComponent(mainFrameExecutionContext.name, mainFrameExecutionContext.identifier);
-        this._selectedExecutionContextPathComponent = this._mainFrameExecutionContextPathComponent;
 
         this._otherExecutionContextPathComponents = [];
         this._frameIdentifierToExecutionContextPathComponentMap = {};
@@ -70,6 +69,8 @@
         WebInspector.Frame.addEventListener(WebInspector.Frame.Event.ExecutionContextsCleared, this._frameExecutionContextsCleared, this);
 
         WebInspector.debuggerManager.addEventListener(WebInspector.DebuggerManager.Event.ActiveCallFrameDidChange, this._debuggerActiveCallFrameDidChange, this);
+
+        WebInspector.runtimeManager.addEventListener(WebInspector.RuntimeManager.Event.DefaultExecutionContextChanged, this._defaultExecutionContextChanged, this);
     }
 
     // Public
@@ -79,11 +80,19 @@
         return this._navigationBar;
     }
 
-    get executionContextIdentifier()
+    get selectedExecutionContextIdentifier()
     {
-        return this._selectedExecutionContextPathComponent._executionContextIdentifier;
+        return WebInspector.runtimeManager.defaultExecutionContextIdentifier;
     }
 
+    set selectedExecutionContextIdentifier(value)
+    {
+        if (value === this.selectedExecutionContextIdentifier)
+            return;
+
+        WebInspector.runtimeManager.defaultExecutionContextIdentifier = value;
+    }
+
     consoleLogVisibilityChanged(visible)
     {
         if (visible === this.element.classList.contains(WebInspector.QuickConsole.ShowingLogClassName))
@@ -124,7 +133,10 @@
         if (!this._otherExecutionContextPathComponents.length)
             return [];
 
-        return [this._selectedExecutionContextPathComponent];
+        if (this.selectedExecutionContextIdentifier === this._mainFrameExecutionContextPathComponent._executionContextIdentifier)
+            return [this._mainFrameExecutionContextPathComponent];
+
+        return this._otherExecutionContextPathComponents.filter((component) => component._executionContextIdentifier === this.selectedExecutionContextIdentifier);
     }
 
     _rebuildExecutionContextPathComponents()
@@ -147,24 +159,22 @@
         var newExecutionContextPathComponent = this._insertExecutionContextPathComponentForFrame(frame, shouldAutomaticallySelect);
 
         if (shouldAutomaticallySelect) {
-            delete this._restoreSelectedExecutionContextForFrame;
-            this._selectedExecutionContextPathComponent = newExecutionContextPathComponent;
-            WebInspector.runtimeManager.defaultExecutionContextIdentifier = this.executionContextIdentifier;
-            this._rebuildExecutionContextPathComponents();
+            this._restoreSelectedExecutionContextForFrame = null;
+            this.selectedExecutionContextIdentifier = newExecutionContextPathComponent._executionContextIdentifier;
         }
     }
 
     _frameExecutionContextsCleared(event)
     {
-        var frame = event.target;
+        let frame = event.target;
 
         // If this frame is navigating and it is selected in the UI we want to reselect its new item after navigation.
         if (event.data.committingProvisionalLoad && !this._restoreSelectedExecutionContextForFrame) {
-            var executionContextPathComponent = this._frameIdentifierToExecutionContextPathComponentMap[frame.id];
-            if (this._selectedExecutionContextPathComponent === executionContextPathComponent) {
+            let executionContextPathComponent = this._frameIdentifierToExecutionContextPathComponentMap[frame.id];
+            if (executionContextPathComponent && executionContextPathComponent._executionContextIdentifier === this.selectedExecutionContextIdentifier) {
                 this._restoreSelectedExecutionContextForFrame = frame;
                 // As a fail safe, if the frame never gets an execution context, clear the restore value.
-                setTimeout(function() { delete this._restoreSelectedExecutionContextForFrame; }.bind(this), 10);
+                setTimeout(() => { this._restoreSelectedExecutionContextForFrame = false; }, 10);
             }
         }
 
@@ -171,6 +181,11 @@
         this._removeExecutionContextPathComponentForFrame(frame);
     }
 
+    _defaultExecutionContextChanged(event)
+    {
+        this._rebuildExecutionContextPathComponents();
+    }
+
     _createExecutionContextPathComponent(name, identifier)
     {
         var pathComponent = new WebInspector.HierarchicalPathComponent(name, "execution-context", identifier, true, true);
@@ -261,11 +276,6 @@
         if (next)
             next.previousSibling = prev;
 
-        if (this._selectedExecutionContextPathComponent === executionContextPathComponent) {
-            this._selectedExecutionContextPathComponent = this._mainFrameExecutionContextPathComponent;
-            WebInspector.runtimeManager.defaultExecutionContextIdentifier = this.executionContextIdentifier;
-        }
-
         this._otherExecutionContextPathComponents.remove(executionContextPathComponent, true);
         delete this._frameIdentifierToExecutionContextPathComponentMap[frame.id];
 
@@ -273,37 +283,9 @@
             this._rebuildExecutionContextPathComponents();
     }
 
-    _updateExecutionContextPathComponentForFrame(frame)
-    {
-        if (frame.isMainFrame())
-            return;
-
-        var executionContextPathComponent = this._frameIdentifierToExecutionContextPathComponentMap[frame.id];
-        if (!executionContextPathComponent)
-            return;
-
-        var wasSelected = this._selectedExecutionContextPathComponent === executionContextPathComponent;
-
-        this._removeExecutionContextPathComponentForFrame(frame, true);
-        var newExecutionContextPathComponent = this._insertExecutionContextPathComponentForFrame(frame, true);
-
-        if (wasSelected) {
-            this._selectedExecutionContextPathComponent = newExecutionContextPathComponent;
-            WebInspector.runtimeManager.defaultExecutionContextIdentifier = this.executionContextIdentifier;
-        }
-
-        this._rebuildExecutionContextPathComponents();
-    }
-
     _pathComponentSelected(event)
     {
-        if (event.data.pathComponent === this._selectedExecutionContextPathComponent)
-            return;
-
-        this._selectedExecutionContextPathComponent = event.data.pathComponent;
-        WebInspector.runtimeManager.defaultExecutionContextIdentifier = this.executionContextIdentifier;
-
-        this._rebuildExecutionContextPathComponents();
+        this.selectedExecutionContextIdentifier = event.data.pathComponent._executionContextIdentifier;
     }
 
     _pathComponentClicked(event)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to