[DevTools] simplify script with source map blackboxing

Current logic produces one Debugger.setBlackboxedRanges call on each
scriptParsed event. It makes our frontend too much chatty.
This CL simplify blackbox manager. With new logic blackbox manager
sends Debugger.setBlackboxedRanges iff script has sourceMap and at
least one sourceURL from sourceMap is blackboxed.
When blackbox pattern changed frontend will send setBlackboxedRanges
iff ranges are affected by change.

R=lushnikov@chromium.org

Change-Id: I8b9511f9feba50127de4add98c8d191877103145
Reviewed-on: https://chromium-review.googlesource.com/1162308
Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Reviewed-by: Andrey Lushnikov <lushnikov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580742}

third_party/blink/renderer/devtools/front_end/bindings/BlackboxManager.js

@@ -13,17 +13,13 @@ Bindings.BlackboxManager = class {
     this._debuggerWorkspaceBinding = debuggerWorkspaceBinding;
 
     SDK.targetManager.addModelListener(
-        SDK.DebuggerModel, SDK.DebuggerModel.Events.ParsedScriptSource, this._parsedScriptSource, this);
-    SDK.targetManager.addModelListener(
-        SDK.DebuggerModel, SDK.DebuggerModel.Events.GlobalObjectCleared, this._globalObjectCleared, this);
+        SDK.DebuggerModel, SDK.DebuggerModel.Events.GlobalObjectCleared, this._clearCacheIfNeeded.bind(this), this);
     Common.moduleSetting('skipStackFramesPattern').addChangeListener(this._patternChanged.bind(this));
     Common.moduleSetting('skipContentScripts').addChangeListener(this._patternChanged.bind(this));
 
     /** @type {!Set<function()>} */
     this._listeners = new Set();
 
-    /** @type {!Map<!SDK.DebuggerModel, !Map<string, !Array<!Protocol.Debugger.ScriptPosition>>>} */
-    this._debuggerModelData = new Map();
     /** @type {!Map<string, boolean>} */
     this._isBlackboxedURLCache = new Map();
 
@@ -57,7 +53,11 @@ Bindings.BlackboxManager = class {
    * @param {!SDK.DebuggerModel} debuggerModel
    */
   modelRemoved(debuggerModel) {
-    this._debuggerModelData.delete(debuggerModel);
+    this._clearCacheIfNeeded();
+  }
+
+  _clearCacheIfNeeded() {
+    if (this._isBlackboxedURLCache.size > 1024)
       this._isBlackboxedURLCache.clear();
   }
 
@@ -83,10 +83,10 @@ Bindings.BlackboxManager = class {
     const script = location.script();
     if (!script)
       return false;
-    const positions = this._scriptPositions(script);
-    if (!positions)
-      return this._isBlackboxedScript(script);
-    const index = positions.lowerBound(location, comparator);
+    const ranges = script[Bindings.BlackboxManager._blackboxedRanges];
+    if (!ranges)
+      return this.isBlackboxedURL(script.sourceURL, script.isContentScript());
+    const index = ranges.lowerBound(location, comparator);
     return !!(index % 2);
 
     /**
@@ -125,7 +125,7 @@ Bindings.BlackboxManager = class {
     if (isContentScript && Common.moduleSetting('skipContentScripts').get())
       return true;
     const regex = Common.moduleSetting('skipStackFramesPattern').asRegExp();
-    const isBlackboxed = regex && regex.test(url);
+    const isBlackboxed = (regex && regex.test(url)) || false;
     this._isBlackboxedURLCache.set(url, isBlackboxed);
     return isBlackboxed;
   }
@@ -135,47 +135,45 @@ Bindings.BlackboxManager = class {
    * @param {?SDK.SourceMap} sourceMap
    * @return {!Promise<undefined>}
    */
-  sourceMapLoaded(script, sourceMap) {
-    if (!sourceMap)
-      return Promise.resolve();
-    const previousScriptState = this._scriptPositions(script);
-    if (!previousScriptState)
-      return Promise.resolve();
-
-    const hasBlackboxedMappings = sourceMap.sourceURLs().some(url => this.isBlackboxedURL(url));
-    const mappings = hasBlackboxedMappings ? sourceMap.mappings().slice() : [];
-    if (!mappings.length) {
-      if (previousScriptState.length > 0)
-        return this._setScriptState(script, []);
-      return Promise.resolve();
-    }
-    mappings.sort(mappingComparator);
+  async sourceMapLoaded(script, sourceMap) {
+    const hasBlackboxedMappings = sourceMap ? sourceMap.sourceURLs().some(url => this.isBlackboxedURL(url)) : false;
+    if (!hasBlackboxedMappings) {
+      if (script[Bindings.BlackboxManager._blackboxedRanges] && await script.setBlackboxedRanges([]))
+        delete script[Bindings.BlackboxManager._blackboxedRanges];
+      return;
+    }
 
+    const mappings = sourceMap.mappings();
+    const newRanges = [];
     let currentBlackboxed = false;
-    let isBlackboxed = false;
-    const positions = [];
-    // If content in script file begin is not mapped and one or more ranges are blackboxed then blackbox it.
     if (mappings[0].lineNumber !== 0 || mappings[0].columnNumber !== 0) {
-      positions.push({lineNumber: 0, columnNumber: 0});
+      newRanges.push({lineNumber: 0, columnNumber: 0});
       currentBlackboxed = true;
     }
     for (const mapping of mappings) {
       if (mapping.sourceURL && currentBlackboxed !== this.isBlackboxedURL(mapping.sourceURL)) {
-        positions.push({lineNumber: mapping.lineNumber, columnNumber: mapping.columnNumber});
+        newRanges.push({lineNumber: mapping.lineNumber, columnNumber: mapping.columnNumber});
         currentBlackboxed = !currentBlackboxed;
       }
-      isBlackboxed = currentBlackboxed || isBlackboxed;
     }
-    return this._setScriptState(script, !isBlackboxed ? [] : positions);
+
+    const oldRanges = script[Bindings.BlackboxManager._blackboxedRanges] || [];
+    if (!isEqual(oldRanges, newRanges) && await script.setBlackboxedRanges(newRanges))
+      script[Bindings.BlackboxManager._blackboxedRanges] = newRanges;
+
     /**
-     * @param {!SDK.SourceMapEntry} a
-     * @param {!SDK.SourceMapEntry} b
-     * @return {number}
+     * @param {!Array<!{lineNumber: number, columnNumber: number}>} rangesA
+     * @param {!Array<!{lineNumber: number, columnNumber: number}>} rangesB
+     * @return {boolean}
      */
-    function mappingComparator(a, b) {
-      if (a.lineNumber !== b.lineNumber)
-        return a.lineNumber - b.lineNumber;
-      return a.columnNumber - b.columnNumber;
+    function isEqual(rangesA, rangesB) {
+      if (rangesA.length !== rangesB.length)
+        return false;
+      for (let i = 0; i < rangesA.length; ++i) {
+        if (rangesA[i].lineNumber !== rangesB[i].lineNumber || rangesA[i].columnNumber !== rangesB[i].columnNumber)
+          return false;
+      }
+      return true;
     }
   }
 
@@ -269,136 +267,29 @@ Bindings.BlackboxManager = class {
     Common.moduleSetting('skipStackFramesPattern').setAsArray(regexPatterns);
   }
 
-  _patternChanged() {
+  async _patternChanged() {
     this._isBlackboxedURLCache.clear();
 
     /** @type {!Array<!Promise>} */
     const promises = [];
     for (const debuggerModel of SDK.targetManager.models(SDK.DebuggerModel)) {
       promises.push(this._setBlackboxPatterns(debuggerModel));
-      for (const script of debuggerModel.scripts())
-        promises.push(this._addScript(script).then(loadSourceMap.bind(this, script)));
+      for (const script of debuggerModel.scripts()) {
+        promises.push(this.sourceMapLoaded(script, this._debuggerWorkspaceBinding.sourceMapForScript(script))
+                          .then(() => this._debuggerWorkspaceBinding.updateLocations(script)));
+      }
     }
-    Promise.all(promises).then(() => {
+    await Promise.all(promises);
     const listeners = Array.from(this._listeners);
     for (const listener of listeners)
       listener();
     this._patternChangeFinishedForTests();
-    });
-
-    /**
-     * @param {!SDK.Script} script
-     * @return {!Promise<undefined>}
-     * @this {Bindings.BlackboxManager}
-     */
-    function loadSourceMap(script) {
-      return this.sourceMapLoaded(script, this._debuggerWorkspaceBinding.sourceMapForScript(script));
-    }
   }
 
   _patternChangeFinishedForTests() {
     // This method is sniffed in tests.
   }
 
-  /**
-   * @param {!Common.Event} event
-   */
-  _globalObjectCleared(event) {
-    const debuggerModel = /** @type {!SDK.DebuggerModel} */ (event.data);
-    this._debuggerModelData.delete(debuggerModel);
-    this._isBlackboxedURLCache.clear();
-  }
-
-  /**
-   * @param {!Common.Event} event
-   */
-  _parsedScriptSource(event) {
-    const script = /** @type {!SDK.Script} */ (event.data);
-    this._addScript(script);
-  }
-
-  /**
-   * @param {!SDK.Script} script
-   * @return {!Promise<undefined>}
-   */
-  _addScript(script) {
-    if (!script.sourceURL && !script.sourceMapURL)
-      return Promise.resolve();
-    const blackboxed = this._isBlackboxedScript(script);
-    return this._setScriptState(script, blackboxed ? [{lineNumber: 0, columnNumber: 0}] : []);
-  }
-
-  /**
-   * @param {!SDK.Script} script
-   * @return {boolean}
-   */
-  _isBlackboxedScript(script) {
-    return this.isBlackboxedURL(script.sourceURL, script.isContentScript());
-  }
-
-  /**
-   * @param {!SDK.Script} script
-   * @return {?Array<!Protocol.Debugger.ScriptPosition>}
-   */
-  _scriptPositions(script) {
-    if (this._debuggerModelData.has(script.debuggerModel))
-      return this._debuggerModelData.get(script.debuggerModel).get(script.scriptId) || null;
-    return null;
-  }
-
-  /**
-   * @param {!SDK.Script} script
-   * @param {!Array<!Protocol.Debugger.ScriptPosition>} positions
-   */
-  _setScriptPositions(script, positions) {
-    const debuggerModel = script.debuggerModel;
-    if (!this._debuggerModelData.has(debuggerModel))
-      this._debuggerModelData.set(debuggerModel, new Map());
-    this._debuggerModelData.get(debuggerModel).set(script.scriptId, positions);
-  }
-
-  /**
-   * @param {!SDK.Script} script
-   * @param {!Array<!Protocol.Debugger.ScriptPosition>} positions
-   * @return {!Promise<undefined>}
-   */
-  _setScriptState(script, positions) {
-    const previousScriptState = this._scriptPositions(script);
-    if (previousScriptState) {
-      let hasChanged = false;
-      hasChanged = previousScriptState.length !== positions.length;
-      for (let i = 0; !hasChanged && i < positions.length; ++i) {
-        hasChanged = positions[i].lineNumber !== previousScriptState[i].lineNumber ||
-            positions[i].columnNumber !== previousScriptState[i].columnNumber;
-      }
-      if (!hasChanged)
-        return Promise.resolve();
-    } else {
-      if (positions.length === 0)
-        return Promise.resolve().then(updateState.bind(this, false));
-    }
-
-    return script.setBlackboxedRanges(positions).then(updateState.bind(this));
-
-    /**
-     * @param {boolean} success
-     * @this {Bindings.BlackboxManager}
-     */
-    function updateState(success) {
-      if (success) {
-        this._setScriptPositions(script, positions);
-        this._debuggerWorkspaceBinding.updateLocations(script);
-        const isBlackboxed = positions.length !== 0;
-        if (!isBlackboxed && script.sourceMapURL)
-          this._debuggerWorkspaceBinding.maybeLoadSourceMap(script);
-      } else {
-        const hasPositions = !!this._scriptPositions(script);
-        if (!hasPositions)
-          this._setScriptPositions(script, []);
-      }
-    }
-  }
-
   /**
    * @param {string} url
    * @return {string}
@@ -430,5 +321,7 @@ Bindings.BlackboxManager = class {
   }
 };
 
+Bindings.BlackboxManager._blackboxedRanges = Symbol('blackboxedRanged');
+
 /** @type {!Bindings.BlackboxManager} */
 Bindings.blackboxManager;

third_party/blink/renderer/devtools/front_end/bindings/CompilerScriptMapping.js

@@ -244,16 +244,6 @@ Bindings.CompilerScriptMapping = class {
     return this._sourceMapManager.sourceMapForClient(script);
   }
 
-  /**
-   * @param {!SDK.Script} script
-   */
-  maybeLoadSourceMap(script) {
-    const sourceMap = this._sourceMapManager.sourceMapForClient(script);
-    if (!sourceMap)
-      return;
-    this._populateSourceMapSources(script, sourceMap);
-  }
-
   /**
    * @param {?SDK.SourceMap} sourceMap
    */

third_party/blink/renderer/devtools/front_end/sdk/Script.js

@@ -254,10 +254,7 @@ SDK.Script = class {
   async setBlackboxedRanges(positions) {
     const response = await this.debuggerModel.target().debuggerAgent().invoke_setBlackboxedRanges(
         {scriptId: this.scriptId, positions});
-    const error = response[Protocol.Error];
-    if (error)
-      console.error(error);
-    return !error;
+    return !response[Protocol.Error];
   }
 };