Bug 1036324 - Adds option to walker.parents() to not traverse DocShellTreeItems of different types
authorBrian Grinstead <bgrinstead@mozilla.com>
Wed, 15 Apr 2015 13:49:00 +0200
changeset 240153 00574d3ed81d68819cfb739a3a30349c3badd968
parent 240152 8bcd9d38bb20446ed9296a9ee5b46c8080beb3c0
child 240154 a100719c003204c7b9c47d5b9dfec4bb3e6ce9ac
push id58752
push usercbook@mozilla.com
push dateTue, 21 Apr 2015 10:39:00 +0000
treeherdermozilla-inbound@b8d59286a581 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1036324
milestone40.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 1036324 - Adds option to walker.parents() to not traverse DocShellTreeItems of different types This option helps avoiding blank inspector situations when trying to use the right-click ctx menu in the browser to inspect an element *while* the page is reloading.
browser/devtools/markupview/markup-view.js
browser/devtools/markupview/test/browser.ini
browser/devtools/markupview/test/browser_markupview_load_01.js
browser/devtools/markupview/test/head.js
browser/devtools/shared/frame-script-utils.js
toolkit/devtools/server/actors/inspector.js
toolkit/devtools/server/actors/styles.js
--- a/browser/devtools/markupview/markup-view.js
+++ b/browser/devtools/markupview/markup-view.js
@@ -2141,17 +2141,17 @@ MarkupElementContainer.prototype = Herit
         data.data.string().then(str => {
           let res = {data: str, size: data.size};
           // Resolving the data promise and, to always keep tooltipData.data
           // as a promise, create a new one that resolves immediately
           def.resolve(res);
           this.tooltipData.data = promise.resolve(res);
         });
       }, () => {
-        this.tooltipData.data = promise.reject();
+        this.tooltipData.data = promise.resolve({});
       });
     }
   },
 
   /**
    * Executed by MarkupView._isImagePreviewTarget which is itself called when the
    * mouse hovers over a target in the markup-view.
    * Checks if the target is indeed something we want to have an image tooltip
@@ -2161,19 +2161,21 @@ MarkupElementContainer.prototype = Herit
    * to decide if/when to show the tooltip
    */
   isImagePreviewTarget: function(target, tooltip) {
     if (!this.tooltipData || this.tooltipData.target !== target) {
       return promise.reject();
     }
 
     return this.tooltipData.data.then(({data, size}) => {
-      tooltip.setImageContent(data, size);
-    }, () => {
-      tooltip.setBrokenImageContent();
+      if (data && size) {
+        tooltip.setImageContent(data, size);
+      } else {
+        tooltip.setBrokenImageContent();
+      }
     });
   },
 
   copyImageDataUri: function() {
     // We need to send again a request to gettooltipData even if one was sent for
     // the tooltip, because we want the full-size image
     this.node.getImageData().then(data => {
       data.data.string().then(str => {
--- a/browser/devtools/markupview/test/browser.ini
+++ b/browser/devtools/markupview/test/browser.ini
@@ -63,16 +63,17 @@ skip-if = e10s # Bug 1040751 - CodeMirro
 [browser_markupview_events_jquery_1.6.js]
 skip-if = e10s # Bug 1040751 - CodeMirror editor.destroy() isn't e10s compatible
 [browser_markupview_events_jquery_1.7.js]
 skip-if = e10s # Bug 1040751 - CodeMirror editor.destroy() isn't e10s compatible
 [browser_markupview_events_jquery_1.11.1.js]
 skip-if = e10s # Bug 1040751 - CodeMirror editor.destroy() isn't e10s compatible
 [browser_markupview_events_jquery_2.1.1.js]
 skip-if = e10s # Bug 1040751 - CodeMirror editor.destroy() isn't e10s compatible
+[browser_markupview_load_01.js]
 [browser_markupview_html_edit_01.js]
 [browser_markupview_html_edit_02.js]
 [browser_markupview_html_edit_03.js]
 [browser_markupview_image_tooltip.js]
 [browser_markupview_keybindings_01.js]
 [browser_markupview_keybindings_02.js]
 [browser_markupview_keybindings_03.js]
 [browser_markupview_mutation_01.js]
new file mode 100644
--- /dev/null
+++ b/browser/devtools/markupview/test/browser_markupview_load_01.js
@@ -0,0 +1,70 @@
+/* vim: set ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Tests that selecting an element with the 'Inspect Element' context
+// menu during a page reload doesn't cause the markup view to become empty.
+// See https://bugzilla.mozilla.org/show_bug.cgi?id=1036324
+
+const server = createTestHTTPServer();
+
+// Register a slow image handler so we can simulate a long time between
+// a reload and the load event firing.
+server.registerContentType("gif", "image/gif");
+server.registerPathHandler("/slow.gif", function (metadata, response) {
+  info ("Image has been requested");
+  response.processAsync();
+  setTimeout(() => {
+    info ("Image is responding");
+    response.finish();
+  }, 500);
+});
+
+// Test page load events.
+const TEST_URL = "data:text/html," +
+  "<!DOCTYPE html>" +
+  "<head><meta charset='utf-8' /></head>" +
+  "<body>" +
+  "<p>Slow script</p>" +
+  "<img src='http://localhost:" + server.identity.primaryPort + "/slow.gif' /></script>" +
+  "</body>" +
+  "</html>";
+
+add_task(function*() {
+  let tab = yield addTab(TEST_URL);
+  let {inspector} = yield openInspector();
+  let domContentLoaded = waitForLinkedBrowserEvent(tab, "DOMContentLoaded");
+  let pageLoaded = waitForLinkedBrowserEvent(tab, "load");
+
+  ok (inspector.markup, "There is a markup view");
+
+  // Select an element while the tab is in the middle of a slow reload.
+  reloadTab();
+  yield domContentLoaded;
+  yield chooseWithInspectElementContextMenu("img");
+  yield pageLoaded;
+
+  yield inspector.once("markuploaded");
+  ok (inspector.markup, "There is a markup view");
+  is (inspector.markup._elt.children.length, 1, "The markup view is rendering");
+});
+
+function* chooseWithInspectElementContextMenu(selector) {
+  yield executeInContent("Test:SynthesizeMouse", {
+    center: true,
+    selector: selector,
+    options: {type: "contextmenu", button: 2}
+  });
+  executeInContent("Test:SynthesizeKey", {key: "Q", options: {}});
+}
+
+function waitForLinkedBrowserEvent(tab, event) {
+  let def = promise.defer();
+  tab.linkedBrowser.addEventListener(event, function cb() {
+    tab.linkedBrowser.removeEventListener(event, cb, true);
+    def.resolve();
+  }, true);
+  return def.promise;
+}
--- a/browser/devtools/markupview/test/head.js
+++ b/browser/devtools/markupview/test/head.js
@@ -168,16 +168,23 @@ function executeInContent(name, data={},
   if (expectResponse) {
     return waitForContentMessage(name);
   } else {
     return promise.resolve();
   }
 }
 
 /**
+ * Reload the current tab location.
+ */
+function reloadTab() {
+  return executeInContent("devtools:test:reload", {}, {}, false);
+}
+
+/**
  * Simple DOM node accesor function that takes either a node or a string css
  * selector as argument and returns the corresponding node
  * @param {String|DOMNode} nodeOrSelector
  * @return {DOMNode|CPOW} Note that in e10s mode a CPOW object is returned which
  * doesn't implement *all* of the DOMNode's properties
  */
 function getNode(nodeOrSelector) {
   info("Getting the node for '" + nodeOrSelector + "'");
@@ -642,8 +649,39 @@ function* waitForMultipleChildrenUpdates
   // As long as child updates are queued up while we wait for an update already
   // wait again
   if (inspector.markup._queuedChildUpdates &&
       inspector.markup._queuedChildUpdates.size) {
     yield waitForChildrenUpdated(inspector);
     return yield waitForMultipleChildrenUpdates(inspector);
   }
 }
+
+/**
+ * Create an HTTP server that can be used to simulate custom requests within
+ * a test.  It is automatically cleaned up when the test ends, so no need to
+ * call `destroy`.
+ *
+ * See https://developer.mozilla.org/en-US/docs/Httpd.js/HTTP_server_for_unit_tests
+ * for more information about how to register handlers.
+ *
+ * The server can be accessed like:
+ *
+ *   const server = createTestHTTPServer();
+ *   let url = "http://localhost: " + server.identity.primaryPort + "/path";
+ *
+ * @returns {HttpServer}
+ */
+function createTestHTTPServer() {
+  const {HttpServer} = Cu.import("resource://testing-common/httpd.js", {});
+  let server = new HttpServer();
+
+  registerCleanupFunction(function* cleanup() {
+    let destroyed = promise.defer();
+    server.stop(() => {
+      destroyed.resolve();
+    });
+    yield destroyed.promise;
+  });
+
+  server.start(-1);
+  return server;
+}
--- a/browser/devtools/shared/frame-script-utils.js
+++ b/browser/devtools/shared/frame-script-utils.js
@@ -1,18 +1,21 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
-const Cu = Components.utils;
-
+const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
 const { devtools } = Cu.import("resource://gre/modules/devtools/Loader.jsm", {});
 devtools.lazyImporter(this, "promise", "resource://gre/modules/Promise.jsm", "Promise");
 devtools.lazyImporter(this, "Task", "resource://gre/modules/Task.jsm", "Task");
+const loader = Cc["@mozilla.org/moz/jssubscript-loader;1"]
+            .getService(Ci.mozIJSSubScriptLoader);
+let EventUtils = {};
+loader.loadSubScript("chrome://marionette/content/EventUtils.js", EventUtils);
 
 addMessageListener("devtools:test:history", function ({ data }) {
   content.history[data.direction]();
 });
 
 addMessageListener("devtools:test:navigate", function ({ data }) {
   content.location = data.location;
 });
@@ -185,16 +188,65 @@ addMessageListener("devtools:test:setAtt
   }
 
   node.setAttribute(attributeName, attributeValue);
 
   sendAsyncMessage("devtools:test:setAttribute");
 });
 
 /**
+ * Synthesize a mouse event on an element. This handler doesn't send a message
+ * back. Consumers should listen to specific events on the inspector/highlighter
+ * to know when the event got synthesized.
+ * @param {Object} msg The msg.data part expects the following properties:
+ * - {Number} x
+ * - {Number} y
+ * - {Boolean} center If set to true, x/y will be ignored and
+ *             synthesizeMouseAtCenter will be used instead
+ * - {Object} options Other event options
+ * - {String} selector An optional selector that will be used to find the node to
+ *            synthesize the event on, if msg.objects doesn't contain the CPOW.
+ * The msg.objects part should be the element.
+ * @param {Object} data Event detail properties:
+ */
+addMessageListener("Test:SynthesizeMouse", function(msg) {
+  let {x, y, center, options, selector} = msg.data;
+  let {node} = msg.objects;
+
+  if (!node && selector) {
+    node = superQuerySelector(selector);
+  }
+
+  if (center) {
+    EventUtils.synthesizeMouseAtCenter(node, options, node.ownerDocument.defaultView);
+  } else {
+    EventUtils.synthesizeMouse(node, x, y, options, node.ownerDocument.defaultView);
+  }
+
+  // Most consumers won't need to listen to this message, unless they want to
+  // wait for the mouse event to be synthesized and don't have another event
+  // to listen to instead.
+  sendAsyncMessage("Test:SynthesizeMouse");
+});
+
+/**
+ * Synthesize a key event for an element. This handler doesn't send a message
+ * back. Consumers should listen to specific events on the inspector/highlighter
+ * to know when the event got synthesized.
+ * @param  {Object} msg The msg.data part expects the following properties:
+ * - {String} key
+ * - {Object} options
+ */
+addMessageListener("Test:SynthesizeKey", function(msg) {
+  let {key, options} = msg.data;
+
+  EventUtils.synthesizeKey(key, options, content);
+});
+
+/**
  * Like document.querySelector but can go into iframes too.
  * ".container iframe || .sub-container div" will first try to find the node
  * matched by ".container iframe" in the root document, then try to get the
  * content document inside it, and then try to match ".sub-container div" inside
  * this document.
  * Any selector coming before the || separator *MUST* match a frame node.
  * @param {String} superSelector.
  * @return {DOMNode} The node, or null if not found.
--- a/toolkit/devtools/server/actors/inspector.js
+++ b/toolkit/devtools/server/actors/inspector.js
@@ -1405,36 +1405,45 @@ var WalkerActor = protocol.ActorClass({
    * Return all parents of the given node, ordered from immediate parent
    * to root.
    * @param NodeActor node
    *    The node whose parents are requested.
    * @param object options
    *    Named options, including:
    *    `sameDocument`: If true, parents will be restricted to the same
    *      document as the node.
+   *    `sameTypeRootTreeItem`: If true, this will not traverse across
+   *     different types of docshells.
    */
   parents: method(function(node, options={}) {
     if (isNodeDead(node)) {
       return [];
     }
 
     let walker = this.getDocumentWalker(node.rawNode);
     let parents = [];
     let cur;
     while((cur = walker.parentNode())) {
-      if (options.sameDocument && cur.ownerDocument != node.rawNode.ownerDocument) {
+      if (options.sameDocument && nodeDocument(cur) != nodeDocument(node.rawNode)) {
         break;
       }
+
+      if (options.sameTypeRootTreeItem &&
+          nodeDocshell(cur).sameTypeRootTreeItem != nodeDocshell(node.rawNode).sameTypeRootTreeItem) {
+        break;
+      }
+
       parents.push(this._ref(cur));
     }
     return parents;
   }, {
     request: {
       node: Arg(0, "domnode"),
-      sameDocument: Option(1)
+      sameDocument: Option(1),
+      sameTypeRootTreeItem: Option(1)
     },
     response: {
       nodes: RetVal("array:domnode")
     },
   }),
 
   parentNode: function(node) {
     let walker = this.getDocumentWalker(node.rawNode);
@@ -3221,17 +3230,17 @@ var WalkerFront = exports.WalkerFront = 
       throw Error("Could not find client side for actor " + this.actorID);
     }
     let nodeActor = walkerActor._ref(rawNode);
 
     // Pass the node through a read/write pair to create the client side actor.
     let nodeType = types.getType("domnode");
     let returnNode = nodeType.read(nodeType.write(nodeActor, walkerActor), this);
     let top = returnNode;
-    let extras = walkerActor.parents(nodeActor);
+    let extras = walkerActor.parents(nodeActor, {sameTypeRootTreeItem: true});
     for (let extraActor of extras) {
       top = nodeType.read(nodeType.write(extraActor, walkerActor), this);
     }
 
     if (top !== this.rootNode) {
       // Imported an already-orphaned node.
       this._orphaned.add(top);
       walkerActor._orphaned.add(this.conn._transport._serverConnection.getActor(top.actorID));
@@ -3514,16 +3523,26 @@ exports._documentWalker = DocumentWalker
 
 function nodeDocument(node) {
   if (Cu.isDeadWrapper(node)) {
     return null;
   }
   return node.ownerDocument || (node.nodeType == Ci.nsIDOMNode.DOCUMENT_NODE ? node : null);
 }
 
+function nodeDocshell(node) {
+  let doc = node ? nodeDocument(node) : null;
+  let win = doc ? doc.defaultView : null;
+  if (win) {
+    return win.
+           QueryInterface(Ci.nsIInterfaceRequestor).
+           getInterface(Ci.nsIDocShell);
+  }
+}
+
 function isNodeDead(node) {
   return !node || !node.rawNode || Cu.isDeadWrapper(node.rawNode);
 }
 
 /**
  * Wrapper for inDeepTreeWalker.  Adds filtering to the traversal methods.
  * See inDeepTreeWalker for more information about the methods.
  *
--- a/toolkit/devtools/server/actors/styles.js
+++ b/toolkit/devtools/server/actors/styles.js
@@ -1015,17 +1015,18 @@ var StyleRuleActor = protocol.ActorClass
       case Ci.nsIDOMCSSRule.STYLE_RULE:
         form.selectors = CssLogic.getSelectors(this.rawRule);
         form.cssText = this.rawStyle.cssText || "";
         break;
       case ELEMENT_STYLE:
         // Elements don't have a parent stylesheet, and therefore
         // don't have an associated URI.  Provide a URI for
         // those.
-        form.href = this.rawNode.ownerDocument.location.href;
+        let doc = this.rawNode.ownerDocument;
+        form.href = doc.location ? doc.location.href : "";
         form.cssText = this.rawStyle.cssText || "";
         break;
       case Ci.nsIDOMCSSRule.CHARSET_RULE:
         form.encoding = this.rawRule.encoding;
         break;
       case Ci.nsIDOMCSSRule.IMPORT_RULE:
         form.href = this.rawRule.href;
         break;
@@ -1226,17 +1227,17 @@ var StyleRuleFront = protocol.FrontClass
     return this.conn.getActor(this._form.element);
   },
 
   get href() {
     if (this._form.href) {
       return this._form.href;
     }
     let sheet = this.parentStyleSheet;
-    return sheet.href;
+    return sheet ? sheet.href : "";
   },
 
   get nodeHref() {
     let sheet = this.parentStyleSheet;
     return sheet ? sheet.nodeHref : "";
   },
 
   get location()