Bug 1546019 - When a focused browser changes remoteness, make sure to activate the remote browser if needed. r=qdot
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 29 Apr 2019 20:06:22 +0000
changeset 530629 d6564ee9d9417fe9eea7b4fdbe96ce04a198afb4
parent 530628 6596c844c27e39c3efdf2f26e8d2d4c52852165c
child 530630 6a60bb97e7ed4ce6fd3d718b983da6a55889b2fe
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersqdot
bugs1546019
milestone68.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 1546019 - When a focused browser changes remoteness, make sure to activate the remote browser if needed. r=qdot Not quite sure what's a good way to add a test for this... Ideas? Differential Revision: https://phabricator.services.mozilla.com/D29104
browser/base/content/tabbrowser.js
dom/base/nsFocusManager.cpp
dom/base/nsFocusManager.h
dom/base/nsFrameLoaderOwner.cpp
dom/base/nsFrameLoaderOwner.h
--- a/browser/base/content/tabbrowser.js
+++ b/browser/base/content/tabbrowser.js
@@ -1722,16 +1722,19 @@ window._gBrowser = {
     if (!Services.prefs.getBoolPref("fission.rebuild_frameloaders_on_remoteness_change", false)) {
       parent.appendChild(aBrowser);
     } else {
       // This call actually switches out our frameloaders. Do this as late as
       // possible before rebuilding the browser, as we'll need the new browser
       // state set up completely first.
       aBrowser.changeRemoteness({ remoteType });
       // Once we have new frameloaders, this call sets the browser back up.
+      //
+      // FIXME(emilio): Shouldn't we call destroy() first? What hides the
+      // select pop-ups and such otherwise?
       aBrowser.construct();
     }
 
     aBrowser.userTypedValue = oldUserTypedValue;
     if (hadStartedLoad) {
       aBrowser.urlbarChangeTracker.startedLoad();
     }
 
--- a/dom/base/nsFocusManager.cpp
+++ b/dom/base/nsFocusManager.cpp
@@ -1715,16 +1715,30 @@ bool nsFocusManager::Blur(nsPIDOMWindowO
     UpdateCaret(false, true, nullptr);
   }
 
   if (clearFirstBlurEvent) mFirstBlurEvent = nullptr;
 
   return result;
 }
 
+void nsFocusManager::ActivateRemoteFrameIfNeeded(Element& aElement) {
+  MOZ_DIAGNOSTIC_ASSERT(mFocusedElement == &aElement);
+  if (BrowserParent* remote = BrowserParent::GetFrom(&aElement)) {
+    remote->Activate();
+    LOGFOCUS(("Remote browser activated %p", remote));
+  }
+
+  // Same as above but for out-of-process iframes
+  if (BrowserBridgeChild* bbc = BrowserBridgeChild::GetFrom(&aElement)) {
+    bbc->Activate();
+    LOGFOCUS(("Out-of-process iframe activated %p", bbc));
+  }
+}
+
 void nsFocusManager::Focus(nsPIDOMWindowOuter* aWindow, Element* aElement,
                            uint32_t aFlags, bool aIsNewDocument,
                            bool aFocusChanged, bool aWindowRaised,
                            bool aAdjustWidgets, nsIContent* aContentLostFocus) {
   LOGFOCUS(("<<Focus begin>>"));
 
   if (!aWindow) return;
 
@@ -1857,26 +1871,17 @@ void nsFocusManager::Focus(nsPIDOMWindow
       // that we might no longer be in the same document, due to the events we
       // fired above when aIsNewDocument.
       if (presShell->GetDocument() == aElement->GetComposedDoc()) {
         if (aAdjustWidgets && objectFrameWidget && !sTestMode)
           objectFrameWidget->SetFocus(false);
 
         // if the object being focused is a remote browser, activate remote
         // content
-        if (BrowserParent* remote = BrowserParent::GetFrom(aElement)) {
-          remote->Activate();
-          LOGFOCUS(("Remote browser activated %p", remote));
-        }
-
-        // Same as above but for out-of-process iframes
-        if (BrowserBridgeChild* bbc = BrowserBridgeChild::GetFrom(aElement)) {
-          bbc->Activate();
-          LOGFOCUS(("Out-of-process iframe activated %p", bbc));
-        }
+        ActivateRemoteFrameIfNeeded(*aElement);
       }
 
       IMEStateManager::OnChangeFocus(presContext, aElement,
                                      GetFocusMoveActionCause(aFlags));
 
       // as long as this focus wasn't because a window was raised, update the
       // commands
       // XXXndeakin P2 someone could adjust the focus during the update
--- a/dom/base/nsFocusManager.h
+++ b/dom/base/nsFocusManager.h
@@ -164,16 +164,22 @@ class nsFocusManager final : public nsIF
    * navigation is not done to parent documents and iteration returns to the
    * beginning (or end) of the starting document.
    */
   nsresult DetermineElementToMoveFocus(nsPIDOMWindowOuter* aWindow,
                                        nsIContent* aStart, int32_t aType,
                                        bool aNoParentTraversal,
                                        nsIContent** aNextContent);
 
+  /**
+   * Given an element, which must be the focused element, activate the remote
+   * frame it embeds, if any.
+   */
+  void ActivateRemoteFrameIfNeeded(mozilla::dom::Element&);
+
   static uint32_t FocusOptionsToFocusManagerFlags(
       const mozilla::dom::FocusOptions& aOptions);
 
   /**
    * Returns the content node that focus will be redirected to if aContent was
    * focused. This is used for the special case of certain XUL elements such
    * as textboxes or input number which redirect focus to an anonymous child.
    *
--- a/dom/base/nsFrameLoaderOwner.cpp
+++ b/dom/base/nsFrameLoaderOwner.cpp
@@ -1,18 +1,22 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* 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/. */
 
 #include "nsFrameLoaderOwner.h"
 #include "nsFrameLoader.h"
+#include "nsFocusManager.h"
+#include "nsSubDocumentFrame.h"
+#include "nsQueryObject.h"
+#include "mozilla/AsyncEventDispatcher.h"
+#include "mozilla/dom/BrowsingContext.h"
 #include "mozilla/dom/FrameLoaderBinding.h"
-#include "mozilla/dom/BrowsingContext.h"
 
 already_AddRefed<nsFrameLoader> nsFrameLoaderOwner::GetFrameLoader() {
   return do_AddRef(mFrameLoader);
 }
 
 void nsFrameLoaderOwner::SetFrameLoader(nsFrameLoader* aNewFrameLoader) {
   mFrameLoader = aNewFrameLoader;
 }
@@ -46,21 +50,23 @@ void nsFrameLoaderOwner::ChangeRemotenes
   if (aOptions.mPendingSwitchID.WasPassed()) {
     mFrameLoader->ResumeLoad(aOptions.mPendingSwitchID.Value());
   } else {
     mFrameLoader->LoadFrame(false);
   }
 
   // Now that we've got a new FrameLoader, we need to reset our
   // nsSubDocumentFrame to use the new FrameLoader.
-  nsIFrame* ourFrame = owner->GetPrimaryFrame();
-  if (ourFrame) {
-    nsSubDocumentFrame* ourFrameFrame = do_QueryFrame(ourFrame);
-    if (ourFrameFrame) {
-      ourFrameFrame->ResetFrameLoader();
+  if (nsSubDocumentFrame* ourFrame = do_QueryFrame(owner->GetPrimaryFrame())) {
+    ourFrame->ResetFrameLoader();
+  }
+
+  if (nsFocusManager* fm = nsFocusManager::GetFocusManager()) {
+    if (fm->GetFocusedElement() == owner) {
+      fm->ActivateRemoteFrameIfNeeded(*owner);
     }
   }
 
   // Assuming this element is a XULFrameElement, once we've reset our
   // FrameLoader, fire an event to act like we've recreated ourselves, similar
   // to what XULFrameElement does after rebinding to the tree.
   // ChromeOnlyDispatch is turns on to make sure this isn't fired into content.
   (new AsyncEventDispatcher(owner, NS_LITERAL_STRING("XULFrameLoaderCreated"),
--- a/dom/base/nsFrameLoaderOwner.h
+++ b/dom/base/nsFrameLoaderOwner.h
@@ -2,16 +2,18 @@
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* 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/. */
 
 #ifndef nsFrameLoaderOwner_h_
 #define nsFrameLoaderOwner_h_
 
+#include "nsISupports.h"
+
 class nsFrameLoader;
 namespace mozilla {
 class ErrorResult;
 namespace dom {
 class BrowsingContext;
 struct RemotenessOptions;
 }  // namespace dom
 }  // namespace mozilla