Bug 1563684 - Fix blank console on GMail. r=yulia a=RyanVM
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Mon, 08 Jul 2019 12:34:20 +0000
changeset 544470 0aa0865a4011004e2966c14b9cecc5991b584b3d
parent 544469 844f93fee80599a6521daef9391e438841b6c5e2
child 544471 36dd2e9d0477df07490746bae9a39029925dc568
push id2131
push userffxbld-merge
push dateMon, 26 Aug 2019 18:30:20 +0000
treeherdermozilla-release@b19ffb3ca153 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyulia, RyanVM
bugs1563684
milestone69.0
Bug 1563684 - Fix blank console on GMail. r=yulia a=RyanVM The console fails to connect to the server because the getCachedMessages function throws on GMail. This is because we try to access a property on a cross-origin object, window.windowUtils, in getInnerWindowId. Wrapping the access to the property fixes the issue. A test is added to make sure we don't regress. // TODO: The test isn't failing without the fix, so it should be re-written. Differential Revision: https://phabricator.services.mozilla.com/D37069
devtools/client/webconsole/test/mochitest/browser.ini
devtools/client/webconsole/test/mochitest/browser_webconsole_cached_messages_cross_domain_iframe.js
devtools/client/webconsole/test/mochitest/test-iframe-parent.html
devtools/server/actors/webconsole/utils.js
--- a/devtools/client/webconsole/test/mochitest/browser.ini
+++ b/devtools/client/webconsole/test/mochitest/browser.ini
@@ -251,16 +251,17 @@ tags = clipboard
 tags = clipboard
 [browser_jsterm_syntax_highlight_output.js]
 skip-if = (os == "win" && processor == "aarch64") # disabled on aarch64 due to 1531574
 [browser_webconsole_allow_mixedcontent_securityerrors.js]
 tags = mcb
 [browser_webconsole_batching.js]
 [browser_webconsole_block_mixedcontent_securityerrors.js]
 tags = mcb
+[browser_webconsole_cached_messages_cross_domain_iframe.js]
 [browser_webconsole_cached_messages.js]
 [browser_webconsole_cd_iframe.js]
 [browser_webconsole_certificate_messages.js]
 [browser_webconsole_clear_cache.js]
 [browser_webconsole_click_function_to_source.js]
 [browser_webconsole_clickable_urls.js]
 [browser_webconsole_close_unfocused_window.js]
 [browser_webconsole_closing_after_completion.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/webconsole/test/mochitest/browser_webconsole_cached_messages_cross_domain_iframe.js
@@ -0,0 +1,30 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test to see if retrieving cached messages in a page with a cross-domain iframe does
+// not crash the console.
+
+"use strict";
+
+const TEST_URI =
+  "http://example.com/browser/devtools/client/webconsole/" +
+  "test/mochitest/test-iframe-parent.html";
+
+add_task(async function() {
+  // test-iframe-parent has an iframe pointing to http://mochi.test:8888/browser/devtools/client/webconsole/test/mochitest/test-iframe-child.html
+  info("Open the tab first");
+  await addTab(TEST_URI);
+
+  info("Evaluate an expression that will throw, so we'll have cached messages");
+  await ContentTask.spawn(gBrowser.selectedBrowser, null, function() {
+    content.wrappedJSObject.document.querySelector("button").click();
+  });
+
+  info("Then open the console, to retrieve cached messages");
+  await openConsole();
+
+  // TODO: Make the test fail without the fix.
+  ok(true, "Everything is okay");
+});
--- a/devtools/client/webconsole/test/mochitest/test-iframe-parent.html
+++ b/devtools/client/webconsole/test/mochitest/test-iframe-parent.html
@@ -4,10 +4,21 @@
 <html lang="en">
   <head>
     <meta charset="utf-8">
     <title>test for bug 989025 - iframe parent</title>
   </head>
   <body>
     <p>test for bug 989025 - iframe parent</p>
     <iframe src="http://mochi.test:8888/browser/devtools/client/webconsole/test/mochitest/test-iframe-child.html"></iframe>
+    <button>Throw Error</button>
+    <script>
+        "use strict";
+
+        /* exported throwError */
+        function throwError() {
+            throwError.asdf();
+        }
+
+        document.querySelector("button").addEventListener("click", throwError);
+        </script>
   </body>
 </html>
--- a/devtools/server/actors/webconsole/utils.js
+++ b/devtools/server/actors/webconsole/utils.js
@@ -85,33 +85,43 @@ var WebConsoleUtils = {
 
     return temp;
   },
 
   /**
    * Gets the ID of the inner window of this DOM window.
    *
    * @param nsIDOMWindow window
-   * @return integer
-   *         Inner ID for the given window.
+   * @return integer|null
+   *         Inner ID for the given window, null if we can't access it.
    */
   getInnerWindowId: function(window) {
-    return window.windowUtils.currentInnerWindowID;
+    // Might throw with SecurityError: Permission denied to access property "windowUtils"
+    // on cross-origin object.
+    try {
+      return window.windowUtils.currentInnerWindowID;
+    } catch (e) {
+      return null;
+    }
   },
 
   /**
    * Recursively gather a list of inner window ids given a
    * top level window.
    *
    * @param nsIDOMWindow window
    * @return Array
    *         list of inner window ids.
    */
   getInnerWindowIDsForFrames: function(window) {
     const innerWindowID = this.getInnerWindowId(window);
+    if (innerWindowID === null) {
+      return [];
+    }
+
     let ids = [innerWindowID];
 
     if (window.frames) {
       for (let i = 0; i < window.frames.length; i++) {
         const frame = window.frames[i];
         ids = ids.concat(this.getInnerWindowIDsForFrames(frame));
       }
     }