DevTools: move binding validation from persistence to automapping

This patch moves binding validation from Persistence/ into Automapping/.

The validation was used in both default mappings and automapping.
Since default mapping is gone, we can safely move it to automapping.

This consolidates all the binding-creation logic in one place,
making it easier to report and manage binding errors.

R=dgozman

Change-Id: Ie969c0b42468cfc863723311bf920510cf587560
Reviewed-on: https://chromium-review.googlesource.com/882486Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Andrey Lushnikov <lushnikov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531732}

third_party/WebKit/LayoutTests/http/tests/devtools/persistence/persistence-do-not-bind-dirty-sourcecode-expected.txt → third_party/WebKit/LayoutTests/http/tests/devtools/persistence/automapping-do-not-bind-dirty-sourcecode-expected.txt

 Verify that dirty uiSourceCodes are not bound.
 
-
-Running: waitForUISourceCodes
-
-Running: addFileSystemMapping
 Failed to create binding: {
        network: http://127.0.0.1:8000/devtools/persistence/resources/foo.js
     fileSystem: file:///var/www/devtools/persistence/resources/foo.js
-    exactMatch: false
+    exactMatch: true
 }
 

third_party/WebKit/LayoutTests/http/tests/devtools/persistence/persistence-do-not-bind-dirty-sourcecode.js → third_party/WebKit/LayoutTests/http/tests/devtools/persistence/automapping-do-not-bind-dirty-sourcecode.js

@@ -8,31 +8,18 @@
   await TestRunner.loadModule('console_test_runner');
   await TestRunner.loadModule('bindings_test_runner');
   await TestRunner.addScriptTag('resources/foo.js');
+  BindingsTestRunner.overrideNetworkModificationTime(
+      {'http://127.0.0.1:8000/devtools/persistence/resources/foo.js': null});
+
+  var networkUISourceCode = await TestRunner.waitForUISourceCode('foo.js', Workspace.projectTypes.Network);
+  var content = await networkUISourceCode.requestContent();
+  content = content.replace(/foo/g, 'bar');
+  networkUISourceCode.setWorkingCopy(content);
 
-  var testMapping = BindingsTestRunner.initializeTestMapping();
   var fs = new BindingsTestRunner.TestFileSystem('file:///var/www');
   BindingsTestRunner.addFooJSFile(fs);
   fs.reportCreated(function() {});
-
-  TestRunner.runTestSuite([
-    function waitForUISourceCodes(next) {
-      Promise
-          .all([
-            TestRunner.waitForUISourceCode('foo.js', Workspace.projectTypes.Network)
-                .then(uiSourceCode => uiSourceCode.setWorkingCopy('dirty.')),
-            TestRunner.waitForUISourceCode('foo.js', Workspace.projectTypes.FileSystem)
-          ])
-          .then(next);
-    },
-
-    function addFileSystemMapping(next) {
-      TestRunner.addSniffer(Persistence.Persistence.prototype, '_prevalidationFailedForTest', onPrevalidationFailed);
-      testMapping.addBinding('foo.js');
-
-      function onPrevalidationFailed(binding) {
-        TestRunner.addResult('Failed to create binding: ' + binding);
-        next();
-      }
-    },
-  ]);
+  var binding = await TestRunner.addSnifferPromise(Persistence.Automapping.prototype, '_prevalidationFailedForTest');
+  TestRunner.addResult('Failed to create binding: ' + binding);
+  TestRunner.completeTest();
 })();

third_party/WebKit/Source/devtools/front_end/bindings_test_runner/AutomappingTestRunner.js

@@ -68,7 +68,7 @@ BindingsTestRunner.AutomappingTest.prototype = {
     for (var url in assets) {
       var asset = assets[url];
       var contentType = asset.contentType || Common.resourceTypes.Script;
-      var contentProvider = new Common.StaticContentProvider(url, contentType, Promise.resolve(asset.content));
+      var contentProvider = Common.StaticContentProvider.fromString(url, contentType, asset.content);
       var metadata =
           (typeof asset.content === 'string' || asset.time ?
                new Workspace.UISourceCodeMetadata(asset.time, asset.content.length) :

third_party/WebKit/Source/devtools/front_end/bindings_test_runner/PersistenceTestRunner.js

@@ -38,7 +38,7 @@ BindingsTestRunner.addFooJSFile = function(fs) {
   return fs.root.mkdir('devtools')
       .mkdir('persistence')
       .mkdir('resources')
-      .addFile('foo.js', '\n\nwindow.foo = ()=>\'foo\';');
+      .addFile('foo.js', '\n\nwindow.foo = ()=>\'foo\';\n');
 };
 
 BindingsTestRunner.initializeTestMapping = function() {

third_party/WebKit/Source/devtools/front_end/persistence/Automapping.js

@@ -178,9 +178,45 @@ Persistence.Automapping = class {
     if (networkSourceCode[Persistence.Automapping._processingPromise] ||
         networkSourceCode[Persistence.Automapping._binding] || !this._enabled)
       return;
-    var createBindingPromise = this._createBinding(networkSourceCode).then(onBinding.bind(this));
+    var createBindingPromise =
+        this._createBinding(networkSourceCode).then(validateBinding.bind(this)).then(onBinding.bind(this));
     networkSourceCode[Persistence.Automapping._processingPromise] = createBindingPromise;
 
+    /**
+     * @param {?Persistence.PersistenceBinding} binding
+     * @return {!Promise<?Persistence.PersistenceBinding>}
+     * @this {Persistence.Automapping}
+     */
+    async function validateBinding(binding) {
+      if (!binding)
+        return null;
+      if (binding.network.contentType().isFromSourceMap() || !binding.fileSystem.contentType().isTextType())
+        return binding;
+
+      await Promise.all([binding.network.requestContent(), binding.fileSystem.requestContent()]);
+
+      if (networkSourceCode[Persistence.Automapping._processingPromise] !== createBindingPromise)
+        return null;
+
+      var fileSystemContent = binding.fileSystem.workingCopy();
+      var networkContent = binding.network.workingCopy();
+      var target = Bindings.NetworkProject.targetForUISourceCode(binding.network);
+      var isValid = false;
+      if (target && target.isNodeJS()) {
+        var rewrappedNetworkContent =
+            Persistence.Persistence.rewrapNodeJSContent(binding, binding.fileSystem, fileSystemContent, networkContent);
+        isValid = fileSystemContent === rewrappedNetworkContent;
+      } else {
+        // Trim trailing whitespaces because V8 adds trailing newline.
+        isValid = fileSystemContent.trimRight() === networkContent.trimRight();
+      }
+      if (!isValid) {
+        this._prevalidationFailedForTest(binding);
+        return null;
+      }
+      return binding;
+    }
+
     /**
      * @param {?Persistence.PersistenceBinding} binding
      * @this {Persistence.Automapping}
@@ -210,6 +246,12 @@ Persistence.Automapping = class {
     }
   }
 
+  /**
+   * @param {!Persistence.PersistenceBinding} binding
+   */
+  _prevalidationFailedForTest(binding) {
+  }
+
   _onBindingFailedForTest() {
   }
 

third_party/WebKit/Source/devtools/front_end/persistence/Persistence.js

@@ -23,7 +23,7 @@ Persistence.Persistence = class extends Common.Object {
     var linkDecorator = new Persistence.PersistenceUtils.LinkDecorator(this);
     Components.Linkifier.setLinkDecorator(linkDecorator);
     this._mapping =
-        new Persistence.Automapping(workspace, this._validateBinding.bind(this), this._onBindingRemoved.bind(this));
+        new Persistence.Automapping(workspace, this._establishBinding.bind(this), this._onBindingRemoved.bind(this));
   }
 
   /**
@@ -52,51 +52,7 @@ Persistence.Persistence = class extends Common.Object {
    */
   _setMappingForTest(mappingFactory) {
     this._mapping.dispose();
-    this._mapping = mappingFactory(this._validateBinding.bind(this), this._onBindingRemoved.bind(this));
-  }
-
-  /**
-   * @param {!Persistence.PersistenceBinding} binding
-   */
-  _validateBinding(binding) {
-    if (binding.network.contentType().isFromSourceMap() || !binding.fileSystem.contentType().isTextType()) {
-      this._establishBinding(binding);
-      return;
-    }
-
-    Promise.all([binding.network.requestContent(), binding.fileSystem.requestContent()]).then(onContents.bind(this));
-
-    /**
-     * @this {Persistence.Persistence}
-     */
-    function onContents() {
-      if (binding._removed)
-        return;
-
-      var fileSystemContent = binding.fileSystem.workingCopy();
-      var networkContent = binding.network.workingCopy();
-      var target = Bindings.NetworkProject.targetForUISourceCode(binding.network);
-      var isValid = false;
-      if (target.isNodeJS()) {
-        var rewrappedNetworkContent = Persistence.Persistence._rewrapNodeJSContent(
-            binding, binding.fileSystem, fileSystemContent, networkContent);
-        isValid = (fileSystemContent === rewrappedNetworkContent);
-      } else {
-        // Trim trailing whitespaces because V8 adds trailing newline.
-        isValid = (fileSystemContent.trimRight() === networkContent.trimRight());
-      }
-
-      if (isValid)
-        this._establishBinding(binding);
-      else
-        this._prevalidationFailedForTest(binding);
-    }
-  }
-
-  /**
-   * @param {!Persistence.PersistenceBinding} binding
-   */
-  _prevalidationFailedForTest(binding) {
+    this._mapping = mappingFactory(this._establishBinding.bind(this), this._onBindingRemoved.bind(this));
   }
 
   /**
@@ -186,7 +142,7 @@ Persistence.Persistence = class extends Common.Object {
       var newContent = uiSourceCode.workingCopy();
       other.requestContent().then(() => {
         var nodeJSContent =
-            Persistence.Persistence._rewrapNodeJSContent(binding, other, other.workingCopy(), newContent);
+            Persistence.Persistence.rewrapNodeJSContent(binding, other, other.workingCopy(), newContent);
         setWorkingCopy.call(this, () => nodeJSContent);
       });
       return;
@@ -227,7 +183,7 @@ Persistence.Persistence = class extends Common.Object {
     var target = Bindings.NetworkProject.targetForUISourceCode(binding.network);
     if (target.isNodeJS()) {
       other.requestContent().then(currentContent => {
-        var nodeJSContent = Persistence.Persistence._rewrapNodeJSContent(binding, other, currentContent, newContent);
+        var nodeJSContent = Persistence.Persistence.rewrapNodeJSContent(binding, other, currentContent, newContent);
         setContent.call(this, nodeJSContent);
       });
       return;
@@ -253,7 +209,7 @@ Persistence.Persistence = class extends Common.Object {
    * @param {string} newContent
    * @return {string}
    */
-  static _rewrapNodeJSContent(binding, uiSourceCode, currentContent, newContent) {
+  static rewrapNodeJSContent(binding, uiSourceCode, currentContent, newContent) {
     if (uiSourceCode === binding.fileSystem) {
       if (newContent.startsWith(Persistence.Persistence._NodePrefix) &&
           newContent.endsWith(Persistence.Persistence._NodeSuffix)) {