Bug 1260359 - fix devtools detached toolbox title update in e10s;r=jryans draft
authorJulian Descottes <jdescottes@mozilla.com>
Tue, 29 Mar 2016 12:01:46 +0200
changeset 345799 720084e9929f334092ffe18e4c786cbf731fe52e
parent 345609 8edeb8f0ca56598918f4e5f6b018d5bb989cf133
child 517271 70672f6f18064b7b39b8d7e3fa454a12dd2ecb48
push id14177
push userjdescottes@mozilla.com
push dateWed, 30 Mar 2016 12:58:00 +0000
reviewersjryans
bugs1260359
milestone48.0a1
Bug 1260359 - fix devtools detached toolbox title update in e10s;r=jryans The devtools host window updates its title when its target navigates. This was done using a contentDocument CPOW, and thus failing on e10s. The url and title are now stored in TabTarget and updated on tab navigation. Updated existing test to cover the case of pages with a title. MozReview-Commit-ID: 4G1keOA7yB6
devtools/client/framework/target.js
devtools/client/framework/test/browser.ini
devtools/client/framework/test/browser_toolbox_window_title_changes.js
devtools/client/framework/test/browser_toolbox_window_title_changes_page.html
--- a/devtools/client/framework/target.js
+++ b/devtools/client/framework/target.js
@@ -123,16 +123,19 @@ function TabTarget(tab) {
   this.activeTab = this.activeConsole = null;
   // Only real tabs need initialization here. Placeholder objects for remote
   // targets will be initialized after a makeRemote method call.
   if (tab && !["client", "form", "chrome"].every(tab.hasOwnProperty, tab)) {
     this._tab = tab;
     this._setupListeners();
   } else {
     this._form = tab.form;
+    this._url = this._form.url;
+    this._title = this._form.title;
+
     this._client = tab.client;
     this._chrome = tab.chrome;
   }
   // Default isTabActor to true if not explicitly specified
   if (typeof tab.isTabActor == "boolean") {
     this._isTabActor = tab.isTabActor;
   } else {
     this._isTabActor = true;
@@ -326,28 +329,24 @@ TabTarget.prototype = {
     // during shutdown.
     if (this._tab && this._tab.linkedBrowser) {
       return this._tab.linkedBrowser.contentWindow;
     }
     return null;
   },
 
   get name() {
-    if (this._tab && this._tab.linkedBrowser.contentDocument) {
-      return this._tab.linkedBrowser.contentDocument.title;
-    }
     if (this.isAddon) {
       return this._form.name;
     }
-    return this._form.title;
+    return this._title;
   },
 
   get url() {
-    return this._tab ? this._tab.linkedBrowser.currentURI.spec :
-                       this._form.url;
+    return this._url;
   },
 
   get isRemote() {
     return !this.isLocalTab;
   },
 
   get isAddon() {
     return !!(this._form && this._form.actor &&
@@ -417,16 +416,19 @@ TabTarget.prototype = {
                                  onConsoleAttached);
     };
 
     if (this.isLocalTab) {
       this._client.connect()
         .then(() => this._client.getTab({ tab: this.tab }))
         .then(response => {
           this._form = response.tab;
+          this._url = this._form.url;
+          this._title = this._form.title;
+
           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 {
       // AddonActor and chrome debugging on RootActor doesn't inherits from
@@ -471,21 +473,26 @@ TabTarget.prototype = {
       // We have to filter message to ensure that this detach is for this tab
       if (aPacket.from == this._form.actor) {
         this.destroy();
       }
     };
     this.client.addListener("tabDetached", this._onTabDetached);
 
     this._onTabNavigated = (aType, aPacket) => {
+      // Update the title and url on tabNavigated event.
+      this._url = aPacket.url;
+      this._title = aPacket.title;
+
       let event = Object.create(null);
       event.url = aPacket.url;
       event.title = aPacket.title;
       event.nativeConsoleAPI = aPacket.nativeConsoleAPI;
       event.isFrameSwitching = aPacket.isFrameSwitching;
+
       // Send any stored event payload (DOMWindow or nsIRequest) for backwards
       // compatibility with non-remotable tools.
       if (aPacket.state == "start") {
         event._navPayload = this._navRequest;
         this.emit("will-navigate", event);
         this._navRequest = null;
       } else {
         event._navPayload = this._navWindow;
--- a/devtools/client/framework/test/browser.ini
+++ b/devtools/client/framework/test/browser.ini
@@ -1,16 +1,17 @@
 [DEFAULT]
 tags = devtools
 subsuite = devtools
 support-files =
   browser_toolbox_options_disable_js.html
   browser_toolbox_options_disable_js_iframe.html
   browser_toolbox_options_disable_cache.sjs
   browser_toolbox_sidebar_tool.xul
+  browser_toolbox_window_title_changes_page.html
   code_math.js
   code_ugly.js
   head.js
   shared-head.js
   shared-redux-head.js
   helper_disable_cache.js
   doc_theme.css
   doc_viewsource.html
--- a/devtools/client/framework/test/browser_toolbox_window_title_changes.js
+++ b/devtools/client/framework/test/browser_toolbox_window_title_changes.js
@@ -3,16 +3,18 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 var {Toolbox} = require("devtools/client/framework/toolbox");
 
 function test() {
   const URL_1 = "data:text/plain;charset=UTF-8,abcde";
   const URL_2 = "data:text/plain;charset=UTF-8,12345";
+  const URL_3 = URL_ROOT + "browser_toolbox_window_title_changes_page.html";
+  const TITLE_URL_3 = "Toolbox test for title update";
 
   const TOOL_ID_1 = "webconsole";
   const TOOL_ID_2 = "jsdebugger";
 
   const LABEL_1 = "Console";
   const LABEL_2 = "Debugger";
 
   let toolbox;
@@ -26,40 +28,49 @@ function test() {
     // undock toolbox and check title
       .then(() => toolbox.switchHost(Toolbox.HostType.WINDOW))
       .then(checkTitle.bind(null, LABEL_1, URL_1, "toolbox undocked"))
 
     // switch to different tool and check title
       .then(() => toolbox.selectTool(TOOL_ID_2))
       .then(checkTitle.bind(null, LABEL_2, URL_1, "tool changed"))
 
-    // navigate to different url and check title
+    // navigate to different local url and check title
       .then(function () {
         let deferred = promise.defer();
         target.once("navigate", () => deferred.resolve());
         gBrowser.loadURI(URL_2);
         return deferred.promise;
       })
       .then(checkTitle.bind(null, LABEL_2, URL_2, "url changed"))
 
+    // navigate to a real url and check title
+      .then(() => {
+        let deferred = promise.defer();
+        target.once("navigate", () => deferred.resolve());
+        gBrowser.loadURI(URL_3);
+        return deferred.promise;
+      })
+      .then(checkTitle.bind(null, LABEL_2, TITLE_URL_3, "url changed"))
+
     // destroy toolbox, create new one hosted in a window (with a
     // different tool id), and check title
       .then(function () {
         // Give the tools a chance to handle the navigation event before
         // destroying the toolbox.
         executeSoon(function() {
           toolbox.destroy()
             .then(function () {
               // After destroying the toolbox, a fresh target is required.
               target = TargetFactory.forTab(gBrowser.selectedTab);
               return gDevTools.showToolbox(target, null, Toolbox.HostType.WINDOW);
             })
             .then(function (aToolbox) { toolbox = aToolbox; })
             .then(() => toolbox.selectTool(TOOL_ID_1))
-            .then(checkTitle.bind(null, LABEL_1, URL_2,
+            .then(checkTitle.bind(null, LABEL_1, TITLE_URL_3,
                                   "toolbox destroyed and recreated"))
 
             // clean up
             .then(() => toolbox.destroy())
             .then(function () {
               toolbox = null;
               gBrowser.removeCurrentTab();
               Services.prefs.clearUserPref("devtools.toolbox.host");
new file mode 100644
--- /dev/null
+++ b/devtools/client/framework/test/browser_toolbox_window_title_changes_page.html
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML>
+<html>
+  <head>
+    <meta charset="UTF-8">
+    <title>Toolbox test for title update</title>
+    <!-- Any copyright is dedicated to the Public Domain.
+         http://creativecommons.org/publicdomain/zero/1.0/ -->
+  </head>
+  <body></body>
+</html>