Bug 1485676 - Adapt gDevTools API to new async forTab and also adapt its callsites. r=yulia
☠☠ backed out by 99b4f09fa32c ☠ ☠
authorAlexandre Poirot <poirot.alex@gmail.com>
Wed, 29 Aug 2018 06:14:38 -0700
changeset 493537 a83636fab16af5537d168c004f812633722e83f4
parent 493536 b1fd24929e093182025b28375a903f61bb821903
child 493538 f9ef30ae3f7f21cc8e92f04575a8d9be0bc9c283
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyulia
bugs1485676
milestone64.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 1485676 - Adapt gDevTools API to new async forTab and also adapt its callsites. r=yulia Summary: When switching to async, it is important to catch exception or register a rejection handler so that errors keep being logged. So in this patch I'm catching exception in a couple of important codepath. Depends On D4541 Reviewers: yulia! Tags: #secure-revision Bug #: 1485676 Differential Revision: https://phabricator.services.mozilla.com/D4542 MozReview-Commit-ID: IDPJVkAPbTs
devtools/client/framework/browser-menus.js
devtools/client/framework/devtools-browser.js
devtools/client/menus.js
devtools/startup/devtools-startup.js
--- a/devtools/client/framework/browser-menus.js
+++ b/devtools/client/framework/browser-menus.js
@@ -73,21 +73,25 @@ function createToolMenuElements(toolDefi
   const id = toolDefinition.id;
   const menuId = "menuitem_" + id;
 
   // Prevent multiple entries for the same tool.
   if (doc.getElementById(menuId)) {
     return;
   }
 
-  const oncommand = function(id, event) {
-    const window = event.target.ownerDocument.defaultView;
-    gDevToolsBrowser.selectToolCommand(window.gBrowser, id, Cu.now());
-    sendEntryPointTelemetry();
-  }.bind(null, id);
+  const oncommand = (async function(id, event) {
+    try {
+      const window = event.target.ownerDocument.defaultView;
+      await gDevToolsBrowser.selectToolCommand(window.gBrowser, id, Cu.now());
+      sendEntryPointTelemetry();
+    } catch (e) {
+      console.error(`Exception while opening ${id}: ${e}\n${e.stack}`);
+    }
+  }).bind(null, id);
 
   const menuitem = createMenuItem({
     doc,
     id: "menuitem_" + id,
     label: toolDefinition.menuLabel || toolDefinition.label,
     accesskey: toolDefinition.accesskey
   });
   // Refer to the key in order to display the key shortcut at menu ends
--- a/devtools/client/framework/devtools-browser.js
+++ b/devtools/client/framework/devtools-browser.js
@@ -53,18 +53,18 @@ var gDevToolsBrowser = exports.gDevTools
   _browserStyleSheets: new WeakMap(),
 
   /**
    * This function is for the benefit of Tools:DevToolbox in
    * browser/base/content/browser-sets.inc and should not be used outside
    * of there
    */
   // used by browser-sets.inc, command
-  toggleToolboxCommand(gBrowser, startTime) {
-    const target = TargetFactory.forTab(gBrowser.selectedTab);
+  async toggleToolboxCommand(gBrowser, startTime) {
+    const target = await TargetFactory.forTab(gBrowser.selectedTab);
     const toolbox = gDevTools.getToolbox(target);
 
     // If a toolbox exists, using toggle from the Main window :
     // - should close a docked toolbox
     // - should focus a windowed toolbox
     const isDocked = toolbox && toolbox.hostType != Toolbox.HostType.WINDOW;
     if (isDocked) {
       gDevTools.closeToolbox(target);
@@ -197,18 +197,18 @@ var gDevToolsBrowser = exports.gDevTools
    *   we select it
    * - if the toolbox is open, and the targeted tool is selected,
    *   and the host is NOT a window, we close the toolbox
    * - if the toolbox is open, and the targeted tool is selected,
    *   and the host is a window, we raise the toolbox window
    */
   // Used when: - registering a new tool
   //            - new xul window, to add menu items
-  selectToolCommand(gBrowser, toolId, startTime) {
-    const target = TargetFactory.forTab(gBrowser.selectedTab);
+  async selectToolCommand(gBrowser, toolId, startTime) {
+    const target = await TargetFactory.forTab(gBrowser.selectedTab);
     const toolbox = gDevTools.getToolbox(target);
     const toolDefinition = gDevTools.getToolDefinition(toolId);
 
     if (toolbox &&
         (toolbox.currentToolId == toolId ||
           (toolId == "webconsole" && toolbox.splitConsole))) {
       toolbox.fireCustomKey(toolId);
 
@@ -238,27 +238,27 @@ var gDevToolsBrowser = exports.gDevTools
    *         are:
    *         - `toolId` used to identify a toolbox's panel like inspector or webconsole,
    *         - `id` used to identify any other key shortcuts like scratchpad or
    *         about:debugging
    * @param {Number} startTime
    *        Optional, indicates the time at which the key event fired. This is a
    *        `Cu.now()` timing.
    */
-  onKeyShortcut(window, key, startTime) {
+  async onKeyShortcut(window, key, startTime) {
     // If this is a toolbox's panel key shortcut, delegate to selectToolCommand
     if (key.toolId) {
-      gDevToolsBrowser.selectToolCommand(window.gBrowser, key.toolId, startTime);
+      await gDevToolsBrowser.selectToolCommand(window.gBrowser, key.toolId, startTime);
       return;
     }
     // Otherwise implement all other key shortcuts individually here
     switch (key.id) {
       case "toggleToolbox":
       case "toggleToolboxF12":
-        gDevToolsBrowser.toggleToolboxCommand(window.gBrowser, startTime);
+        await gDevToolsBrowser.toggleToolboxCommand(window.gBrowser, startTime);
         break;
       case "webide":
         gDevToolsBrowser.openWebIDE();
         break;
       case "browserToolbox":
         BrowserToolboxProcess.init();
         break;
       case "browserConsole":
@@ -269,17 +269,17 @@ var gDevToolsBrowser = exports.gDevTools
         ResponsiveUIManager.toggle(window, window.gBrowser.selectedTab, {
           trigger: "shortcut"
         });
         break;
       case "scratchpad":
         ScratchpadManager.openScratchpad();
         break;
       case "inspectorMac":
-        gDevToolsBrowser.selectToolCommand(window.gBrowser, "inspector", startTime);
+        await gDevToolsBrowser.selectToolCommand(window.gBrowser, "inspector", startTime);
         break;
     }
   },
 
   /**
    * Open a tab on "about:debugging", optionally pre-select a given tab.
    */
    // Used by browser-sets.inc, command
--- a/devtools/client/menus.js
+++ b/devtools/client/menus.js
@@ -36,19 +36,23 @@ loader.lazyRequireGetter(this, "Responsi
 loader.lazyRequireGetter(this, "openDocLink", "devtools/client/shared/link", true);
 
 loader.lazyImporter(this, "BrowserToolboxProcess", "resource://devtools/client/framework/ToolboxProcess.jsm");
 loader.lazyImporter(this, "ScratchpadManager", "resource://devtools/client/scratchpad/scratchpad-manager.jsm");
 
 exports.menuitems = [
   { id: "menu_devToolbox",
     l10nKey: "devToolboxMenuItem",
-    oncommand(event) {
-      const window = event.target.ownerDocument.defaultView;
-      gDevToolsBrowser.toggleToolboxCommand(window.gBrowser, Cu.now());
+    async oncommand(event) {
+      try {
+        const window = event.target.ownerDocument.defaultView;
+        await gDevToolsBrowser.toggleToolboxCommand(window.gBrowser, Cu.now());
+      } catch (e) {
+        console.error(`Exception while opening the toolbox: ${e}\n${e.stack}`);
+      }
     },
     keyId: "toggleToolbox",
     checkbox: true
   },
   { id: "menu_devtools_separator",
     separator: true },
   { id: "menu_webide",
     l10nKey: "webide",
--- a/devtools/startup/devtools-startup.js
+++ b/devtools/startup/devtools-startup.js
@@ -577,28 +577,32 @@ DevToolsStartup.prototype = {
 
     // Appending a <key> element is not always enough. The <keyset> needs
     // to be detached and reattached to make sure the <key> is taken into
     // account (see bug 832984).
     const mainKeyset = doc.getElementById("mainKeyset");
     mainKeyset.parentNode.insertBefore(keyset, mainKeyset);
   },
 
-  onKey(window, key) {
-    if (!Services.prefs.getBoolPref(DEVTOOLS_ENABLED_PREF)) {
-      const id = key.toolId || key.id;
-      this.openInstallPage("KeyShortcut", id);
-    } else {
-      // Record the timing at which this event started in order to compute later in
-      // gDevTools.showToolbox, the complete time it takes to open the toolbox.
-      // i.e. especially take `initDevTools` into account.
-      const startTime = Cu.now();
-      const require = this.initDevTools("KeyShortcut", key);
-      const { gDevToolsBrowser } = require("devtools/client/framework/devtools-browser");
-      gDevToolsBrowser.onKeyShortcut(window, key, startTime);
+  async onKey(window, key) {
+    try {
+      if (!Services.prefs.getBoolPref(DEVTOOLS_ENABLED_PREF)) {
+        const id = key.toolId || key.id;
+        this.openInstallPage("KeyShortcut", id);
+      } else {
+        // Record the timing at which this event started in order to compute later in
+        // gDevTools.showToolbox, the complete time it takes to open the toolbox.
+        // i.e. especially take `initDevTools` into account.
+        const startTime = Cu.now();
+        const require = this.initDevTools("KeyShortcut", key);
+        const { gDevToolsBrowser } = require("devtools/client/framework/devtools-browser");
+        await gDevToolsBrowser.onKeyShortcut(window, key, startTime);
+      }
+    } catch (e) {
+      console.error(`Exception while trigerring key ${key}: ${e}\n${e.stack}`);
     }
   },
 
   // Create a <xul:key> DOM Element
   createKey(doc, { id, toolId, shortcut, modifiers: mod }, oncommand) {
     const k = doc.createXULElement("key");
     k.id = "key_" + (id || toolId);