Changeset 199897 in webkit

Timestamp:

Apr 22, 2016 12:53:37 PM (8 years ago)

Author:

timothy@apple.com

Message:

Web Inspector: Debugger statement in console does not provide any call frames and debugger UI is confused

​https://bugs.webkit.org/show_bug.cgi?id=156919
rdar://problem/25857118

This makes console expressions show up in the Debugger tab sidebar if a ScriptContentView is shown for them.
We now also show call frames that originate from a console expression, so the call frames in the sidebar is not empty.
Also fix a bug where when there are no call frames we auto resume the debugger and don't leave it in a broken state.

Reviewed by Joseph Pecoraro.

• Localizations/en.lproj/localizedStrings.js: Updated.

• UserInterface/Base/Utilities.js:

(appendWebInspectorSourceURL): Don't append if another sourceURL is already added.
(appendWebInspectorConsoleEvaluationSourceURL): Added.
(isWebInspectorConsoleEvaluationScript): Added.
(isWebKitInternalScript): Return false for isWebInspectorConsoleEvaluationScript().

• UserInterface/Controllers/DebuggerManager.js:

(WebInspector.DebuggerManager.prototype.debuggerDidPause): Resume if call frames is empty. This is not as common now
since console expression call frames are not skipped.
(WebInspector.DebuggerManager.prototype.scriptDidParse): Change an early return for isWebInspectorInternalScript() that
was skipping adding internal scripts to the known script lists, but it should only do that when the debug UI is disabled.

• UserInterface/Controllers/JavaScriptLogViewController.js:

(WebInspector.JavaScriptLogViewController.prototype.consolePromptTextCommitted):
Call appendWebInspectorConsoleEvaluationSourceURL so the console expressions are tagged before evaluateInInspectedWindow
added the internal sourceURL name.

• UserInterface/Models/Script.js:

(WebInspector.Script): Assign unique identifiers to console scripts so they are named correctly.
(WebInspector.Script.resetUniqueDisplayNameNumbers): Reset _nextUniqueConsoleDisplayNameNumber.
(WebInspector.Script.prototype.get displayName): Special case console expressions with a better name.

• UserInterface/Views/DebuggerSidebarPanel.js:

(WebInspector.DebuggerSidebarPanel.prototype.treeElementForRepresentedObject): Add a script tree element on demand
like the ResourceSidebarPanel does for anonymous scripts.
(WebInspector.DebuggerSidebarPanel.prototype._addScript): Return treeElement so treeElementForRepresentedObject can use it.

Location:

trunk/Source/WebInspectorUI

Files:

• ChangeLog (modified) (1 diff)
• Localizations/en.lproj/localizedStrings.js (modified) (1 diff)
• UserInterface/Base/Utilities.js (modified) (2 diffs)
• UserInterface/Controllers/DebuggerManager.js (modified) (3 diffs)
• UserInterface/Controllers/JavaScriptLogViewController.js (modified) (1 diff)
• UserInterface/Models/Script.js (modified) (4 diffs)
• UserInterface/Views/DebuggerSidebarPanel.js (modified) (3 diffs)

trunk/Source/WebInspectorUI/ChangeLog

@@ +1 @@
+2016-04-22  Timothy Hatcher  <timothy@apple.com>
+
+        Web Inspector: Debugger statement in console does not provide any call frames and debugger UI is confused
+
+        https://bugs.webkit.org/show_bug.cgi?id=156919
+        rdar://problem/25857118
+
+        This makes console expressions show up in the Debugger tab sidebar if a ScriptContentView is shown for them.
+        We now also show call frames that originate from a console expression, so the call frames in the sidebar is not empty.
+        Also fix a bug where when there are no call frames we auto resume the debugger and don't leave it in a broken state.
+
+        Reviewed by Joseph Pecoraro.
+
+        * Localizations/en.lproj/localizedStrings.js: Updated.
+
+        * UserInterface/Base/Utilities.js:
+        (appendWebInspectorSourceURL): Don't append if another sourceURL is already added.
+        (appendWebInspectorConsoleEvaluationSourceURL): Added.
+        (isWebInspectorConsoleEvaluationScript): Added.
+        (isWebKitInternalScript): Return false for isWebInspectorConsoleEvaluationScript().
+
+        * UserInterface/Controllers/DebuggerManager.js:
+        (WebInspector.DebuggerManager.prototype.debuggerDidPause): Resume if call frames is empty. This is not as common now
+        since console expression call frames are not skipped.
+        (WebInspector.DebuggerManager.prototype.scriptDidParse): Change an early return for isWebInspectorInternalScript() that
+        was skipping adding internal scripts to the known script lists, but it should only do that when the debug UI is disabled.
+
+        * UserInterface/Controllers/JavaScriptLogViewController.js:
+        (WebInspector.JavaScriptLogViewController.prototype.consolePromptTextCommitted):
+        Call appendWebInspectorConsoleEvaluationSourceURL so the console expressions are tagged before evaluateInInspectedWindow
+        added the internal sourceURL name.
+
+        * UserInterface/Models/Script.js:
+        (WebInspector.Script): Assign unique identifiers to console scripts so they are named correctly.
+        (WebInspector.Script.resetUniqueDisplayNameNumbers): Reset _nextUniqueConsoleDisplayNameNumber.
+        (WebInspector.Script.prototype.get displayName): Special case console expressions with a better name.
+
+        * UserInterface/Views/DebuggerSidebarPanel.js:
+        (WebInspector.DebuggerSidebarPanel.prototype.treeElementForRepresentedObject): Add a script tree element on demand
+        like the ResourceSidebarPanel does for anonymous scripts.
+        (WebInspector.DebuggerSidebarPanel.prototype._addScript): Return treeElement so treeElementForRepresentedObject can use it.
+
 2016-04-21  Joseph Pecoraro  <pecoraro@apple.com>
 

trunk/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js

@@ -182 +182 @@
 localizedStrings["Conditional expression"] = "Conditional expression";
 localizedStrings["Console"] = "Console";
+localizedStrings["Console Evaluation %d"] = "Console Evaluation %d";
 localizedStrings["Console Profile Recorded"] = "Console Profile Recorded";
 localizedStrings["Console errors, click to show the Console tab"] = "Console errors, click to show the Console tab";

trunk/Source/WebInspectorUI/UserInterface/Base/Utilities.js

@@ -1257 +1257 @@
 function appendWebInspectorSourceURL(string)
 {
+    if (string.includes("//# sourceURL"))
+        return string;
     return "\n//# sourceURL=__WebInspectorInternal__\n" + string;
 }
 
+function appendWebInspectorConsoleEvaluationSourceURL(string)
+{
+    if (string.includes("//# sourceURL"))
+        return string;
+    return "\n//# sourceURL=__WebInspectorConsoleEvaluation__\n" + string;
+}
+
 function isWebInspectorInternalScript(url)
 {
@@ -1265 +1274 @@
 }
 
+function isWebInspectorConsoleEvaluationScript(url)
+{
+    return url === "__WebInspectorConsoleEvaluation__";
+}
+
 function isWebKitInternalScript(url)
 {
+    if (isWebInspectorConsoleEvaluationScript(url))
+        return false;
     return url && url.startsWith("__Web") && url.endsWith("__");
 }

trunk/Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js

@@ -546 +546 @@
         this._activeCallFrame = this._callFrames[0];
 
+        if (!this._activeCallFrame) {
+            console.assert("We should always have one call frame. This could indicate we are hitting an exception or debugger statement in an internal injected script.");
+            this._didResumeInternal();
+            return;
+        }
+
         if (!wasStillPaused)
             this.dispatchEventToListeners(WebInspector.DebuggerManager.Event.Paused);
@@ -579 +585 @@
         }
 
-        if (isWebInspectorInternalScript(sourceURL))
+        if (!WebInspector.isDebugUIEnabled() && isWebKitInternalScript(sourceURL))
             return;
 
@@ -600 +606 @@
                 return;
         }
+
+        // Console expressions are not added to the UI by default.
+        if (isWebInspectorConsoleEvaluationScript(script.sourceURL))
+            return;
 
         this.dispatchEventToListeners(WebInspector.DebuggerManager.Event.ScriptAdded, {script});

trunk/Source/WebInspectorUI/UserInterface/Controllers/JavaScriptLogViewController.js

@@ -229 +229 @@
         }
 
+        text = appendWebInspectorConsoleEvaluationSourceURL(text);
+
         WebInspector.runtimeManager.evaluateInInspectedWindow(text, WebInspector.RuntimeManager.ConsoleObjectGroup, true, false, false, true, true, printResult.bind(this));
     }

trunk/Source/WebInspectorUI/UserInterface/Models/Script.js

@@ -46 +46 @@
             WebInspector.sourceMapManager.downloadSourceMap(sourceMapURL, this._url, this);
 
+        if (isWebInspectorConsoleEvaluationScript(this._sourceURL)) {
+            // Assign a unique number to the script object so it will stay the same.
+            this._uniqueDisplayNameNumber = this.constructor._nextUniqueConsoleDisplayNameNumber++;
+        }
+
         this._scriptSyntaxTree = null;
     }
@@ -54 +59 @@
     {
         WebInspector.Script._nextUniqueDisplayNameNumber = 1;
+        WebInspector.Script._nextUniqueConsoleDisplayNameNumber = 1;
     }
 
@@ -94 +100 @@
         if (this._url)
             return WebInspector.displayNameForURL(this._url, this.urlComponents);
+
+        if (isWebInspectorConsoleEvaluationScript(this._sourceURL)) {
+            console.assert(this._uniqueDisplayNameNumber);
+            return WebInspector.UIString("Console Evaluation %d").format(this._uniqueDisplayNameNumber);
+        }
 
         if (this._sourceURL) {
@@ -248 +259 @@
 
 WebInspector.Script._nextUniqueDisplayNameNumber = 1;
+WebInspector.Script._nextUniqueConsoleDisplayNameNumber = 1;

trunk/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js

@@ -209 +209 @@
             representedObject = representedObject.mainResource;
 
-        return this.contentTreeOutline.getCachedTreeElement(representedObject);
+        let treeElement = this.contentTreeOutline.findTreeElement(representedObject);
+        if (treeElement)
+            return treeElement;
+
+        // Only special case Script objects.
+        if (!(representedObject instanceof WebInspector.Script)) {
+            console.error("Didn't find a TreeElement for representedObject", representedObject);
+            return null;
+        }
+
+        // If the Script has a URL we should have found it earlier.
+        if (representedObject.url) {
+            console.error("Didn't find a ScriptTreeElement for a Script with a URL.");
+            return null;
+        }
+
+        // Since the Script does not have a URL we consider it an 'anonymous' script. These scripts happen from calls to
+        // window.eval() or browser features like Auto Fill and Reader. They are not normally added to the sidebar, but since
+        // we have a ScriptContentView asking for the tree element we will make a ScriptTreeElement on demand and add it.
+
+        return this._addScript(representedObject);
     }
 
@@ -430 +450 @@
         // COMPATIBILITY(iOS 9): Backends could send the frontend built-in code, filter out JSC internals.
         if (!script.url && !script.sourceURL)
-            return;
+            return null;
 
         // Don't add breakpoints if the script is represented by a Resource. They were
         // already added by _resourceAdded.
         if (script.resource)
-            return;
+            return null;
 
         let treeElement = this._addTreeElementForSourceCodeToContentTreeOutline(script);
@@ -443 +463 @@
         if (!this.contentBrowser.currentContentView)
             this.showDefaultContentViewForTreeElement(treeElement);
+
+        return treeElement;
     }