DevTools: teach UISourceCodeFrame to merge messages

This patch teaches UISourceCodeFrame to display messages from both UISourceCodes of PersistenceBinding. This is reasonable, since there might be different "builders" which add messages to either of the source codes. This patch also slightly modifies the handling of decorations: in case of persistence binding, we prefer network uiSourceCode decorations over the fileSystem ones: it's hard to merge decorations. This is also an imperative step towards displaying fileSystem file by default in the editor. BUG=649837 R=dgozman Review-Url: https://codereview.chromium.org/2530483002 Cr-Commit-Position: refs/heads/master@{#434847}

DevTools: teach UISourceCodeFrame to merge messages
This patch teaches UISourceCodeFrame to display messages from both UISourceCodes of PersistenceBinding. This is reasonable, since there might be different "builders" which add messages to either of the source codes. This patch also slightly modifies the handling of decorations: in case of persistence binding, we prefer network uiSourceCode decorations over the fileSystem ones: it's hard to merge decorations. This is also an imperative step towards displaying fileSystem file by default in the editor. BUG=649837 R=dgozman Review-Url: https://codereview.chromium.org/2530483002 Cr-Commit-Position: refs/heads/master@{#434847}
0496be27 · lushnikov · Commit bot · 7d224832 · 0496be27 · 0496be27
Commit 0496be27 authored Nov 28, 2016 by lushnikov Committed by Commit bot Nov 29, 2016
6 changed files
--- a/third_party/WebKit/LayoutTests/http/tests/inspector/debugger-test.js
+++ b/third_party/WebKit/LayoutTests/http/tests/inspector/debugger-test.js
@@ -134,6 +134,20 @@ InspectorTest.runAsyncCallStacksTest = function(totalDebuggerStatements, maxAsyn
    }
 };
+InspectorTest.dumpSourceFrameMessages = function(sourceFrame, dumpFullURL)
+{
+    var messages = [];
+    for (var bucket of sourceFrame._rowMessageBuckets.values()) {
+        for (var rowMessage of bucket._messages) {
+            var message = rowMessage.message();
+            messages.push(String.sprintf("  %d:%d [%s] %s", message.lineNumber(), message.columnNumber(), message.level(), message.text()));
+        }
+    }
+    var name = dumpFullURL ? sourceFrame.uiSourceCode().url() : sourceFrame.uiSourceCode().displayName();
+    InspectorTest.addResult("SourceFrame " + name + ": " + messages.length + " message(s)");
+    InspectorTest.addResult(messages.join("\n"));
+}
 InspectorTest.waitUntilPausedNextTime = function(callback)
 {
    InspectorTest._waitUntilPausedCallback = InspectorTest.safeWrap(callback);

--- a/third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-sourceframe-messages-expected.txt
+++ b/third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-sourceframe-messages-expected.txt
+Verify that messages are synced in UISourceCodeFrame between UISourceCodes of persistence binding.
+Running: waitForUISourceCodes
+Running: addMessages
+SourceFrame file:///var/www/inspector/persistence/resources/foo.js: 1 message(s)
+  0:0 [Error] error in filesystem
+SourceFrame http://127.0.0.1:8000/inspector/persistence/resources/foo.js: 1 message(s)
+  1:0 [Warning] warning in network
+Running: addMapping
+SourceFrame file:///var/www/inspector/persistence/resources/foo.js: 2 message(s)
+  1:0 [Warning] warning in network
+  0:0 [Error] error in filesystem
+SourceFrame http://127.0.0.1:8000/inspector/persistence/resources/foo.js: 2 message(s)
+  1:0 [Warning] warning in network
+  0:0 [Error] error in filesystem
+Running: removeMapping
+SourceFrame file:///var/www/inspector/persistence/resources/foo.js: 1 message(s)
+  0:0 [Error] error in filesystem
+SourceFrame http://127.0.0.1:8000/inspector/persistence/resources/foo.js: 1 message(s)
+  1:0 [Warning] warning in network
--- a/third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-sourceframe-messages.html
+++ b/third_party/WebKit/LayoutTests/http/tests/inspector/persistence/persistence-sourceframe-messages.html
+<html>
+<head>
+<script src="../inspector-test.js"></script>
+<script src="../debugger-test.js"></script>
+<script src="../workspace-test.js"></script>
+<script src="../isolated-filesystem-test.js"></script>
+<script src="./persistence-test.js"></script>
+<script src="./resources/foo.js"></script>
+<script>
+function test()
+{
+    var fs = new InspectorTest.TestFileSystem("file:///var/www");
+    InspectorTest.addFooJSFile(fs);
+    var networkSourceCode;
+    var fileSystemSourceCode;
+    var fileSystemSourceFrame, networkSourceFrame;
+    InspectorTest.runTestSuite([
+        function waitForUISourceCodes(next)
+        {
+            fs.reportCreated(function() { });
+            Promise.all([
+                InspectorTest.waitForUISourceCode("foo.js", Workspace.projectTypes.FileSystem)
+                    .then(sourceCode => fileSystemSourceCode = sourceCode),
+                InspectorTest.waitForUISourceCode("foo.js", Workspace.projectTypes.Network)
+                    .then(sourceCode => networkSourceCode = sourceCode),
+            ]).then(next);
+        },
+        function addMessages(next)
+        {
+            fileSystemSourceCode.addLineMessage(Workspace.UISourceCode.Message.Level.Error, 'error in filesystem', 0, 0);
+            networkSourceCode.addLineMessage(Workspace.UISourceCode.Message.Level.Warning, 'warning in network', 1, 0);
+            Promise.all([
+                InspectorTest.showUISourceCodePromise(fileSystemSourceCode),
+                InspectorTest.showUISourceCodePromise(networkSourceCode)
+            ]).then(onSourceFrames);
+            function onSourceFrames(sourceFrames)
+            {
+                fileSystemSourceFrame = sourceFrames[0];
+                networkSourceFrame = sourceFrames[1];
+                InspectorTest.dumpSourceFrameMessages(fileSystemSourceFrame, /* dumpFullURL */ true);
+                InspectorTest.dumpSourceFrameMessages(networkSourceFrame, /* dumpFullURL */ true);
+                next();
+            }
+        },
+        function addMapping(next)
+        {
+            InspectorTest.waitForBinding("foo.js").then(onBindingCreated);
+            Workspace.fileSystemMapping.addFileMapping(fs.fileSystemPath, "http://127.0.0.1:8000", "/");
+            function onBindingCreated(binding)
+            {
+                InspectorTest.dumpSourceFrameMessages(fileSystemSourceFrame, /* dumpFullURL */ true);
+                InspectorTest.dumpSourceFrameMessages(networkSourceFrame, /* dumpFullURL */ true);
+                next();
+            }
+        },
+        function removeMapping(next)
+        {
+            Persistence.persistence.addEventListener(Persistence.Persistence.Events.BindingRemoved, onBindingRemoved);
+            Workspace.fileSystemMapping.removeFileMapping(fs.fileSystemPath, "http://127.0.0.1:8000", "/");
+            function onBindingRemoved(event)
+            {
+                var binding = event.data;
+                if (binding.network.name() !== "foo.js")
+                    return
+                Persistence.persistence.removeEventListener(Persistence.Persistence.Events.BindingRemoved, onBindingRemoved);
+                InspectorTest.dumpSourceFrameMessages(fileSystemSourceFrame, /* dumpFullURL */ true);
+                InspectorTest.dumpSourceFrameMessages(networkSourceFrame, /* dumpFullURL */ true);
+                next();
+            }
+        },
+    ]);
+};
+</script>
+</head>
+<body onload="runTest()">
+<p>Verify that messages are synced in UISourceCodeFrame between UISourceCodes of persistence binding.</p>
+</body>
+</html>
--- a/third_party/WebKit/LayoutTests/inspector/sources/compile-javascript-expected.txt
+++ b/third_party/WebKit/LayoutTests/inspector/sources/compile-javascript-expected.txt
 Verifies proactive javascript compilation.
-0:13 [Error] Uncaught SyntaxError: Unexpected identifier
+SourceFrame edit-me.js: 1 message(s)
+  0:13 [Error] Uncaught SyntaxError: Unexpected identifier
--- a/third_party/WebKit/LayoutTests/inspector/sources/compile-javascript.html
+++ b/third_party/WebKit/LayoutTests/inspector/sources/compile-javascript.html
@@ -19,12 +19,7 @@ function test()
    function onCompilationFinished(sourceFrame)
    {
-        for (var bucket of sourceFrame._rowMessageBuckets.values()) {
+        InspectorTest.dumpSourceFrameMessages(sourceFrame);
-            for (var rowMessage of bucket._messages) {
-                var message = rowMessage.message();
-                InspectorTest.addResult(String.sprintf("%d:%d [%s] %s", message.lineNumber(), message.columnNumber(), message.level(), message.text()));
-            }
-        }
        InspectorTest.completeTest();
    }
 }

--- a/third_party/WebKit/Source/devtools/front_end/sources/UISourceCodeFrame.js
+++ b/third_party/WebKit/Source/devtools/front_end/sources/UISourceCodeFrame.js
@@ -46,6 +46,9 @@ Sources.UISourceCodeFrame = class extends SourceFrame.SourceFrame {
    Common.moduleSetting('textEditorAutocompletion').addChangeListener(this._updateAutocomplete, this);
    this._updateAutocomplete();
+    /** @type {?Persistence.PersistenceBinding} */
+    this._persistenceBinding = Persistence.persistence.binding(uiSourceCode);
    /** @type {!Map<number, !Sources.UISourceCodeFrame.RowMessageBucket>} */
    this._rowMessageBuckets = new Map();
    /** @type {!Set<string>} */
@@ -54,12 +57,10 @@ Sources.UISourceCodeFrame = class extends SourceFrame.SourceFrame {
        Workspace.UISourceCode.Events.WorkingCopyChanged, this._onWorkingCopyChanged, this);
    this._uiSourceCode.addEventListener(
        Workspace.UISourceCode.Events.WorkingCopyCommitted, this._onWorkingCopyCommitted, this);
-    this._uiSourceCode.addEventListener(Workspace.UISourceCode.Events.MessageAdded, this._onMessageAdded, this);
-    this._uiSourceCode.addEventListener(Workspace.UISourceCode.Events.MessageRemoved, this._onMessageRemoved, this);
+    this._messageAndDecorationListeners = [];
-    this._uiSourceCode.addEventListener(
+    this._installMessageAndDecorationListeners();
-        Workspace.UISourceCode.Events.LineDecorationAdded, this._onLineDecorationAdded, this);
-    this._uiSourceCode.addEventListener(
-        Workspace.UISourceCode.Events.LineDecorationRemoved, this._onLineDecorationRemoved, this);
    Persistence.persistence.addEventListener(
        Persistence.Persistence.Events.BindingCreated, this._onBindingChanged, this);
    Persistence.persistence.addEventListener(
@@ -89,6 +90,34 @@ Sources.UISourceCodeFrame = class extends SourceFrame.SourceFrame {
    }
  }
+  _installMessageAndDecorationListeners() {
+    if (this._persistenceBinding) {
+      var networkSourceCode = this._persistenceBinding.network;
+      var fileSystemSourceCode = this._persistenceBinding.fileSystem;
+      this._messageAndDecorationListeners = [
+        networkSourceCode.addEventListener(Workspace.UISourceCode.Events.MessageAdded, this._onMessageAdded, this),
+        networkSourceCode.addEventListener(Workspace.UISourceCode.Events.MessageRemoved, this._onMessageRemoved, this),
+        networkSourceCode.addEventListener(
+            Workspace.UISourceCode.Events.LineDecorationAdded, this._onLineDecorationAdded, this),
+        networkSourceCode.addEventListener(
+            Workspace.UISourceCode.Events.LineDecorationRemoved, this._onLineDecorationRemoved, this),
+        fileSystemSourceCode.addEventListener(Workspace.UISourceCode.Events.MessageAdded, this._onMessageAdded, this),
+        fileSystemSourceCode.addEventListener(
+            Workspace.UISourceCode.Events.MessageRemoved, this._onMessageRemoved, this),
+      ];
+    } else {
+      this._messageAndDecorationListeners = [
+        this._uiSourceCode.addEventListener(Workspace.UISourceCode.Events.MessageAdded, this._onMessageAdded, this),
+        this._uiSourceCode.addEventListener(Workspace.UISourceCode.Events.MessageRemoved, this._onMessageRemoved, this),
+        this._uiSourceCode.addEventListener(
+            Workspace.UISourceCode.Events.LineDecorationAdded, this._onLineDecorationAdded, this),
+        this._uiSourceCode.addEventListener(
+            Workspace.UISourceCode.Events.LineDecorationRemoved, this._onLineDecorationRemoved, this)
+      ];
+    }
+  }
  /**
   * @return {!Workspace.UISourceCode}
   */
@@ -161,11 +190,20 @@ Sources.UISourceCodeFrame = class extends SourceFrame.SourceFrame {
    if (this._diff)
      this._diff.updateDiffMarkersImmediately();
    super.onTextEditorContentSet();
-    for (var message of this._uiSourceCode.messages())
+    for (var message of this._allMessages())
      this._addMessageToSource(message);
    this._decorateAllTypes();
  }
+  /**
+   * @return {!Array<!Workspace.UISourceCode.Message>}
+   */
+  _allMessages() {
+    return this._persistenceBinding ?
+        this._persistenceBinding.network.messages().concat(this._persistenceBinding.fileSystem.messages()) :
+        this._uiSourceCode.messages();
+  }
  /**
   * @override
   * @param {!Common.TextRange} oldRange
@@ -208,13 +246,21 @@ Sources.UISourceCodeFrame = class extends SourceFrame.SourceFrame {
    this._updateStyle();
  }
-  /**
+  _onBindingChanged() {
-   * @param {!Common.Event} event
+    var binding = Persistence.persistence.binding(this._uiSourceCode);
-   */
+    if (binding === this._persistenceBinding)
-  _onBindingChanged(event) {
+      return;
-    var binding = /** @type {!Persistence.PersistenceBinding} */ (event.data);
+    for (var message of this._allMessages())
-    if (binding.network === this._uiSourceCode || binding.fileSystem === this._uiSourceCode)
+      this._removeMessageFromSource(message);
-      this._updateStyle();
+    Common.EventTarget.removeEventListeners(this._messageAndDecorationListeners);
+    this._persistenceBinding = binding;
+    for (var message of this._allMessages())
+      this._addMessageToSource(message);
+    this._installMessageAndDecorationListeners();
+    this._updateStyle();
+    this._decorateAllTypes();
  }
  _updateStyle() {
@@ -296,8 +342,6 @@ Sources.UISourceCodeFrame = class extends SourceFrame.SourceFrame {
   * @param {!Common.Event} event
   */
  _onMessageAdded(event) {
-    if (!this.loaded)
-      return;
    var message = /** @type {!Workspace.UISourceCode.Message} */ (event.data);
    this._addMessageToSource(message);
  }
@@ -306,6 +350,8 @@ Sources.UISourceCodeFrame = class extends SourceFrame.SourceFrame {
   * @param {!Workspace.UISourceCode.Message} message
   */
  _addMessageToSource(message) {
+    if (!this.loaded)
+      return;
    var lineNumber = message.lineNumber();
    if (lineNumber >= this._textEditor.linesCount)
      lineNumber = this._textEditor.linesCount - 1;
@@ -324,8 +370,6 @@ Sources.UISourceCodeFrame = class extends SourceFrame.SourceFrame {
   * @param {!Common.Event} event
   */
  _onMessageRemoved(event) {
-    if (!this.loaded)
-      return;
    var message = /** @type {!Workspace.UISourceCode.Message} */ (event.data);
    this._removeMessageFromSource(message);
  }
@@ -334,6 +378,9 @@ Sources.UISourceCodeFrame = class extends SourceFrame.SourceFrame {
   * @param {!Workspace.UISourceCode.Message} message
   */
  _removeMessageFromSource(message) {
+    if (!this.loaded)
+      return;
    var lineNumber = message.lineNumber();
    if (lineNumber >= this._textEditor.linesCount)
      lineNumber = this._textEditor.linesCount - 1;
@@ -409,7 +456,8 @@ Sources.UISourceCodeFrame = class extends SourceFrame.SourceFrame {
        .instance()
        .then(decorator => {
          this._typeDecorationsPending.delete(type);
-          decorator.decorate(this.uiSourceCode(), this._textEditor);
+          decorator.decorate(
+              this._persistenceBinding ? this._persistenceBinding.network : this.uiSourceCode(), this._textEditor);
        });
  }