Bug 1510690 - Do focus the content window from the actor when the toolbox closes. r=nchevobbe
authorAlexandre Poirot <poirot.alex@gmail.com>
Wed, 31 Jul 2019 08:12:16 +0000
changeset 485462 27ac681ded07d7b70ac14634a23e1aba9d0d6ddf
parent 485461 2a53b590e435e4d2667444b1dca4507afa9990eb
child 485463 150bf969aea0ec4434daccd86c52cc96f0f3c02a
push id91313
push userapoirot@mozilla.com
push dateWed, 31 Jul 2019 08:15:16 +0000
treeherderautoland@150bf969aea0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnchevobbe
bugs1510690
milestone70.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 1510690 - Do focus the content window from the actor when the toolbox closes. r=nchevobbe No RDP request should be done when the toolbox closes as there no guarantee that the request will complete. Instead, such cleanup should be done by the actors. Differential Revision: https://phabricator.services.mozilla.com/D38309
devtools/client/framework/toolbox.js
devtools/client/webconsole/test/mochitest/browser_webconsole_document_focus.js
devtools/client/webconsole/webconsole.js
devtools/server/actors/targets/browsing-context.js
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -758,16 +758,25 @@ Toolbox.prototype = {
           null,
           "splitconsole",
           false
         );
       }
 
       await promise.all([splitConsolePromise, framesPromise]);
 
+      // Request the actor to restore the focus to the content page once the
+      // target is detached. This typically happens when the console closes.
+      // We restore the focus as it may have been stolen by the console input.
+      await this.target.reconfigure({
+        options: {
+          restoreFocus: true,
+        },
+      });
+
       // Lazily connect to the profiler here and don't wait for it to complete,
       // used to intercept console.profile calls before the performance tools are open.
       const performanceFrontConnection = this.initPerformance();
 
       // If in testing environment, wait for performance connection to finish,
       // so we don't have to explicitly wait for this in tests; ideally, all tests
       // will handle this on their own, but each have their own tear down function.
       if (flags.testing) {
--- a/devtools/client/webconsole/test/mochitest/browser_webconsole_document_focus.js
+++ b/devtools/client/webconsole/test/mochitest/browser_webconsole_document_focus.js
@@ -8,19 +8,27 @@ const TEST_URI =
   "data:text/html;charset=utf-8,Test content focus after closing console";
 
 add_task(async function() {
   const hud = await openNewTabAndConsole(TEST_URI);
 
   info("Focus after console is opened");
   ok(isInputFocused(hud), "input node is focused after console is opened");
 
+  await ContentTask.spawn(gBrowser.selectedBrowser, {}, function() {
+    this.onFocus = new Promise(resolve => {
+      content.addEventListener("focus", resolve, { once: true });
+    });
+  });
+
   info("Closing console");
   await closeConsole();
+
   const isFocused = await ContentTask.spawn(
     gBrowser.selectedBrowser,
     {},
-    function() {
+    async function() {
+      await this.onFocus;
       return Services.focus.focusedWindow == content;
     }
   );
   ok(isFocused, "content document has focus after closing the console");
 });
--- a/devtools/client/webconsole/webconsole.js
+++ b/devtools/client/webconsole/webconsole.js
@@ -381,24 +381,16 @@ class WebConsole {
 
     this._destroyer = (async () => {
       this.hudService.consoles.delete(this.hudId);
 
       if (this.ui) {
         this.ui.destroy();
       }
 
-      if (!this.isBrowserConsole) {
-        try {
-          await this.target.focus();
-        } catch (ex) {
-          // Tab focus can fail if the tab or target is closed.
-        }
-      }
-
       if (this._parserService) {
         this._parserService.stop();
         this._parserService = null;
       }
 
       const id = Utils.supportsString(this.hudId);
       Services.obs.notifyObservers(id, "web-console-destroyed");
     })();
--- a/devtools/server/actors/targets/browsing-context.js
+++ b/devtools/server/actors/targets/browsing-context.js
@@ -1130,16 +1130,19 @@ const browsingContextTargetPrototype = {
       typeof options.serviceWorkersTestingEnabled !== "undefined" &&
       options.serviceWorkersTestingEnabled !==
         this._getServiceWorkersTestingEnabled()
     ) {
       this._setServiceWorkersTestingEnabled(
         options.serviceWorkersTestingEnabled
       );
     }
+    if (typeof options.restoreFocus == "boolean") {
+      this._restoreFocus = options.restoreFocus;
+    }
 
     // Reload if:
     //  - there's an explicit `performReload` flag and it's true
     //  - there's no `performReload` flag, but it makes sense to do so
     const hasExplicitReloadFlag = "performReload" in options;
     if (
       (hasExplicitReloadFlag && options.performReload) ||
       (!hasExplicitReloadFlag && reload)
@@ -1152,16 +1155,19 @@ const browsingContextTargetPrototype = {
    * Opposite of the _toggleDevToolsSettings method, that reset document state
    * when closing the toolbox.
    */
   _restoreDocumentSettings() {
     this._restoreJavascript();
     this._setCacheDisabled(false);
     this._setServiceWorkersTestingEnabled(false);
     this._setPaintFlashingEnabled(false);
+    if (this._restoreFocus) {
+      this.window.focus();
+    }
   },
 
   /**
    * Disable or enable the cache via docShell.
    */
   _setCacheDisabled(disabled) {
     const enable = Ci.nsIRequest.LOAD_NORMAL;
     const disable =