Bug 1134180 - Introduce RootActor.getTab to prevent creating TabActor for all tabs. r=past
authorAlexandre Poirot <poirot.alex@gmail.com>
Wed, 25 Mar 2015 09:30:00 -0400
changeset 264730 d058933e812e8aada6b92e48da81f28dfea9f939
parent 264729 718d356434012dd3d4db320bc68be89266b023fa
child 264731 583035a90903b06c60e434f25f70a74bbfe82f4e
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspast
bugs1134180
milestone39.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1134180 - Introduce RootActor.getTab to prevent creating TabActor for all tabs. r=past
browser/devtools/framework/target.js
browser/devtools/framework/test/browser_two_tabs.js
toolkit/devtools/client/dbg-client.jsm
toolkit/devtools/server/actors/root.js
toolkit/devtools/server/actors/webbrowser.js
--- a/browser/devtools/framework/target.js
+++ b/browser/devtools/framework/target.js
@@ -421,36 +421,19 @@ TabTarget.prototype = {
         this.activeTab = aTabClient;
         this.threadActor = aResponse.threadActor;
         this._remote.resolve(null);
       });
     };
 
     if (this.isLocalTab) {
       this._client.connect((aType, aTraits) => {
-        this._client.listTabs(aResponse => {
-          this._root = aResponse;
-
-          if (this.window) {
-            let windowUtils = this.window
-              .QueryInterface(Ci.nsIInterfaceRequestor)
-              .getInterface(Ci.nsIDOMWindowUtils);
-            let outerWindow = windowUtils.outerWindowID;
-            aResponse.tabs.some((tab) => {
-              if (tab.outerWindowID === outerWindow) {
-                this._form = tab;
-                return true;
-              }
-              return false;
-            });
-          }
-
-          if (!this._form) {
-            this._form = aResponse.tabs[aResponse.selected];
-          }
+        this._client.getTab({ tab: this.tab })
+            .then(aResponse => {
+          this._form = aResponse.tab;
           attachTab();
         });
       });
     } else if (this.isTabActor) {
       // In the remote debugging case, the protocol connection will have been
       // already initialized in the connection screen code.
       attachTab();
     } else {
--- a/browser/devtools/framework/test/browser_two_tabs.js
+++ b/browser/devtools/framework/test/browser_two_tabs.js
@@ -49,21 +49,74 @@ function connect() {
   // Connect to debugger server to fetch the two tab actors
   gClient = new DebuggerClient(DebuggerServer.connectPipe());
   gClient.connect(() => {
     gClient.listTabs(response => {
       // Fetch the tab actors for each tab
       gTabActor1 = response.tabs.filter(a => a.url === TAB_URL_1)[0];
       gTabActor2 = response.tabs.filter(a => a.url === TAB_URL_2)[0];
 
-      checkSelectedTabActor();
+      checkGetTab();
     });
   });
 }
 
+function checkGetTab() {
+  gClient.getTab({tab: gTab1})
+         .then(response => {
+           is(JSON.stringify(gTabActor1), JSON.stringify(response.tab),
+              "getTab returns the same tab grip for first tab");
+         })
+         .then(() => {
+           let filter = {};
+           // Filter either by tabId or outerWindowID,
+           // if we are running tests OOP or not.
+           if (gTab1.linkedBrowser.frameLoader.tabParent) {
+             filter.tabId = gTab1.linkedBrowser.frameLoader.tabParent.tabId;
+           } else {
+             let windowUtils = gTab1.linkedBrowser.contentWindow
+               .QueryInterface(Ci.nsIInterfaceRequestor)
+               .getInterface(Ci.nsIDOMWindowUtils);
+             filter.outerWindowID = windowUtils.outerWindowID;
+           }
+           return gClient.getTab(filter);
+         })
+         .then(response => {
+           is(JSON.stringify(gTabActor1), JSON.stringify(response.tab),
+              "getTab returns the same tab grip when filtering by tabId/outerWindowID");
+         })
+         .then(() => gClient.getTab({tab: gTab2}))
+         .then(response => {
+           is(JSON.stringify(gTabActor2), JSON.stringify(response.tab),
+              "getTab returns the same tab grip for second tab");
+         })
+         .then(checkGetTabFailures);
+}
+
+function checkGetTabFailures() {
+  gClient.getTab({ tabId: -999 })
+    .then(
+      response => ok(false, "getTab unexpectedly succeed with a wrong tabId"),
+      response => {
+        is(response.error, "noTab");
+        is(response.message, "Unable to find tab with tabId '-999'");
+      }
+    )
+    .then(() => gClient.getTab({ outerWindowID: -999 }))
+    .then(
+      response => ok(false, "getTab unexpectedly succeed with a wrong outerWindowID"),
+      response => {
+        is(response.error, "noTab");
+        is(response.message, "Unable to find tab with outerWindowID '-999'");
+      }
+    )
+    .then(checkSelectedTabActor);
+
+}
+
 function checkSelectedTabActor() {
   // Send a naive request to the second tab actor
   // to check if it works
   gClient.request({ to: gTabActor2.consoleActor, type: "startListeners", listeners: [] }, aResponse => {
     ok("startedListeners" in aResponse, "Actor from the selected tab should respond to the request.");
 
     closeSecondTab();
   });
--- a/toolkit/devtools/client/dbg-client.jsm
+++ b/toolkit/devtools/client/dbg-client.jsm
@@ -463,16 +463,18 @@ DebuggerClient.prototype = {
   listTabs: function (aOnResponse) { return this.mainRoot.listTabs(aOnResponse); },
 
   /*
    * This function exists only to preserve DebuggerClient's interface;
    * new code should say 'client.mainRoot.listAddons()'.
    */
   listAddons: function (aOnResponse) { return this.mainRoot.listAddons(aOnResponse); },
 
+  getTab: function (aFilter) { return this.mainRoot.getTab(aFilter); },
+
   /**
    * Attach to a tab actor.
    *
    * @param string aTabActor
    *        The actor ID for the tab to attach.
    * @param function aOnResponse
    *        Called with the response packet and a TabClient
    *        (which will be undefined on error).
@@ -1402,16 +1404,61 @@ RootClient.prototype = {
    *
    * @param function aOnResponse
    *        Called with the response packet.
    */
   listProcesses: DebuggerClient.requester({ type: "listProcesses" },
                                        { telemetry: "LISTPROCESSES" }),
 
   /**
+   * Fetch the TabActor for the currently selected tab, or for a specific
+   * tab given as first parameter.
+   *
+   * @param [optional] object aFilter
+   *        A dictionary object with following optional attributes:
+   *         - outerWindowID: used to match tabs in parent process
+   *         - tabId: used to match tabs in child processes
+   *         - tab: a reference to xul:tab element
+   *        If nothing is specified, returns the actor for the currently
+   *        selected tab.
+   */
+  getTab: function (aFilter) {
+    let packet = {
+      to: this.actor,
+      type: "getTab"
+    };
+
+    if (aFilter) {
+      if (typeof(aFilter.outerWindowID) == "number") {
+        packet.outerWindowID = aFilter.outerWindowID;
+      } else if (typeof(aFilter.tabId) == "number") {
+        packet.tabId = aFilter.tabId;
+      } else if ("tab" in aFilter) {
+        let browser = aFilter.tab.linkedBrowser;
+        if (browser.frameLoader.tabParent) {
+          // Tabs in child process
+          packet.tabId = browser.frameLoader.tabParent.tabId;
+        } else {
+          // Tabs in parent process
+          let windowUtils = browser.contentWindow
+            .QueryInterface(Ci.nsIInterfaceRequestor)
+            .getInterface(Ci.nsIDOMWindowUtils);
+          packet.outerWindowID = windowUtils.outerWindowID;
+        }
+      } else {
+        // Throw if a filter object have been passed but without
+        // any clearly idenfified filter.
+        throw new Error("Unsupported argument given to getTab request");
+      }
+    }
+
+    return this.request(packet);
+  },
+
+  /**
    * Description of protocol's actors and methods.
    *
    * @param function aOnResponse
    *        Called with the response packet.
    */
    protocolDescription: DebuggerClient.requester({ type: "protocolDescription" },
                                                  { telemetry: "PROTOCOLDESCRIPTION" }),
 
--- a/toolkit/devtools/server/actors/root.js
+++ b/toolkit/devtools/server/actors/root.js
@@ -263,16 +263,43 @@ RootActor.prototype = {
        * changes.
        */
       tabList.onListChanged = this._onTabListChanged;
 
       return reply;
     });
   },
 
+  onGetTab: function (options) {
+    let tabList = this._parameters.tabList;
+    if (!tabList) {
+      return { error: "noTabs",
+               message: "This root actor has no browser tabs." };
+    }
+    if (!this._tabActorPool) {
+      this._tabActorPool = new ActorPool(this.conn);
+      this.conn.addActorPool(this._tabActorPool);
+    }
+    return tabList.getTab(options)
+                  .then(tabActor => {
+      tabActor.parentID = this.actorID;
+      this._tabActorPool.addActor(tabActor);
+
+      return { tab: tabActor.form() };
+    }, error => {
+      if (error.error) {
+        // Pipe expected errors as-is to the client
+        return error;
+      } else {
+        return { error: "noTab",
+                 message: "Unexpected error while calling getTab(): " + error };
+      }
+    });
+  },
+
   onTabListChanged: function () {
     this.conn.send({ from: this.actorID, type:"tabListChanged" });
     /* It's a one-shot notification; no need to watch any more. */
     this._parameters.tabList.onListChanged = null;
   },
 
   onListAddons: function () {
     let addonList = this._parameters.addonList;
@@ -386,16 +413,17 @@ RootActor.prototype = {
       }
       delete this._extraActors[aName];
     }
    }
 };
 
 RootActor.prototype.requestTypes = {
   "listTabs": RootActor.prototype.onListTabs,
+  "getTab": RootActor.prototype.onGetTab,
   "listAddons": RootActor.prototype.onListAddons,
   "listProcesses": RootActor.prototype.onListProcesses,
   "attachProcess": RootActor.prototype.onAttachProcess,
   "echo": RootActor.prototype.onEcho,
   "protocolDescription": RootActor.prototype.onProtocolDescription
 };
 
 exports.RootActor = RootActor;
--- a/toolkit/devtools/server/actors/webbrowser.js
+++ b/toolkit/devtools/server/actors/webbrowser.js
@@ -325,55 +325,108 @@ BrowserTabList.prototype.getList = funct
   let selectedBrowser = null;
   if (topXULWindow) {
     selectedBrowser = this._getSelectedBrowser(topXULWindow);
   }
 
   // As a sanity check, make sure all the actors presently in our map get
   // picked up when we iterate over all windows' tabs.
   let initialMapSize = this._actorByBrowser.size;
-  let foundCount = 0;
+  this._foundCount = 0;
 
   // To avoid mysterious behavior if tabs are closed or opened mid-iteration,
   // we update the map first, and then make a second pass over it to yield
   // the actors. Thus, the sequence yielded is always a snapshot of the
   // actors that were live when we began the iteration.
 
   let actorPromises = [];
 
   for (let browser of this._getBrowsers()) {
-    // Do we have an existing actor for this browser? If not, create one.
-    let actor = this._actorByBrowser.get(browser);
-    if (actor) {
-      actorPromises.push(actor.update());
-      foundCount++;
-    } else if (this._isRemoteBrowser(browser)) {
-      actor = new RemoteBrowserTabActor(this._connection, browser);
-      this._actorByBrowser.set(browser, actor);
-      actorPromises.push(actor.connect());
-    } else {
-      actor = new BrowserTabActor(this._connection, browser,
-                                  browser.getTabBrowser());
-      this._actorByBrowser.set(browser, actor);
-      actorPromises.push(promise.resolve(actor));
-    }
-
-    // Set the 'selected' properties on all actors correctly.
-    actor.selected = browser === selectedBrowser;
+    let selected = browser === selectedBrowser;
+    actorPromises.push(
+      this._getActorForBrowser(browser)
+          .then(actor => {
+            // Set the 'selected' properties on all actors correctly.
+            actor.selected = selected;
+            return actor;
+          })
+    );
   }
 
-  if (this._testing && initialMapSize !== foundCount)
+  if (this._testing && initialMapSize !== this._foundCount)
     throw Error("_actorByBrowser map contained actors for dead tabs");
 
   this._mustNotify = true;
   this._checkListening();
 
   return promise.all(actorPromises);
 };
 
+BrowserTabList.prototype._getActorForBrowser = function(browser) {
+  // Do we have an existing actor for this browser? If not, create one.
+  let actor = this._actorByBrowser.get(browser);
+  if (actor) {
+    this._foundCount++;
+    return actor.update();
+  } else if (this._isRemoteBrowser(browser)) {
+    actor = new RemoteBrowserTabActor(this._connection, browser);
+    this._actorByBrowser.set(browser, actor);
+    this._checkListening();
+    return actor.connect();
+  } else {
+    actor = new BrowserTabActor(this._connection, browser,
+                                browser.getTabBrowser());
+    this._actorByBrowser.set(browser, actor);
+    this._checkListening();
+    return promise.resolve(actor);
+  }
+};
+
+BrowserTabList.prototype.getTab = function({ outerWindowID, tabId }) {
+  if (typeof(outerWindowID) == "number") {
+    // Tabs in parent process
+    for (let browser of this._getBrowsers()) {
+      if (browser.contentWindow) {
+        let windowUtils = browser.contentWindow
+          .QueryInterface(Ci.nsIInterfaceRequestor)
+          .getInterface(Ci.nsIDOMWindowUtils);
+        if (windowUtils.outerWindowID === outerWindowID) {
+          return this._getActorForBrowser(browser);
+        }
+      }
+    }
+    return promise.reject({
+      error: "noTab",
+      message: "Unable to find tab with outerWindowID '" + outerWindowID + "'"
+    });
+  } else if (typeof(tabId) == "number") {
+    // Tabs OOP
+    for (let browser of this._getBrowsers()) {
+      if (browser.frameLoader.tabParent &&
+          browser.frameLoader.tabParent.tabId === tabId) {
+        return this._getActorForBrowser(browser);
+      }
+    }
+    return promise.reject({
+      error: "noTab",
+      message: "Unable to find tab with tabId '" + tabId + "'"
+    });
+  }
+
+  let topXULWindow = Services.wm.getMostRecentWindow(DebuggerServer.chromeWindowType);
+  if (topXULWindow) {
+    let selectedBrowser = this._getSelectedBrowser(topXULWindow);
+    return this._getActorForBrowser(selectedBrowser);
+  }
+  return promise.reject({
+    error: "noTab",
+    message: "Unable to find any selected browser"
+  });
+};
+
 Object.defineProperty(BrowserTabList.prototype, 'onListChanged', {
   enumerable: true, configurable:true,
   get: function() { return this._onListChanged; },
   set: function(v) {
     if (v !== null && typeof v !== 'function') {
       throw Error("onListChanged property may only be set to 'null' or a function");
     }
     this._onListChanged = v;