[Devtools] Lazily build reverse mappings

third_party/WebKit/Source/devtools/front_end/sdk/SourceMap.js

Index: third_party/WebKit/Source/devtools/front_end/sdk/SourceMap.js
diff --git a/third_party/WebKit/Source/devtools/front_end/sdk/SourceMap.js b/third_party/WebKit/Source/devtools/front_end/sdk/SourceMap.js
index 58c6def49e424619adb62bfab9bcaacf1a62423e..33c11ea55db6be4b8a3aa4886dfcdbf7b41cfdef 100644
--- a/third_party/WebKit/Source/devtools/front_end/sdk/SourceMap.js
+++ b/third_party/WebKit/Source/devtools/front_end/sdk/SourceMap.js
@@ -96,7 +96,7 @@ WebInspector.SourceMap.prototype = {
     url: function() { },
 
     /**
-     * @return {!Array<string>}
+     * @return {!Iterable<string>|!Array<string>}

[lushnikov, 2016/09/28 16:48:12]

let's keep this array - we try to avoid polymorphic types as hard as possible

[eostroukhov, 2016/09/28 23:26:33]

This CL is about saving memory! ;)

(Ok, I fixed this)

      */
     sourceURLs: function() { },
 
@@ -172,13 +172,14 @@ WebInspector.TextSourceMap = function(compiledURL, sourceMappingURL, payload)
             WebInspector.TextSourceMap.prototype._base64Map[base64Digits.charAt(i)] = i;
     }
 
+    this._json = payload;
     this._compiledURL = compiledURL;
     this._sourceMappingURL = sourceMappingURL;
-    this._reverseMappingsBySourceURL = new Map();
-    this._mappings = [];
-    this._sources = {};
+    /** @type {?Array<!WebInspector.SourceMapEntry>} */
+    this._mappings = null;
+    this._sources = new Map();

[lushnikov, 2016/09/28 16:48:12]

1. let's add jsdoc
2. let's name this _sourceURLToReversedMappings

[eostroukhov, 2016/09/28 23:26:33]

Done.

     this._sourceContentByURL = {};
-    this._parseMappingPayload(payload);
+    this._eachSection(this._parseSources.bind(this));
 }
 
 /**
@@ -211,7 +212,6 @@ WebInspector.TextSourceMap.load = function(sourceMapURL, compiledURL)
         try {
             var payload = /** @type {!SourceMapV3} */ (JSON.parse(content));
             var baseURL = sourceMapURL.startsWith("data:") ? compiledURL : sourceMapURL;
-
             callback(new WebInspector.TextSourceMap(compiledURL, baseURL, payload));
         } catch (e) {
             console.error(e);
@@ -242,11 +242,11 @@ WebInspector.TextSourceMap.prototype = {
 
     /**
      * @override
-     * @return {!Array.<string>}
+     * @return {!Iterable.<string>}
      */
     sourceURLs: function()
     {
-        return Object.keys(this._sources);
+        return this._sources.keys();

[lushnikov, 2016/09/28 16:48:13]

let's use .keysArray() to return array

[eostroukhov, 2016/09/28 23:26:33]

Done.

     },
 
     /**
@@ -284,28 +284,6 @@ WebInspector.TextSourceMap.prototype = {
     },
 
     /**
-     * @param {!SourceMapV3} mappingPayload
-     */
-    _parseMappingPayload: function(mappingPayload)
-    {
-        if (mappingPayload.sections)
-            this._parseSections(mappingPayload.sections);
-        else
-            this._parseMap(mappingPayload, 0, 0);
-    },
-
-    /**
-     * @param {!Array.<!SourceMapV3.Section>} sections
-     */
-    _parseSections: function(sections)
-    {
-        for (var i = 0; i < sections.length; ++i) {
-            var section = sections[i];
-            this._parseMap(section.map, section.offset.line, section.offset.column);
-        }
-    },
-
-    /**
      * @override
      * @param {number} lineNumber in compiled resource
      * @param {number} columnNumber in compiled resource
@@ -314,11 +292,12 @@ WebInspector.TextSourceMap.prototype = {
     findEntry: function(lineNumber, columnNumber)
     {
         var first = 0;
-        var count = this._mappings.length;
+        var mappings = this.mappings();
+        var count = mappings.length;
         while (count > 1) {
             var step = count >> 1;
             var middle = first + step;
-            var mapping = this._mappings[middle];
+            var mapping = mappings[middle];
             if (lineNumber < mapping.lineNumber || (lineNumber === mapping.lineNumber && columnNumber < mapping.columnNumber)) {
                 count = step;
             } else {
@@ -326,7 +305,7 @@ WebInspector.TextSourceMap.prototype = {
                 count -= step;
             }
         }
-        var entry = this._mappings[first];
+        var entry = mappings[first];
         if (!first && entry && (lineNumber < entry.lineNumber || (lineNumber === entry.lineNumber && columnNumber < entry.columnNumber)))
             return null;
         return entry;
@@ -361,7 +340,12 @@ WebInspector.TextSourceMap.prototype = {
      */
     mappings: function()
     {
-        return this._mappings;
+        if (this._mappings === null) {
+            this._mappings = [];
+            this._eachSection(this._parseMap.bind(this));
+            this._json = null;
+        }
+        return /** @type {!Array<!WebInspector.SourceMapEntry>} */ (this._mappings);
     },
 
     /**
@@ -370,14 +354,15 @@ WebInspector.TextSourceMap.prototype = {
      */
     _reversedMappings: function(sourceURL)
     {
-        var mappings = this._reverseMappingsBySourceURL.get(sourceURL);
-        if (!mappings)
+        if (this._sources && !this._sources.has(sourceURL))
             return [];
-        if (!mappings._sorted) {
-            mappings.sort(sourceMappingComparator);
-            mappings._sorted = true;
+        let mappings = this.mappings();

[lushnikov, 2016/09/28 16:48:13]

nit: we're not yet using "let" in the codebase

[eostroukhov, 2016/09/28 23:26:34]

Done.

+        let reverseMappings = this._sources.get(sourceURL);
+        if (reverseMappings === null) {
+            reverseMappings = mappings.filter((mapping) => mapping.sourceURL === sourceURL).sort(sourceMappingComparator);
+            this._sources.set(sourceURL, reverseMappings);
         }
-        return mappings;
+        return reverseMappings;
 
         /**
          * @param {!WebInspector.SourceMapEntry} a
@@ -399,6 +384,36 @@ WebInspector.TextSourceMap.prototype = {
     },
 
     /**
+     * @param {function(!SourceMapV3, number, number)} callback
+     */
+    _eachSection: function(callback) {
+        const sections = this._json.sections || [this._json];

[lushnikov, 2016/09/28 16:48:13]

let's bail out early instead:

if (!this._json.sections) {
    callback(this._json, 0, 0);
    return;
}

...

[eostroukhov, 2016/09/28 23:26:33]

Done.

+        for (const section of sections) {

[lushnikov, 2016/09/28 16:48:13]

nit: we're not yet using "const" in the codebase

[eostroukhov, 2016/09/28 23:26:33]

Done.

+            const offset = section.offset || {line: 0, column: 0};
+            callback(section.map || section, offset.line, offset.column);
+        }
+    },
+
+    /**
+     * @param {!SourceMapV3} sourceMap
+     */
+    _parseSources: function(sourceMap) {
+        var sourceRoot = sourceMap.sourceRoot || "";
+        if (sourceRoot && !sourceRoot.endsWith("/"))
+            sourceRoot += "/";
+        for (var i = 0; i < sourceMap.sources.length; ++i) {

[lushnikov, 2016/09/28 16:48:13]

this looks like a copy of the code in _parseMap. Can we either extract this into
a separate function, or maybe re-use sources for the section (so that we don't
need to re-do this in _parseMap)?

[eostroukhov, 2016/09/28 23:26:33]

There are building different things - one is building source-map wide collection
of the sourceURLs and another needs a per-section array of the same URLs. I
tried to refactor it, ended up with another callback.

+            var href = sourceRoot + sourceMap.sources[i];
+            var url = WebInspector.ParsedURL.completeURL(this._sourceMappingURL, href) || href;
+            var hasSource = sourceMap.sourcesContent && sourceMap.sourcesContent[i];
+            if (url === this._compiledURL && hasSource)
+                url += WebInspector.UIString(" [sm]");
+            this._sources.set(url, null);
+            if (hasSource)
+                this._sourceContentByURL[url] = sourceMap.sourcesContent[i];
+        }
+    },
+
+    /**
      * @param {!SourceMapV3} map
      * @param {number} lineNumber
      * @param {number} columnNumber
@@ -422,10 +437,6 @@ WebInspector.TextSourceMap.prototype = {
             if (url === this._compiledURL && hasSource)
                 url += WebInspector.UIString(" [sm]");
             sources.push(url);
-            this._sources[url] = true;
-
-            if (hasSource)
-                this._sourceContentByURL[url] = map.sourcesContent[i];
         }
 
         var stringCharIterator = new WebInspector.TextSourceMap.StringCharIterator(map.mappings);
@@ -466,17 +477,6 @@ WebInspector.TextSourceMap.prototype = {
             nameIndex += this._decodeVLQ(stringCharIterator);
             this._mappings.push(new WebInspector.SourceMapEntry(lineNumber, columnNumber, sourceURL, sourceLineNumber, sourceColumnNumber, names[nameIndex]));
         }
-
-        for (var i = 0; i < this._mappings.length; ++i) {
-            var mapping = this._mappings[i];
-            var url = mapping.sourceURL;
-            if (!url)
-                continue;
-            if (!this._reverseMappingsBySourceURL.has(url))
-                this._reverseMappingsBySourceURL.set(url, []);
-            var reverseMappings = this._reverseMappingsBySourceURL.get(url);
-            reverseMappings.push(mapping);
-        }
     },
 
     /**