Title: [154998] trunk
Revision
154998
Author
grao...@apple.com
Date
2013-09-03 13:16:29 -0700 (Tue, 03 Sep 2013)

Log Message

Web Inspector: exceptions triggered from console evaluation do not pause the debugger
https://bugs.webkit.org/show_bug.cgi?id=120460

Source/WebCore:

Reviewed by Timothy Hatcher.

* inspector/InjectedScriptSource.js:
Explicitly set a sourceURL such that the frontend may identify injected script when
processing call frames in order to hide such code from the debugger.

Source/WebInspectorUI:

We used to preclude any debugging from errors stemming from code evaluated in the console
as we would always set the doNotPauseOnExceptionsAndMuteConsole parameter to "false" when
calling _javascript_LogViewController._evaluateInInspectedWindow(). However, it is desirable
to allow debugging code ran from the console.

We now allow debugging in such a scenario and we filter out call frames coming from the
Web Inspector injected script as well as the call frame for the console prompt such that
we only pause in the debugger in case the exception is in code under the console prompt
and not the console code prompt itself.

Additionally, to prevent stepping out to call frames we may have filtered out, we disable
the "step out" button in cases where there are no further frames in the frontend to go out to.

Reviewed by Timothy Hatcher.

* UserInterface/DebuggerManager.js:
(WebInspector.DebuggerManager.prototype.debuggerDidPause):
Filter out call frames that have a URL coming from Web Inspector injected script by looking
for a URL starting with the "__WebInspector" prefix. If we determine that there are no call
frames left after filtering, we resume code evaluation such that we only pause in the debugger
when the exception is in code evluated under the console prompt.

* UserInterface/DebuggerSidebarPanel.js:
(WebInspector.DebuggerSidebarPanel):
(WebInspector.DebuggerSidebarPanel.prototype._debuggerDidPause):
(WebInspector.DebuggerSidebarPanel.prototype._debuggerActiveCallFrameDidChange):
Monitor any change to the active call frame such that we may tie the state of the
"step out" button to the availability of a call frame to step out to in the filtered
list set on the DebuggerManager.

* UserInterface/_javascript_LogViewController.js:
(WebInspector._javascript_LogViewController.prototype.consolePromptTextCommitted):
Set the doNotPauseOnExceptionsAndMuteConsole to "false" when calling _evaluateInInspectedWindow()
in order to allow pausing on exceptions coming from code evalued in the console. Also, explicitly
set a sourceURL for the script to evaluate such that we may identify its origin when filtering
call frames stemming from inspector code.

* UserInterface/ResourceSidebarPanel.js:
(WebInspector.ResourceSidebarPanel.prototype._scriptWasAdded):
Filter out any script resource starting with the Web Inspector-specific "__WebInspector" prefix
so that injected script does not show up.

LayoutTests:

Reviewed by Timothy Hatcher.

* platform/mac/inspector/console/command-line-api-expected.txt:
Take into account the addition of a sourceURL to inspector/InjectedScriptSource.js.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (154997 => 154998)


--- trunk/LayoutTests/ChangeLog	2013-09-03 19:30:20 UTC (rev 154997)
+++ trunk/LayoutTests/ChangeLog	2013-09-03 20:16:29 UTC (rev 154998)
@@ -1,3 +1,13 @@
+2013-09-03  Antoine Quint  <grao...@apple.com>
+
+        Web Inspector: exceptions triggered from console evaluation do not pause the debugger
+        https://bugs.webkit.org/show_bug.cgi?id=120460
+
+        Reviewed by Timothy Hatcher.
+
+        * platform/mac/inspector/console/command-line-api-expected.txt:
+        Take into account the addition of a sourceURL to inspector/InjectedScriptSource.js.
+
 2013-08-28  Sergio Villar Senin  <svil...@igalia.com>
 
         [CSS Grid Layout] Add parsing for named grid lines

Modified: trunk/LayoutTests/platform/mac/inspector/console/command-line-api-expected.txt (154997 => 154998)


--- trunk/LayoutTests/platform/mac/inspector/console/command-line-api-expected.txt	2013-09-03 19:30:20 UTC (rev 154997)
+++ trunk/LayoutTests/platform/mac/inspector/console/command-line-api-expected.txt	2013-09-03 20:16:29 UTC (rev 154998)
@@ -1,4 +1,4 @@
-CONSOLE MESSAGE: line 1225: The console function $() has changed from $=getElementById(id) to $=querySelector(selector). You might try $("#%s")
+CONSOLE MESSAGE: line 1227: The console function $() has changed from $=getElementById(id) to $=querySelector(selector). You might try $("#%s")
 Tests that command line api works.
 
 

Modified: trunk/Source/WebCore/ChangeLog (154997 => 154998)


--- trunk/Source/WebCore/ChangeLog	2013-09-03 19:30:20 UTC (rev 154997)
+++ trunk/Source/WebCore/ChangeLog	2013-09-03 20:16:29 UTC (rev 154998)
@@ -1,3 +1,14 @@
+2013-09-03  Antoine Quint  <grao...@apple.com>
+
+        Web Inspector: exceptions triggered from console evaluation do not pause the debugger
+        https://bugs.webkit.org/show_bug.cgi?id=120460
+
+        Reviewed by Timothy Hatcher.
+
+        * inspector/InjectedScriptSource.js:
+        Explicitly set a sourceURL such that the frontend may identify injected script when
+        processing call frames in order to hide such code from the debugger.
+
 2013-09-03  Andreas Kling  <akl...@apple.com>
 
         Support Vector<Ref<T>>.

Modified: trunk/Source/WebCore/inspector/InjectedScriptSource.js (154997 => 154998)


--- trunk/Source/WebCore/inspector/InjectedScriptSource.js	2013-09-03 19:30:20 UTC (rev 154997)
+++ trunk/Source/WebCore/inspector/InjectedScriptSource.js	2013-09-03 20:16:29 UTC (rev 154998)
@@ -26,6 +26,8 @@
  * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+//# sourceURL=__WebInspectorInjectedScript__
+
 /**
  * @param {InjectedScriptHost} InjectedScriptHost
  * @param {Window} inspectedWindow

Modified: trunk/Source/WebInspectorUI/ChangeLog (154997 => 154998)


--- trunk/Source/WebInspectorUI/ChangeLog	2013-09-03 19:30:20 UTC (rev 154997)
+++ trunk/Source/WebInspectorUI/ChangeLog	2013-09-03 20:16:29 UTC (rev 154998)
@@ -1,3 +1,50 @@
+2013-09-03  Antoine Quint  <grao...@apple.com>
+
+        Web Inspector: exceptions triggered from console evaluation do not pause the debugger
+        https://bugs.webkit.org/show_bug.cgi?id=120460
+
+        We used to preclude any debugging from errors stemming from code evaluated in the console
+        as we would always set the doNotPauseOnExceptionsAndMuteConsole parameter to "false" when
+        calling _javascript_LogViewController._evaluateInInspectedWindow(). However, it is desirable
+        to allow debugging code ran from the console.
+
+        We now allow debugging in such a scenario and we filter out call frames coming from the
+        Web Inspector injected script as well as the call frame for the console prompt such that
+        we only pause in the debugger in case the exception is in code under the console prompt
+        and not the console code prompt itself.
+
+        Additionally, to prevent stepping out to call frames we may have filtered out, we disable
+        the "step out" button in cases where there are no further frames in the frontend to go out to.
+
+        Reviewed by Timothy Hatcher.
+
+        * UserInterface/DebuggerManager.js:
+        (WebInspector.DebuggerManager.prototype.debuggerDidPause):
+        Filter out call frames that have a URL coming from Web Inspector injected script by looking
+        for a URL starting with the "__WebInspector" prefix. If we determine that there are no call
+        frames left after filtering, we resume code evaluation such that we only pause in the debugger
+        when the exception is in code evluated under the console prompt.
+
+        * UserInterface/DebuggerSidebarPanel.js:
+        (WebInspector.DebuggerSidebarPanel):
+        (WebInspector.DebuggerSidebarPanel.prototype._debuggerDidPause):
+        (WebInspector.DebuggerSidebarPanel.prototype._debuggerActiveCallFrameDidChange):
+        Monitor any change to the active call frame such that we may tie the state of the
+        "step out" button to the availability of a call frame to step out to in the filtered
+        list set on the DebuggerManager.
+
+        * UserInterface/_javascript_LogViewController.js:
+        (WebInspector._javascript_LogViewController.prototype.consolePromptTextCommitted):
+        Set the doNotPauseOnExceptionsAndMuteConsole to "false" when calling _evaluateInInspectedWindow()
+        in order to allow pausing on exceptions coming from code evalued in the console. Also, explicitly
+        set a sourceURL for the script to evaluate such that we may identify its origin when filtering
+        call frames stemming from inspector code.
+
+        * UserInterface/ResourceSidebarPanel.js:
+        (WebInspector.ResourceSidebarPanel.prototype._scriptWasAdded):
+        Filter out any script resource starting with the Web Inspector-specific "__WebInspector" prefix
+        so that injected script does not show up.
+
 2013-08-30  Brent Fulgham  <bfulg...@apple.com>
 
         [Windows] Correct windows build. Get rid of unnecessary pre-build step. We don't build anything

Modified: trunk/Source/WebInspectorUI/UserInterface/DebuggerManager.js (154997 => 154998)


--- trunk/Source/WebInspectorUI/UserInterface/DebuggerManager.js	2013-09-03 19:30:20 UTC (rev 154997)
+++ trunk/Source/WebInspectorUI/UserInterface/DebuggerManager.js	2013-09-03 20:16:29 UTC (rev 154998)
@@ -343,7 +343,8 @@
         for (var i = 0; i < callFramesPayload.length; ++i) {
             var callFramePayload = callFramesPayload[i];
             var sourceCodeLocation = this._sourceCodeLocationFromPayload(callFramePayload.location);
-            if (!sourceCodeLocation)
+            // Exclude the case where the call frame is in the inspector code.
+            if (!sourceCodeLocation || sourceCodeLocation._sourceCode._url.indexOf("__WebInspector") === 0)
                 continue;
             var thisObject = WebInspector.RemoteObject.fromPayload(callFramePayload.this);
             var scopeChain = this._scopeChainFromPayload(callFramePayload.scopeChain);
@@ -351,6 +352,11 @@
             this._callFrames.push(callFrame);
         }
 
+        if (!this._callFrames.length) {
+            this.resume();
+            return;
+        }
+
         this._activeCallFrame = this._callFrames[0];
 
         if (!wasStillPaused)

Modified: trunk/Source/WebInspectorUI/UserInterface/DebuggerSidebarPanel.js (154997 => 154998)


--- trunk/Source/WebInspectorUI/UserInterface/DebuggerSidebarPanel.js	2013-09-03 19:30:20 UTC (rev 154997)
+++ trunk/Source/WebInspectorUI/UserInterface/DebuggerSidebarPanel.js	2013-09-03 20:16:29 UTC (rev 154998)
@@ -37,6 +37,7 @@
     WebInspector.debuggerManager.addEventListener(WebInspector.DebuggerManager.Event.ScriptsCleared, this._scriptsCleared, this);
     WebInspector.debuggerManager.addEventListener(WebInspector.DebuggerManager.Event.Paused, this._debuggerDidPause, this);
     WebInspector.debuggerManager.addEventListener(WebInspector.DebuggerManager.Event.Resumed, this._debuggerDidResume, this);
+    WebInspector.debuggerManager.addEventListener(WebInspector.DebuggerManager.Event.ActiveCallFrameDidChange, this._debuggerActiveCallFrameDidChange, this);
 
     this._pauseOrResumeKeyboardShortcut = new WebInspector.KeyboardShortcut(WebInspector.KeyboardShortcut.Modifier.Control | WebInspector.KeyboardShortcut.Modifier.Command, "Y", this._debuggerPauseResumeButtonClicked.bind(this));
     this._stepOverKeyboardShortcut = new WebInspector.KeyboardShortcut(null, WebInspector.KeyboardShortcut.Key.F6, this._debuggerStepOverButtonClicked.bind(this));
@@ -156,8 +157,7 @@
         this._debuggerPauseResumeButtonItem.enabled = true;
         this._debuggerPauseResumeButtonItem.toggled = true;
         this._debuggerStepOverButtonItem.enabled = true;
-        this._debuggerStepIntoButtonItem.enabled = true;
-        this._debuggerStepOutButtonItem.enabled = true;
+        this._debuggerStepIntoButtonItem.enabled = true;        
     },
 
     _debuggerDidResume: function(event)
@@ -332,6 +332,19 @@
             treeElementToSelect.select(true, true);
     },
 
+    _debuggerActiveCallFrameDidChange: function()
+    {
+        var callFrames = WebInspector.debuggerManager.callFrames;
+        if (!callFrames)
+            return;
+
+        var indexOfActiveCallFrame = callFrames.indexOf(WebInspector.debuggerManager.activeCallFrame);
+        // It is useful to turn off the step out button when there is no call frame to go through
+        // since there might be call frames in the backend that were removed when processing the call
+        // frame payload.
+        this._debuggerStepOutButtonItem.enabled = indexOfActiveCallFrame < callFrames.length - 1;
+    },
+
     _breakpointsBeneathTreeElement: function(treeElement)
     {
         console.assert(treeElement instanceof WebInspector.ResourceTreeElement || treeElement instanceof WebInspector.ScriptTreeElement);

Modified: trunk/Source/WebInspectorUI/UserInterface/_javascript_LogViewController.js (154997 => 154998)


--- trunk/Source/WebInspectorUI/UserInterface/_javascript_LogViewController.js	2013-09-03 19:30:20 UTC (rev 154997)
+++ trunk/Source/WebInspectorUI/UserInterface/_javascript_LogViewController.js	2013-09-03 20:16:29 UTC (rev 154998)
@@ -232,7 +232,9 @@
             this._appendConsoleMessage(new WebInspector.ConsoleCommandResult(result, wasThrown, commandMessage), true);
         }
 
-        this._evaluateInInspectedWindow(text, "console", true, true, false, printResult.bind(this));
+        text += "\n//# sourceURL=__WebInspectorConsole__\n";
+
+        this._evaluateInInspectedWindow(text, "console", true, false, false, printResult.bind(this));
     },
 
     consolePromptCompletionsNeeded: function(prompt, defaultCompletions, base, prefix, suffix, forced)

Modified: trunk/Source/WebInspectorUI/UserInterface/ResourceSidebarPanel.js (154997 => 154998)


--- trunk/Source/WebInspectorUI/UserInterface/ResourceSidebarPanel.js	2013-09-03 19:30:20 UTC (rev 154997)
+++ trunk/Source/WebInspectorUI/UserInterface/ResourceSidebarPanel.js	2013-09-03 20:16:29 UTC (rev 154998)
@@ -681,6 +681,10 @@
         if (!script.url)
             return;
 
+        // Exclude inspector scripts.
+        if (script.url.indexOf("__WebInspector") === 0)
+            return;
+
         // If the script URL matches a resource we can assume it is part of that resource and does not need added.
         if (script.resource)
             return;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to