Web Inspector: React.js JSXTransformer produces bogus error locations
authorjoepeck@webkit.org <joepeck@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 26 Apr 2016 00:44:52 +0000 (00:44 +0000)
committerjoepeck@webkit.org <joepeck@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 26 Apr 2016 00:44:52 +0000 (00:44 +0000)
https://bugs.webkit.org/show_bug.cgi?id=150010
<rdar://problem/23062233>

Reviewed by Timothy Hatcher.

Source/WebInspectorUI:

Show dynamically added <script> elements added to a frame as resources.
For cases where the scripts had source map resources or error messages
we have a root from which to associate them to.

* Localizations/en.lproj/localizedStrings.js:
"Script Element %d" tree element title.

* UserInterface/Models/Frame.js:
(WebInspector.Frame.prototype.commitProvisionalLoad):
(WebInspector.Frame.prototype.get extraScripts):
(WebInspector.Frame.prototype.addExtraScript):
Have a frame keep a list of its extra scripts.

* UserInterface/Models/Script.js:
(WebInspector.Script):
(WebInspector.Script.prototype.get displayName):
(WebInspector.Script.prototype.get dynamicallyAddedScriptElement):
Identify dynamically added script elements and associate them
with the frame, instead of the frame's main resource.

* UserInterface/Views/FrameTreeElement.js:
(WebInspector.FrameTreeElement.prototype.onpopulate):
(WebInspector.FrameTreeElement.prototype._extraScriptAdded):
Show named / source mapped dynamic script elements under a frame.

* UserInterface/Views/ResourceSidebarPanel.js:
(WebInspector.ResourceSidebarPanel.prototype._scriptWasAdded):
Dynamically added script element Scripts will be added by the frame that
owns them.

* UserInterface/Views/ScriptTreeElement.js:
(WebInspector.ScriptTreeElement):
Don't include a subtitle for dynamicallyAddedScriptElement, details match
the frame that owns them.

LayoutTests:

* inspector/model/frame-extra-scripts-expected.txt: Added.
* inspector/model/frame-extra-scripts.html: Added.
Add a test for a WebInspector.Frame's extraScripts list.

* inspector/model/script-resource-relationship-expected.txt
* inspector/model/script-resource-relationship.html
Add a test for a dynamicallyAddedScriptElement.
Remove debug logging.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@200065 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
LayoutTests/ChangeLog
LayoutTests/inspector/model/frame-extra-scripts-expected.txt [new file with mode: 0644]
LayoutTests/inspector/model/frame-extra-scripts.html [new file with mode: 0644]
LayoutTests/inspector/model/script-resource-relationship-expected.txt
LayoutTests/inspector/model/script-resource-relationship.html
LayoutTests/inspector/model/stack-trace-expected.txt
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js
Source/WebInspectorUI/UserInterface/Models/Frame.js
Source/WebInspectorUI/UserInterface/Models/Script.js
Source/WebInspectorUI/UserInterface/Views/FrameTreeElement.js
Source/WebInspectorUI/UserInterface/Views/ResourceSidebarPanel.js
Source/WebInspectorUI/UserInterface/Views/ScriptTreeElement.js

index aedf1b4..8c96b78 100644 (file)
@@ -1,3 +1,23 @@
+2016-04-25  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: React.js JSXTransformer produces bogus error locations
+        https://bugs.webkit.org/show_bug.cgi?id=150010
+        <rdar://problem/23062233>
+
+        Reviewed by Timothy Hatcher.
+
+        * inspector/model/frame-extra-scripts-expected.txt: Added.
+        * inspector/model/frame-extra-scripts.html: Added.
+        Add a test for a WebInspector.Frame's extraScripts list.
+
+        * inspector/model/stack-trace-expected.txt:
+        Update output that we correctly get the sourceURL of the inner script.
+
+        * inspector/model/script-resource-relationship-expected.txt
+        * inspector/model/script-resource-relationship.html
+        Add a test for a dynamicallyAddedScriptElement.
+        Remove debug logging.
+
 2016-04-25  Ryan Haddad  <ryanhaddad@apple.com>
 
         Skip fast/layers/no-clipping-overflow-hidden-added-after-transform.html on mac-wk1 debug
diff --git a/LayoutTests/inspector/model/frame-extra-scripts-expected.txt b/LayoutTests/inspector/model/frame-extra-scripts-expected.txt
new file mode 100644 (file)
index 0000000..3d4c30e
--- /dev/null
@@ -0,0 +1,13 @@
+CONSOLE MESSAGE: line 1: dynamically added script element
+WebInspector.Frame.extraScripts.
+
+
+== Running test suite: WebInspector.Frame.extraScripts
+-- Running test case: FrameHasNoExtraScriptsYet
+PASS: Main frame should have no dynamic scripts.
+
+-- Running test case: AddExtraScript
+PASS: ExtraScriptAdded event fired.
+PASS: Script should identify as dynamic.
+PASS: Main frame should have 1 dynamic script.
+
diff --git a/LayoutTests/inspector/model/frame-extra-scripts.html b/LayoutTests/inspector/model/frame-extra-scripts.html
new file mode 100644 (file)
index 0000000..353c640
--- /dev/null
@@ -0,0 +1,50 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
+<script>
+function triggerAddScriptElement() {
+    let script = document.createElement("script");
+    script.text = "console.log('dynamically added script element');";
+    document.body.appendChild(script);
+}
+
+function test()
+{
+    let suite = InspectorTest.createAsyncSuite("WebInspector.Frame.extraScripts");
+
+    suite.addTestCase({
+        name: "FrameHasNoExtraScriptsYet",
+        description: "No extra scripts yet.",
+        test: (resolve, reject) => {
+            let mainFrame = WebInspector.frameResourceManager.mainFrame;
+            InspectorTest.expectThat(mainFrame.extraScripts.length === 0, "Main frame should have no dynamic scripts.");
+            resolve();
+        }
+    });
+
+    suite.addTestCase({
+        name: "AddExtraScript",
+        description: "Add extra script.",
+        test: (resolve, reject) => {            
+            WebInspector.frameResourceManager.mainFrame.singleFireEventListener(WebInspector.Frame.Event.ExtraScriptAdded, (event) => {
+                InspectorTest.pass("ExtraScriptAdded event fired.");
+                InspectorTest.expectThat(event.data.script.dynamicallyAddedScriptElement, "Script should identify as dynamic.");
+            });
+
+            InspectorTest.evaluateInPage("triggerAddScriptElement()", () => {
+                let mainFrame = WebInspector.frameResourceManager.mainFrame;
+                InspectorTest.expectThat(mainFrame.extraScripts.length === 1, "Main frame should have 1 dynamic script.");
+                resolve();
+            });
+        }
+    });
+
+    suite.runTestCasesAndFinish();
+}
+</script>
+</head>
+<body onload="runTest()">
+<p>WebInspector.Frame.extraScripts.</p>
+</body>
+</html>
index 9ecdac4..ed62220 100644 (file)
@@ -25,6 +25,11 @@ PASS: Script was added.
 PASS: Script should not be associated with a Resource.
 PASS: Script should have a sourceURL.
 
+-- Running test case: DynamicallyAddedScriptElementNoResource
+PASS: Script was added.
+PASS: Script should identify as a dynamically added script element.
+PASS: Script should not be associated when a resource.
+
 -- Running test case: DocumentWithInlineScripts
 PASS: Main Resource should have 4 scripts.
 PASS: Inline script 1 does not have a sourceURL.
index bf12898..c3ffa65 100644 (file)
@@ -26,13 +26,16 @@ function triggerScriptWithoutResource() {
     eval("//# sourceURL=script-only.js\n1+1");
 }
 
+function triggerScriptElement() {
+    let script = document.createElement("script");
+    script.text = "//# sourceURL=dynamically-added-script-element.js\n1+1";
+    document.body.appendChild(script);
+}
+
 function test()
 {
     let suite = InspectorTest.createAsyncSuite("WebInspector.Script and WebInspector.Resource relationship");
 
-    InspectorTest.dumpActivityToSystemConsole = true;
-    InspectorBackend.dumpInspectorProtocolMessages = true;
-
     function validateNormalRelationship(resource, script) {
         InspectorTest.expectThat(resource.url === script.url, "Resource and Script have the same URL.");
         InspectorTest.expectThat(resource.scripts.length === 1, "Resource should be related to one script.");
@@ -133,6 +136,29 @@ function test()
     });
 
     suite.addTestCase({
+        name: "DynamicallyAddedScriptElementNoResource",
+        description: "A dynamically added script element has no resource.",
+        test: (resolve, reject) => {
+            WebInspector.debuggerManager.addEventListener(WebInspector.DebuggerManager.Event.ScriptAdded, scriptListener);
+
+            function scriptListener(event) {
+                if (event.data.script.sourceURL !== "dynamically-added-script-element.js")
+                    return;
+                InspectorTest.pass("Script was added.");
+                let script = event.data.script;
+
+                InspectorTest.expectThat(script.dynamicallyAddedScriptElement, "Script should identify as a dynamically added script element.");
+                InspectorTest.expectThat(script.resource === null, "Script should not be associated when a resource.");
+
+                WebInspector.debuggerManager.removeEventListener(WebInspector.DebuggerManager.Event.ScriptAdded, scriptListener, null);
+                resolve();
+            }
+
+            InspectorTest.evaluateInPage("triggerScriptElement()");
+        }
+    });
+
+    suite.addTestCase({
         name: "DocumentWithInlineScripts",
         description: "A document resource may be associated with multiple inline scripts.",
         test: (resolve, reject) => {
index 14a7da2..0816e9d 100644 (file)
@@ -12,7 +12,7 @@ StackTrace: 11
   1: foo (Anonymous Script 1 (line 1)) - nativeCode (false) programCode (false)
   2: Eval Code (Anonymous Script 1 (line 1)) - nativeCode (false) programCode (true)
   3: eval - nativeCode (true) programCode (false)
-  4: Global Code (stack-trace.html:2) - nativeCode (false) programCode (true)
+  4: Global Code (inline-script.js:2) - nativeCode (false) programCode (true)
   5: appendChild - nativeCode (true) programCode (false)
   6: triggerConsoleMessage (stack-trace.html:9) - nativeCode (false) programCode (false)
   7: Eval Code (Anonymous Script 2 (line 1)) - nativeCode (false) programCode (true)
index e509fac..d1b2f11 100644 (file)
@@ -1,5 +1,48 @@
 2016-04-25  Joseph Pecoraro  <pecoraro@apple.com>
 
+        Web Inspector: React.js JSXTransformer produces bogus error locations
+        https://bugs.webkit.org/show_bug.cgi?id=150010
+        <rdar://problem/23062233>
+
+        Reviewed by Timothy Hatcher.
+
+        Show dynamically added <script> elements added to a frame as resources.
+        For cases where the scripts had source map resources or error messages
+        we have a root from which to associate them to.
+
+        * Localizations/en.lproj/localizedStrings.js:
+        "Script Element %d" tree element title.
+
+        * UserInterface/Models/Frame.js:
+        (WebInspector.Frame.prototype.commitProvisionalLoad):
+        (WebInspector.Frame.prototype.get extraScripts):
+        (WebInspector.Frame.prototype.addExtraScript):
+        Have a frame keep a list of its extra scripts.
+
+        * UserInterface/Models/Script.js:
+        (WebInspector.Script):
+        (WebInspector.Script.prototype.get displayName):
+        (WebInspector.Script.prototype.get dynamicallyAddedScriptElement):
+        Identify dynamically added script elements and associate them
+        with the frame, instead of the frame's main resource.
+
+        * UserInterface/Views/FrameTreeElement.js:
+        (WebInspector.FrameTreeElement.prototype.onpopulate):
+        (WebInspector.FrameTreeElement.prototype._extraScriptAdded):
+        Show named / source mapped dynamic script elements under a frame.
+
+        * UserInterface/Views/ResourceSidebarPanel.js:
+        (WebInspector.ResourceSidebarPanel.prototype._scriptWasAdded):
+        Dynamically added script element Scripts will be added by the frame that
+        owns them.
+
+        * UserInterface/Views/ScriptTreeElement.js:
+        (WebInspector.ScriptTreeElement):
+        Don't include a subtitle for dynamicallyAddedScriptElement, details match
+        the frame that owns them.
+
+2016-04-25  Joseph Pecoraro  <pecoraro@apple.com>
+
         Web Inspector: Line error widget showed in the wrong resource
         https://bugs.webkit.org/show_bug.cgi?id=150009
         <rdar://problem/23062199>
index 031ca75..5a967c6 100644 (file)
Binary files a/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js and b/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js differ
index 321a216..181b37e 100644 (file)
@@ -38,6 +38,7 @@ WebInspector.Frame = class Frame extends WebInspector.Object
 
         this._resourceCollection = new WebInspector.ResourceCollection;
         this._provisionalResourceCollection = new WebInspector.ResourceCollection;
+        this._extraScripts = [];
 
         this._childFrames = [];
         this._childFrameIdentifierMap = {};
@@ -127,6 +128,7 @@ WebInspector.Frame = class Frame extends WebInspector.Object
 
         this._resourceCollection = this._provisionalResourceCollection;
         this._provisionalResourceCollection = new WebInspector.ResourceCollection;
+        this._extraScripts = [];
 
         this.clearExecutionContexts(true);
         this.clearProvisionalLoad(true);
@@ -437,6 +439,18 @@ WebInspector.Frame = class Frame extends WebInspector.Object
         this.dispatchEventToListeners(WebInspector.Frame.Event.AllResourcesRemoved);
     }
 
+    get extraScripts()
+    {
+        return this._extraScripts;
+    }
+
+    addExtraScript(script)
+    {
+        this._extraScripts.push(script);
+
+        this.dispatchEventToListeners(WebInspector.Frame.Event.ExtraScriptAdded, {script});
+    }
+
     saveIdentityToCookie(cookie)
     {
         cookie[WebInspector.Frame.MainResourceURLCookieKey] = this.mainResource.url.hash;
@@ -495,6 +509,7 @@ WebInspector.Frame.Event = {
     ResourceWasAdded: "frame-resource-was-added",
     ResourceWasRemoved: "frame-resource-was-removed",
     AllResourcesRemoved: "frame-all-resources-removed",
+    ExtraScriptAdded: "frame-extra-script-added",
     ChildFrameWasAdded: "frame-child-frame-was-added",
     ChildFrameWasRemoved: "frame-child-frame-was-removed",
     AllChildFramesRemoved: "frame-all-child-frames-removed",
index 777a3ad..9dcd565 100644 (file)
@@ -36,21 +36,33 @@ WebInspector.Script = class Script extends WebInspector.SourceCode
         this._range = range || null;
         this._url = url || null;
         this._sourceURL = sourceURL || null;
+        this._sourceMappingURL = sourceMapURL || null;
         this._injected = injected || false;
+        this._dynamicallyAddedScriptElement = false;
+        this._scriptSyntaxTree = null;
 
         this._resource = this._resolveResource();
-        if (this._resource)
-            this._resource.associateWithScript(this);
 
-        if (sourceMapURL)
-            WebInspector.sourceMapManager.downloadSourceMap(sourceMapURL, this._url, this);
+        // If this Script was a dynamically added <script> to a Document,
+        // do not associate with the Document resource, instead associate
+        // with the frame as a dynamic script.
+        if (this._resource && this._resource.type === WebInspector.Resource.Type.Document && !this._range.startLine && !this._range.startColumn) {
+            console.assert(this._resource.isMainResource());
+            let documentResource = this._resource;
+            this._resource = null;
+            this._dynamicallyAddedScriptElement = true;
+            documentResource.parentFrame.addExtraScript(this);
+            this._dynamicallyAddedScriptElementNumber = documentResource.parentFrame.extraScripts.length;
+        } else if (this._resource)
+            this._resource.associateWithScript(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;
+        if (this._sourceMappingURL)
+            WebInspector.sourceMapManager.downloadSourceMap(this._sourceMappingURL, this._url, this);
     }
 
     // Static
@@ -83,6 +95,11 @@ WebInspector.Script = class Script extends WebInspector.SourceCode
         return this._sourceURL;
     }
 
+    get sourceMappingURL()
+    {
+        return this._sourceMappingURL;
+    }
+
     get urlComponents()
     {
         if (!this._urlComponents)
@@ -97,7 +114,7 @@ WebInspector.Script = class Script extends WebInspector.SourceCode
 
     get displayName()
     {
-        if (this._url)
+        if (this._url && !this._dynamicallyAddedScriptElement)
             return WebInspector.displayNameForURL(this._url, this.urlComponents);
 
         if (isWebInspectorConsoleEvaluationScript(this._sourceURL)) {
@@ -111,6 +128,9 @@ WebInspector.Script = class Script extends WebInspector.SourceCode
             return WebInspector.displayNameForURL(this._sourceURL, this._sourceURLComponents);
         }
 
+        if (this._dynamicallyAddedScriptElement)
+            return WebInspector.UIString("Script Element %d").format(this._dynamicallyAddedScriptElementNumber);
+
         // Assign a unique number to the script object so it will stay the same.
         if (!this._uniqueDisplayNameNumber)
             this._uniqueDisplayNameNumber = this.constructor._nextUniqueDisplayNameNumber++;
@@ -135,6 +155,11 @@ WebInspector.Script = class Script extends WebInspector.SourceCode
         return this._injected;
     }
 
+    get dynamicallyAddedScriptElement()
+    {
+        return this._dynamicallyAddedScriptElement;
+    }
+
     get anonymous()
     {
         return !this._resource && !this._url && !this._sourceURL;
index af3e3ae..f0a39ad 100644 (file)
@@ -38,6 +38,7 @@ WebInspector.FrameTreeElement = class FrameTreeElement extends WebInspector.Reso
         frame.addEventListener(WebInspector.Frame.Event.MainResourceDidChange, this._mainResourceDidChange, this);
         frame.addEventListener(WebInspector.Frame.Event.ResourceWasAdded, this._resourceWasAdded, this);
         frame.addEventListener(WebInspector.Frame.Event.ResourceWasRemoved, this._resourceWasRemoved, this);
+        frame.addEventListener(WebInspector.Frame.Event.ExtraScriptAdded, this._extraScriptAdded, this);
         frame.addEventListener(WebInspector.Frame.Event.ChildFrameWasAdded, this._childFrameWasAdded, this);
         frame.addEventListener(WebInspector.Frame.Event.ChildFrameWasRemoved, this._childFrameWasRemoved, this);
 
@@ -52,17 +53,23 @@ WebInspector.FrameTreeElement = class FrameTreeElement extends WebInspector.Reso
         this.folderSettingsKey = this._frame.url.hash;
 
         this.registerFolderizeSettings("frames", WebInspector.UIString("Frames"),
-            function(representedObject) { return representedObject instanceof WebInspector.Frame; },
-            function() { return this.frame.childFrames.length; }.bind(this),
+            (representedObject) => representedObject instanceof WebInspector.Frame,
+            () => this.frame.childFrames.length,
             WebInspector.FrameTreeElement
         );
 
         this.registerFolderizeSettings("flows", WebInspector.UIString("Flows"),
-            function(representedObject) { return representedObject instanceof WebInspector.ContentFlow; },
-            function() { return this.frame.domTree.flowsCount; }.bind(this),
+            (representedObject) => representedObject instanceof WebInspector.ContentFlow,
+            () => this.frame.domTree.flowsCount,
             WebInspector.ContentFlowTreeElement
         );
 
+        this.registerFolderizeSettings("extra-scripts", WebInspector.UIString("Extra Scripts"),
+            (representedObject) => representedObject instanceof WebInspector.Script && representedObject.dynamicallyAddedScriptElement,
+            () => this.frame.extraScripts.length,
+            WebInspector.ScriptTreeElement
+        );
+
         function makeValidateCallback(resourceType) {
             return function(representedObject) {
                 return representedObject instanceof WebInspector.Resource && representedObject.type === resourceType;
@@ -189,6 +196,10 @@ WebInspector.FrameTreeElement = class FrameTreeElement extends WebInspector.Reso
         for (var flowKey in flowMap)
             this.addChildForRepresentedObject(flowMap[flowKey]);
 
+        for (let extraScript of this._frame.extraScripts) {
+            if (extraScript.sourceURL || extraScript.sourceMappingURL)
+                this.addChildForRepresentedObject(extraScript);
+        }
     }
 
     onexpand()
@@ -240,6 +251,13 @@ WebInspector.FrameTreeElement = class FrameTreeElement extends WebInspector.Reso
         this.removeChildForRepresentedObject(event.data.resource);
     }
 
+    _extraScriptAdded(event)
+    {
+        let extraScript = event.data.script;
+        if (extraScript.sourceURL || extraScript.sourceMappingURL)
+            this.addRepresentedObjectToNewChildQueue(extraScript);
+    }
+
     _childFrameWasAdded(event)
     {
         this.addRepresentedObjectToNewChildQueue(event.data.childFrame);
index 4267ad6..b8a17c7 100644 (file)
@@ -279,7 +279,7 @@ WebInspector.ResourceSidebarPanel = class ResourceSidebarPanel extends WebInspec
             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)
+        if (script.resource || script.dynamicallyAddedScriptElement)
             return;
 
         var insertIntoTopLevel = false;
index b83d260..6494c3c 100644 (file)
@@ -33,7 +33,7 @@ WebInspector.ScriptTreeElement = class ScriptTreeElement extends WebInspector.So
 
         this.mainTitle = script.displayName;
 
-        if (script.url) {
+        if (script.url && !script.dynamicallyAddedScriptElement) {
             // Show the host as the subtitle if it is different from the main title.
             var subtitle = WebInspector.displayNameForHost(script.urlComponents.host);
             this.subtitle = this.mainTitle !== subtitle ? subtitle : null;