Bug 1240907 - Flatten RemoteBrowserTabActor into BrowserTabActor. r=ochameau draft
authorJ. Ryan Stinnett <jryans@gmail.com>
Wed, 06 Jul 2016 16:17:03 -0500
changeset 386522 a36e8ba82c7887a9505e47425d4a2a429d1f813c
parent 386521 49d39db3fed8f1d3baa7740e9416b03b40fc6b43
child 525136 e7d50069953dc18cd9111d463a04c947a57a7f56
push id22732
push userbmo:jryans@gmail.com
push dateTue, 12 Jul 2016 05:53:16 +0000
reviewersochameau
bugs1240907
milestone50.0a1
Bug 1240907 - Flatten RemoteBrowserTabActor into BrowserTabActor. r=ochameau MozReview-Commit-ID: EXEWehphLIY
devtools/server/actors/webbrowser.js
devtools/server/child.js
--- a/devtools/server/actors/webbrowser.js
+++ b/devtools/server/actors/webbrowser.js
@@ -310,20 +310,16 @@ BrowserTabList.prototype._getChildren = 
   return gBrowser.browsers.filter(browser => {
     // Filter tabs that are closing. listTabs calls made right after TabClose
     // events still list tabs in process of being closed.
     let tab = gBrowser.getTabForBrowser(browser);
     return !tab.closing;
   });
 };
 
-BrowserTabList.prototype._isRemoteBrowser = function (browser) {
-  return browser.getAttribute("remote") == "true";
-};
-
 BrowserTabList.prototype.getList = function () {
   let topXULWindow = Services.wm.getMostRecentWindow(
     DebuggerServer.chromeWindowType);
   let selectedBrowser = null;
   if (topXULWindow) {
     selectedBrowser = this._getSelectedBrowser(topXULWindow);
   }
 
@@ -362,27 +358,22 @@ BrowserTabList.prototype.getList = funct
 };
 
 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();
   }
 
   actor = new BrowserTabActor(this._connection, browser);
   this._actorByBrowser.set(browser, actor);
   this._checkListening();
-  return promise.resolve(actor);
+  return actor.connect();
 };
 
 BrowserTabList.prototype.getTab = function ({ outerWindowID, tabId }) {
   if (typeof outerWindowID == "number") {
     for (let browser of this._getBrowsers()) {
       if (browser.outerWindowID == outerWindowID) {
         return this._getActorForBrowser(browser);
       }
@@ -594,18 +585,17 @@ DevToolsUtils.makeInfallible(function (e
     case "TabClose": {
       let actor = this._actorByBrowser.get(browser);
       if (actor) {
         this._handleActorClose(actor, browser);
       }
       break;
     }
     case "TabRemotenessChange": {
-      // We have to remove the cached actor as we have to create a new instance
-      // based on BrowserTabActor or RemoteBrowserTabActor.
+      // We have to remove the cached actor as we have to create a new instance.
       let actor = this._actorByBrowser.get(browser);
       if (actor) {
         this._actorByBrowser.delete(browser);
         // Don't create a new actor; iterate will take care of that. Just notify.
         this._notifyListChanged();
         this._checkListening();
       }
       break;
@@ -822,19 +812,18 @@ exports.BrowserTabList = BrowserTabList;
  * Note that *all* these events are dispatched in the following order
  * when we switch the context of the TabActor to a given iframe:
  *  - will-navigate
  *  - window-destroyed
  *  - changed-toplevel-document
  *  - window-ready
  *  - navigate
  *
- * This class is subclassed by BrowserTabActor and
- * ContentActor. Subclasses are expected to implement a getter
- * for the docShell property.
+ * This class is subclassed by ContentActor and others.
+ * Subclasses are expected to implement a getter for the docShell property.
  *
  * @param connection DebuggerServerConnection
  *        The conection to the client.
  */
 function TabActor(connection) {
   this.conn = connection;
   this._tabActorPool = null;
   // A map of actor names to actor instances provided by extensions.
@@ -2125,134 +2114,49 @@ TabActor.prototype.requestTypes = {
   "listFrames": TabActor.prototype.onListFrames,
   "listWorkers": TabActor.prototype.onListWorkers,
   "resolveLocation": TabActor.prototype.onResolveLocation
 };
 
 exports.TabActor = TabActor;
 
 /**
- * Creates a tab actor for handling requests to a single in-process
- * <xul:browser> tab, or <html:iframe>.
- * Most of the implementation comes from TabActor.
+ * Creates a tab actor for handling requests to a single browser frame.
+ * Both <xul:browser> and <iframe mozbrowser> are supported.
+ * This actor is a shim that connects to a ContentActor in a remote browser process.
+ * All RDP packets get forwarded using the message manager.
  *
- * @param connection DebuggerServerConnection
- *        The connection to the client.
- * @param browser browser
- *        The frame instance that contains this tab.
+ * @param connection The main RDP connection.
+ * @param browser <xul:browser> or <iframe mozbrowser> element to connect to.
  */
 function BrowserTabActor(connection, browser) {
-  TabActor.call(this, connection, browser);
-  this._browser = browser;
-  if (typeof browser.getTabBrowser == "function") {
-    this._tabbrowser = browser.getTabBrowser();
-  }
-
-  Object.defineProperty(this, "docShell", {
-    value: this._browser.docShell,
-    configurable: true
-  });
-}
-
-BrowserTabActor.prototype = Object.create(TabActor.prototype);
-
-BrowserTabActor.prototype.constructor = BrowserTabActor;
-
-Object.defineProperty(BrowserTabActor.prototype, "title", {
-  get() {
-    // On Fennec, we can check the session store data for zombie tabs
-    if (this._browser.__SS_restore) {
-      let sessionStore = this._browser.__SS_data;
-      // Get the last selected entry
-      let entry = sessionStore.entries[sessionStore.index - 1];
-      return entry.title;
-    }
-    let title = this.contentDocument.title || this._browser.contentTitle;
-    // If contentTitle is empty (e.g. on a not-yet-restored tab), but there is a
-    // tabbrowser (i.e. desktop Firefox, but not Fennec), we can use the label
-    // as the title.
-    if (!title && this._tabbrowser) {
-      let tab = this._tabbrowser._getTabForContentWindow(this.window);
-      if (tab) {
-        title = tab.label;
-      }
-    }
-    return title;
-  },
-  enumerable: true,
-  configurable: false
-});
-
-Object.defineProperty(BrowserTabActor.prototype, "url", {
-  get() {
-    // On Fennec, we can check the session store data for zombie tabs
-    if (this._browser.__SS_restore) {
-      let sessionStore = this._browser.__SS_data;
-      // Get the last selected entry
-      let entry = sessionStore.entries[sessionStore.index - 1];
-      return entry.url;
-    }
-    if (this.webNavigation.currentURI) {
-      return this.webNavigation.currentURI.spec;
-    }
-    return null;
-  },
-  enumerable: true,
-  configurable: true
-});
-
-Object.defineProperty(BrowserTabActor.prototype, "browser", {
-  get() {
-    return this._browser;
-  },
-  enumerable: true,
-  configurable: false
-});
-
-BrowserTabActor.prototype.disconnect = function () {
-  TabActor.prototype.disconnect.call(this);
-  this._browser = null;
-  this._tabbrowser = null;
-};
-
-BrowserTabActor.prototype.exit = function () {
-  TabActor.prototype.exit.call(this);
-  this._browser = null;
-  this._tabbrowser = null;
-};
-
-exports.BrowserTabActor = BrowserTabActor;
-
-/**
- * This actor is a shim that connects to a ContentActor in a remote
- * browser process. All RDP packets get forwarded using the message
- * manager.
- *
- * @param connection The main RDP connection.
- * @param browser XUL <browser> element to connect to.
- */
-function RemoteBrowserTabActor(connection, browser) {
   this._conn = connection;
   this._browser = browser;
   this._form = null;
 }
 
-RemoteBrowserTabActor.prototype = {
+BrowserTabActor.prototype = {
   connect() {
     let onDestroy = () => {
       this._form = null;
     };
-    let connect = DebuggerServer.connectToChild(
-      this._conn, this._browser, onDestroy);
+    let connect = DebuggerServer.connectToChild(this._conn, this._browser, onDestroy);
     return connect.then(form => {
       this._form = form;
       return this;
     });
   },
 
+  get _tabbrowser() {
+    if (typeof this._browser.getTabBrowser == "function") {
+      return this._browser.getTabBrowser();
+    }
+    return null;
+  },
+
   get _mm() {
     // Get messageManager from XUL browser (which might be a specialized tunnel for RDM)
     // or else fallback to asking the frameLoader itself.
     return this._browser.messageManager ||
            this._browser.frameLoader.messageManager;
   },
 
   update() {
@@ -2273,26 +2177,75 @@ RemoteBrowserTabActor.prototype = {
       this._mm.addMessageListener("debug:form", onFormUpdate);
       this._mm.sendAsyncMessage("debug:form");
       return deferred.promise;
     }
 
     return this.connect();
   },
 
+  /**
+   * If we don't have a title from the content side because it's a zombie tab, try to find
+   * it on the chrome side.
+   */
+  get title() {
+    // On Fennec, we can check the session store data for zombie tabs
+    if (this._browser.__SS_restore) {
+      let sessionStore = this._browser.__SS_data;
+      // Get the last selected entry
+      let entry = sessionStore.entries[sessionStore.index - 1];
+      return entry.title;
+    }
+    // If contentTitle is empty (e.g. on a not-yet-restored tab), but there is a
+    // tabbrowser (i.e. desktop Firefox, but not Fennec), we can use the label
+    // as the title.
+    if (this._tabbrowser) {
+      let tab = this._tabbrowser.getTabForBrowser(this._browser);
+      if (tab) {
+        return tab.label;
+      }
+    }
+    return "";
+  },
+
+  /**
+   * If we don't have a url from the content side because it's a zombie tab, try to find
+   * it on the chrome side.
+   */
+  get url() {
+    // On Fennec, we can check the session store data for zombie tabs
+    if (this._browser.__SS_restore) {
+      let sessionStore = this._browser.__SS_data;
+      // Get the last selected entry
+      let entry = sessionStore.entries[sessionStore.index - 1];
+      return entry.url;
+    }
+    return null;
+  },
+
   form() {
-    return this._form;
+    let form = Object.assign({}, this._form);
+    // In some cases, the title and url fields might be empty.  Zombie tabs (not yet
+    // restored) are a good example.  In such cases, try to look up values for these
+    // fields using other data in the parent process.
+    if (!form.title) {
+      form.title = this.title;
+    }
+    if (!form.url) {
+      form.url = this.url;
+    }
+    return form;
   },
 
   exit() {
     this._browser = null;
   },
 };
 
-exports.RemoteBrowserTabActor = RemoteBrowserTabActor;
+exports.BrowserTabActor = BrowserTabActor;
 
 function BrowserAddonList(connection) {
   this._connection = connection;
   this._actorByAddonId = new Map();
   this._onListChanged = null;
 }
 
 BrowserAddonList.prototype.getList = function () {
--- a/devtools/server/child.js
+++ b/devtools/server/child.js
@@ -6,29 +6,35 @@
 
 /* global addMessageListener, removeMessageListener, sendAsyncMessage */
 
 try {
   var chromeGlobal = this;
 
   // Encapsulate in its own scope to allows loading this frame script more than once.
   (function () {
-    let Cu = Components.utils;
-    let { require } = Cu.import("resource://devtools/shared/Loader.jsm", {});
+    const Cu = Components.utils;
+    const { require } = Cu.import("resource://devtools/shared/Loader.jsm", {});
+
     const DevToolsUtils = require("devtools/shared/DevToolsUtils");
     const { dumpn } = DevToolsUtils;
     const { DebuggerServer, ActorPool } = require("devtools/server/main");
 
-    // Note that this frame script may be evaluated in non-e10s build. In such case,
-    // DebuggerServer is already going to be initialized.
     if (!DebuggerServer.initialized) {
       DebuggerServer.init();
-      DebuggerServer.isInChildProcess = true;
+      // For non-e10s mode, there is only one server instance, so be sure the browser
+      // actors get loaded.
+      DebuggerServer.addBrowserActors();
     }
 
+    // XXX: This flag is pretty confusing and has different meanings in different places.
+    // We should strive to use the message manager directly instead of trying to check the
+    // process via this flag.
+    DebuggerServer.isInChildProcess = true;
+
     // In case of apps being loaded in parent process, DebuggerServer is already
     // initialized, but child specific actors are not registered. Otherwise, for apps in
     // child process, we need to load actors the first time we load child.js.
     DebuggerServer.addChildActors();
 
     let connections = new Map();
 
     let onConnect = DevToolsUtils.makeInfallible(function (msg) {