Bug 1290263 - Source maps in console don't work after first log; r=jsantell
authorJaideep Bhoosreddy <jaideepb@buffalo.edu>
Wed, 10 Aug 2016 17:01:25 -0700
changeset 399632 4fb7850312d547f61a467f0150ad835b281aacfc
parent 399631 c50708c38f8119fe16a8235aa067fa4934ea0af0
child 399633 6275988f9d94c9694d9a28f505ab92289110f1d5
push id25905
push userdtownsend@mozilla.com
push dateThu, 11 Aug 2016 17:57:58 +0000
reviewersjsantell
bugs1290263
milestone51.0a1
Bug 1290263 - Source maps in console don't work after first log; r=jsantell
devtools/client/framework/source-map-service.js
devtools/client/framework/test/browser_source_map-01.js
--- a/devtools/client/framework/source-map-service.js
+++ b/devtools/client/framework/source-map-service.js
@@ -14,17 +14,16 @@ const { LocationStore, serialize, deseri
  * auto-update when the source changes (from pretty printing, source maps loading, etc)
  *
  * @param {TabTarget} target
  */
 
 function SourceMapService(target) {
   this._target = target;
   this._locationStore = new LocationStore();
-  this._isInitialResolve = true;
 
   EventEmitter.decorate(this);
 
   this._onSourceUpdated = this._onSourceUpdated.bind(this);
   this._resolveLocation = this._resolveLocation.bind(this);
   this._resolveAndUpdate = this._resolveAndUpdate.bind(this);
   this.subscribe = this.subscribe.bind(this);
   this.unsubscribe = this.unsubscribe.bind(this);
@@ -36,72 +35,66 @@ function SourceMapService(target) {
   target.on("will-navigate", this.reset);
   target.on("close", this.destroy);
 }
 
 /**
  * Clears the store containing the cached resolved locations and promises
  */
 SourceMapService.prototype.reset = function () {
-  this._isInitialResolve = true;
   this._locationStore.clear();
 };
 
 SourceMapService.prototype.destroy = function () {
   this.reset();
   this._target.off("source-updated", this._onSourceUpdated);
   this._target.off("navigate", this.reset);
   this._target.off("will-navigate", this.reset);
   this._target.off("close", this.destroy);
-  this._isInitialResolve = null;
   this._target = this._locationStore = null;
 };
 
 /**
  * Sets up listener for the callback to update the FrameView and tries to resolve location
  * @param location
  * @param callback
  */
 SourceMapService.prototype.subscribe = function (location, callback) {
   this.on(serialize(location), callback);
   this._locationStore.set(location);
-  if (this._isInitialResolve) {
-    this._resolveAndUpdate(location);
-    this._isInitialResolve = false;
-  }
+  this._resolveAndUpdate(location);
 };
 
 /**
  * Removes the listener for the location and clears cached locations
  * @param location
  * @param callback
  */
 SourceMapService.prototype.unsubscribe = function (location, callback) {
   this.off(serialize(location), callback);
-  this._locationStore.clearByURL(location.url);
+  // Check to see if the store exists before attempting to clear a location
+  // Sometimes un-subscribe happens during the destruction cascades and this
+  // condition is to protect against that. Could be looked into in the future.
+  if (this._locationStore) {
+    this._locationStore.clearByURL(location.url);
+  }
 };
 
 /**
  * Tries to resolve the location and if successful,
  * emits the resolved location and caches it
  * @param location
  * @private
  */
 SourceMapService.prototype._resolveAndUpdate = function (location) {
   this._resolveLocation(location).then(resolvedLocation => {
-    // We try to source map the first console log to initiate the source-updated event from
-    // target. The isSameLocation check is to make sure we don't update the frame, if the
-    // location is not source-mapped.
-    if (resolvedLocation) {
-      if (this._isInitialResolve) {
-        if (!isSameLocation(location, resolvedLocation)) {
-          this.emit(serialize(location), location, resolvedLocation);
-          return;
-        }
-      }
+    // We try to source map the first console log to initiate the source-updated
+    // event from target. The isSameLocation check is to make sure we don't update
+    // the frame, if the location is not source-mapped.
+    if (resolvedLocation && !isSameLocation(location, resolvedLocation)) {
       this.emit(serialize(location), location, resolvedLocation);
     }
   });
 };
 
 /**
  * Validates the location model,
  * checks if there is existing promise to resolve location, if so returns cached promise
@@ -177,24 +170,23 @@ function resolveLocation(target, locatio
       url: location.url,
       line: location.line,
       column: location.column || Infinity
     });
     // Source or mapping not found, so don't do anything
     if (newLocation.error) {
       return null;
     }
-
     return newLocation;
   });
 }
 
 /**
  * Returns if the original location and resolved location are the same
  * @param location
  * @param resolvedLocation
  * @returns {boolean}
  */
 function isSameLocation(location, resolvedLocation) {
   return location.url === resolvedLocation.url &&
     location.line === resolvedLocation.line &&
     location.column === resolvedLocation.column;
-};
\ No newline at end of file
+}
--- a/devtools/client/framework/test/browser_source_map-01.js
+++ b/devtools/client/framework/test/browser_source_map-01.js
@@ -14,49 +14,48 @@ thisTestLeaksUncaughtRejectionsAndShould
  */
 
 const DEBUGGER_ROOT = "http://example.com/browser/devtools/client/debugger/test/mochitest/";
 // Empty page
 const PAGE_URL = `${DEBUGGER_ROOT}doc_empty-tab-01.html`;
 const JS_URL = `${URL_ROOT}code_binary_search.js`;
 const COFFEE_URL = `${URL_ROOT}code_binary_search.coffee`;
 const { SourceMapService } = require("devtools/client/framework/source-map-service");
+const { serialize } = require("devtools/client/framework/location-store");
 
 add_task(function* () {
   const toolbox = yield openNewTabAndToolbox(PAGE_URL, "jsdebugger");
-
   const service = new SourceMapService(toolbox.target);
-
-  const aggregator = [];
+  let aggregator = new Map();
 
   function onUpdate(e, oldLoc, newLoc) {
     if (oldLoc.line === 6) {
       checkLoc1(oldLoc, newLoc);
     } else if (oldLoc.line === 8) {
       checkLoc2(oldLoc, newLoc);
-    } else if (oldLoc.line === 2) {
-      checkLoc3(oldLoc, newLoc);
     } else {
       throw new Error(`Unexpected location update: ${JSON.stringify(oldLoc)}`);
     }
-    aggregator.push(newLoc);
+    aggregator.set(serialize(oldLoc), newLoc);
   }
 
   let loc1 = { url: JS_URL, line: 6 };
   let loc2 = { url: JS_URL, line: 8, column: 3 };
 
   service.subscribe(loc1, onUpdate);
   service.subscribe(loc2, onUpdate);
 
   // Inject JS script
   let sourceShown = waitForSourceShown(toolbox.getCurrentPanel(), "code_binary_search");
   yield createScript(JS_URL);
   yield sourceShown;
 
-  yield waitUntil(() => aggregator.length === 2);
+  yield waitUntil(() => aggregator.size === 2);
+
+  aggregator = Array.from(aggregator.values());
 
   ok(aggregator.find(i => i.url === COFFEE_URL && i.line === 4), "found first updated location");
   ok(aggregator.find(i => i.url === COFFEE_URL && i.line === 6), "found second updated location");
 
   yield toolbox.destroy();
   gBrowser.removeCurrentTab();
   finish();
 });