Bug 1541088 - Revert focus-related Fission event delivery changes on beta. r=nika a=pascalc
authorHenri Sivonen <hsivonen@hsivonen.fi>
Tue, 09 Apr 2019 10:08:51 +0200
changeset 526107 661df9ccbd0de5eda56c3cf0a766c2ddcee3abbe
parent 526106 590d0706d113fca9c085f866f3c7ea29955cc8e2
child 526108 1796fcfc0b87f938ad8d1e7a584ca5fbf2c56ffe
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnika, pascalc
bugs1541088, 1524231, 1532334, 1524980, 1524977, 1534258, 1534255
milestone67.0
Bug 1541088 - Revert focus-related Fission event delivery changes on beta. r=nika a=pascalc This changeset reverts: * Bug 1524231 - Find TabParent to dispatch to by LayersId attached to the event. * Bug 1532334 - Make nsContentUtils::IsSubDocumentTabbable() return true for out-of-process iframes. * Bug 1524980 - Use RemoteFrameChild if present in nsFocusManager::Focus(). * Bug 1524977 - Use RemoteFrameChild if present in nsFocusManager::GetNextTabbableContent(). * Bug 1534258 - Send Deactivate() messages to out-of-process iframes. * Bug 1534255 - Enable out-of-process iframes to take APZ focus. Differential Revision: https://phabricator.services.mozilla.com//D26689
dom/base/nsContentUtils.cpp
dom/base/nsFocusManager.cpp
dom/base/nsFrameLoader.cpp
dom/base/nsFrameLoader.h
dom/events/EventStateManager.cpp
dom/ipc/BrowserBridgeChild.cpp
dom/ipc/BrowserBridgeChild.h
dom/ipc/BrowserBridgeParent.cpp
dom/ipc/BrowserBridgeParent.h
dom/ipc/PBrowserBridge.ipdl
dom/ipc/TabParent.cpp
gfx/layers/apz/src/FocusTarget.cpp
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@@ -6195,18 +6195,17 @@ bool nsContentUtils::IsFocusedContent(co
 bool nsContentUtils::IsSubDocumentTabbable(nsIContent* aContent) {
   Document* doc = aContent->GetComposedDoc();
   if (!doc) {
     return false;
   }
 
   // If the subdocument lives in another process, the frame is
   // tabbable.
-  if (EventStateManager::IsRemoteTarget(aContent) ||
-      BrowserBridgeChild::GetFrom(aContent)) {
+  if (EventStateManager::IsRemoteTarget(aContent)) {
     return true;
   }
 
   // XXXbz should this use OwnerDoc() for GetSubDocumentFor?
   // sXBL/XBL2 issue!
   Document* subDoc = doc->GetSubDocumentFor(aContent);
   if (!subDoc) {
     return false;
--- a/dom/base/nsFocusManager.cpp
+++ b/dom/base/nsFocusManager.cpp
@@ -46,17 +46,16 @@
 #include "nsRange.h"
 
 #include "mozilla/AccessibleCaretEventHub.h"
 #include "mozilla/ContentEvents.h"
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/HTMLImageElement.h"
 #include "mozilla/dom/HTMLInputElement.h"
 #include "mozilla/dom/HTMLSlotElement.h"
-#include "mozilla/dom/BrowserBridgeChild.h"
 #include "mozilla/dom/Text.h"
 #include "mozilla/EventDispatcher.h"
 #include "mozilla/EventStateManager.h"
 #include "mozilla/EventStates.h"
 #include "mozilla/HTMLEditor.h"
 #include "mozilla/IMEStateManager.h"
 #include "mozilla/LookAndFeel.h"
 #include "mozilla/Preferences.h"
@@ -1632,22 +1631,16 @@ bool nsFocusManager::Blur(nsPIDOMWindowO
     }
 
     // if the object being blurred is a remote browser, deactivate remote
     // content
     if (TabParent* remote = TabParent::GetFrom(element)) {
       remote->Deactivate();
       LOGFOCUS(("Remote browser deactivated %p", remote));
     }
-
-    // Same as above but for out-of-process iframes
-    if (BrowserBridgeChild* bbc = BrowserBridgeChild::GetFrom(element)) {
-      bbc->Deactivate();
-      LOGFOCUS(("Out-of-process iframe deactivated %p", bbc));
-    }
   }
 
   bool result = true;
   if (sendBlurEvent) {
     // if there is an active window, update commands. If there isn't an active
     // window, then this was a blur caused by the active window being lowered,
     // so there is no need to update the commands
     if (mActiveWindow)
@@ -1853,22 +1846,16 @@ void nsFocusManager::Focus(nsPIDOMWindow
           objectFrameWidget->SetFocus(false);
 
         // if the object being focused is a remote browser, activate remote
         // content
         if (TabParent* remote = TabParent::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));
-        }
       }
 
       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
@@ -3481,23 +3468,16 @@ nsresult nsFocusManager::GetNextTabbable
           // being focusable in some way, the child process will call back
           // into document navigation again by calling MoveFocus.
           TabParent* remote = TabParent::GetFrom(currentContent);
           if (remote) {
             remote->NavigateByKey(aForward, aForDocumentNavigation);
             return NS_SUCCESS_DOM_NO_OPERATION;
           }
 
-          // Same as above but for out-of-process iframes
-          BrowserBridgeChild* bbc = BrowserBridgeChild::GetFrom(currentContent);
-          if (bbc) {
-            bbc->NavigateByKey(aForward, aForDocumentNavigation);
-            return NS_SUCCESS_DOM_NO_OPERATION;
-          }
-
           // Next, for document navigation, check if this a non-remote child
           // document.
           bool checkSubDocument = true;
           if (aForDocumentNavigation &&
               TryDocumentNavigation(currentContent, &checkSubDocument,
                                     aResultContent)) {
             return NS_OK;
           }
--- a/dom/base/nsFrameLoader.cpp
+++ b/dom/base/nsFrameLoader.cpp
@@ -2665,20 +2665,16 @@ bool nsFrameLoader::IsRemoteFrame() {
   }
   return false;
 }
 
 mozilla::dom::PBrowserParent* nsFrameLoader::GetRemoteBrowser() const {
   return mRemoteBrowser;
 }
 
-mozilla::dom::BrowserBridgeChild* nsFrameLoader::GetBrowserBridgeChild() const {
-  return mBrowserBridgeChild;
-}
-
 mozilla::layers::LayersId nsFrameLoader::GetLayersId() const {
   MOZ_ASSERT(mIsRemoteFrame);
   if (mRemoteBrowser) {
     return mRemoteBrowser->GetRenderFrame()->GetLayersId();
   }
   if (mBrowserBridgeChild) {
     return mBrowserBridgeChild->GetLayersId();
   }
--- a/dom/base/nsFrameLoader.h
+++ b/dom/base/nsFrameLoader.h
@@ -290,22 +290,16 @@ class nsFrameLoader final : public nsStu
 
   /**
    * Returns the IPDL actor used if this is a top-level remote browser, or null
    * otherwise.
    */
   PBrowserParent* GetRemoteBrowser() const;
 
   /**
-   * Returns the BrowserBridgeChild if this is an out-of-process iframe, or null
-   * otherwise.
-   */
-  mozilla::dom::BrowserBridgeChild* GetBrowserBridgeChild() const;
-
-  /**
    * Returns the layers ID that this remote frame is using to render.
    *
    * This must only be called if this is a remote frame.
    */
   mozilla::layers::LayersId GetLayersId() const;
 
   mozilla::dom::ChromeMessageSender* GetFrameMessageManager() {
     return mMessageManager;
--- a/dom/events/EventStateManager.cpp
+++ b/dom/events/EventStateManager.cpp
@@ -1221,30 +1221,16 @@ bool EventStateManager::WalkESMTreeToHan
 void EventStateManager::DispatchCrossProcessEvent(WidgetEvent* aEvent,
                                                   nsFrameLoader* aFrameLoader,
                                                   nsEventStatus* aStatus) {
   TabParent* remote = TabParent::GetFrom(aFrameLoader);
   if (!remote) {
     return;
   }
 
-  if (aEvent->mLayersId.IsValid()) {
-    TabParent* preciseRemote =
-        TabParent::GetTabParentFromLayersId(aEvent->mLayersId);
-    if (preciseRemote) {
-      remote = preciseRemote;
-    }
-    // else there is a race between APZ and the LayersId to TabParent mapping,
-    // so fall back to delivering the event to the topmost child process.
-  }
-  // else if aEvent->mLayersId was not valid: APZ thinks a pointer
-  // event didn't hit anything but traditional targeting believed it
-  // belongs to a child process-backed frame loader. Dispatch to the
-  // top-level content process found by traditional targeting.
-
   switch (aEvent->mClass) {
     case eMouseEventClass: {
       remote->SendRealMouseEvent(*aEvent->AsMouseEvent());
       return;
     }
     case eKeyboardEventClass: {
       remote->SendRealKeyEvent(*aEvent->AsKeyboardEvent());
       return;
--- a/dom/ipc/BrowserBridgeChild.cpp
+++ b/dom/ipc/BrowserBridgeChild.cpp
@@ -79,100 +79,21 @@ void BrowserBridgeChild::UpdateDimension
   CSSSize unscaledSize = devicePixelSize / widgetScale;
   hal::ScreenOrientation orientation = hal::eScreenOrientation_Default;
   DimensionInfo di(unscaledRect, unscaledSize, orientation, clientOffset,
                    chromeOffset);
 
   Unused << SendUpdateDimensions(di);
 }
 
-void BrowserBridgeChild::NavigateByKey(bool aForward,
-                                       bool aForDocumentNavigation) {
-  Unused << SendNavigateByKey(aForward, aForDocumentNavigation);
-}
-
-void BrowserBridgeChild::Activate() { Unused << SendActivate(); }
-
-void BrowserBridgeChild::Deactivate() { Unused << SendDeactivate(); }
-
-/*static*/
-BrowserBridgeChild* BrowserBridgeChild::GetFrom(nsFrameLoader* aFrameLoader) {
-  if (!aFrameLoader) {
-    return nullptr;
-  }
-  return aFrameLoader->GetBrowserBridgeChild();
-}
-
-/*static*/
-BrowserBridgeChild* BrowserBridgeChild::GetFrom(nsIContent* aContent) {
-  RefPtr<nsFrameLoaderOwner> loaderOwner = do_QueryObject(aContent);
-  if (!loaderOwner) {
-    return nullptr;
-  }
-  RefPtr<nsFrameLoader> frameLoader = loaderOwner->GetFrameLoader();
-  return GetFrom(frameLoader);
-}
-
 IPCResult BrowserBridgeChild::RecvSetLayersId(
     const mozilla::layers::LayersId& aLayersId) {
   MOZ_ASSERT(!mLayersId.IsValid() && aLayersId.IsValid());
   mLayersId = aLayersId;
   return IPC_OK();
 }
 
-mozilla::ipc::IPCResult BrowserBridgeChild::RecvRequestFocus(
-    const bool& aCanRaise) {
-  // Adapted from TabParent
-  nsCOMPtr<nsIFocusManager> fm = nsFocusManager::GetFocusManager();
-  if (!fm) {
-    return IPC_OK();
-  }
-
-  RefPtr<Element> owner = mFrameLoader->GetOwnerContent();
-
-  if (!owner || !owner->OwnerDoc()) {
-    return IPC_OK();
-  }
-
-  uint32_t flags = nsIFocusManager::FLAG_NOSCROLL;
-  if (aCanRaise) {
-    flags |= nsIFocusManager::FLAG_RAISE;
-  }
-
-  fm->SetFocus(owner, flags);
-  return IPC_OK();
-}
-
-mozilla::ipc::IPCResult BrowserBridgeChild::RecvMoveFocus(
-    const bool& aForward, const bool& aForDocumentNavigation) {
-  // Adapted from TabParent
-  nsCOMPtr<nsIFocusManager> fm = nsFocusManager::GetFocusManager();
-  if (!fm) {
-    return IPC_OK();
-  }
-
-  RefPtr<Element> owner = mFrameLoader->GetOwnerContent();
-
-  if (!owner || !owner->OwnerDoc()) {
-    return IPC_OK();
-  }
-
-  RefPtr<Element> dummy;
-
-  uint32_t type =
-      aForward
-          ? (aForDocumentNavigation
-                 ? static_cast<uint32_t>(nsIFocusManager::MOVEFOCUS_FORWARDDOC)
-                 : static_cast<uint32_t>(nsIFocusManager::MOVEFOCUS_FORWARD))
-          : (aForDocumentNavigation
-                 ? static_cast<uint32_t>(nsIFocusManager::MOVEFOCUS_BACKWARDDOC)
-                 : static_cast<uint32_t>(nsIFocusManager::MOVEFOCUS_BACKWARD));
-  fm->MoveFocus(nullptr, owner, type, nsIFocusManager::FLAG_BYKEY,
-                getter_AddRefs(dummy));
-  return IPC_OK();
-}
-
 void BrowserBridgeChild::ActorDestroy(ActorDestroyReason aWhy) {
   mIPCOpen = false;
 }
 
 }  // namespace dom
 }  // namespace mozilla
--- a/dom/ipc/BrowserBridgeChild.h
+++ b/dom/ipc/BrowserBridgeChild.h
@@ -35,26 +35,16 @@ class BrowserBridgeChild : public PBrows
 
   static already_AddRefed<BrowserBridgeChild> Create(
       nsFrameLoader* aFrameLoader, const TabContext& aContext,
       const nsString& aRemoteType, BrowsingContext* aBrowsingContext);
 
   void UpdateDimensions(const nsIntRect& aRect,
                         const mozilla::ScreenIntSize& aSize);
 
-  void NavigateByKey(bool aForward, bool aForDocumentNavigation);
-
-  void Activate();
-
-  void Deactivate();
-
-  static BrowserBridgeChild* GetFrom(nsFrameLoader* aFrameLoader);
-
-  static BrowserBridgeChild* GetFrom(nsIContent* aContent);
-
  protected:
   friend class PBrowserBridgeChild;
 
   mozilla::ipc::IPCResult RecvSetLayersId(
       const mozilla::layers::LayersId& aLayersId);
 
   mozilla::ipc::IPCResult RecvRequestFocus(const bool& aCanRaise);
 
--- a/dom/ipc/BrowserBridgeParent.cpp
+++ b/dom/ipc/BrowserBridgeParent.cpp
@@ -120,30 +120,14 @@ IPCResult BrowserBridgeParent::RecvUpdat
 
 IPCResult BrowserBridgeParent::RecvRenderLayers(
     const bool& aEnabled, const bool& aForceRepaint,
     const layers::LayersObserverEpoch& aEpoch) {
   Unused << mTabParent->SendRenderLayers(aEnabled, aForceRepaint, aEpoch);
   return IPC_OK();
 }
 
-IPCResult BrowserBridgeParent::RecvNavigateByKey(
-    const bool& aForward, const bool& aForDocumentNavigation) {
-  Unused << mTabParent->SendNavigateByKey(aForward, aForDocumentNavigation);
-  return IPC_OK();
-}
-
-IPCResult BrowserBridgeParent::RecvActivate() {
-  mTabParent->Activate();
-  return IPC_OK();
-}
-
-IPCResult BrowserBridgeParent::RecvDeactivate() {
-  mTabParent->Deactivate();
-  return IPC_OK();
-}
-
 void BrowserBridgeParent::ActorDestroy(ActorDestroyReason aWhy) {
   mIPCOpen = false;
 }
 
 }  // namespace dom
 }  // namespace mozilla
--- a/dom/ipc/BrowserBridgeParent.h
+++ b/dom/ipc/BrowserBridgeParent.h
@@ -43,23 +43,16 @@ class BrowserBridgeParent : public PBrow
                                    const nsSizeMode& aSizeMode);
   mozilla::ipc::IPCResult RecvLoadURL(const nsCString& aUrl);
   mozilla::ipc::IPCResult RecvUpdateDimensions(
       const DimensionInfo& aDimensions);
   mozilla::ipc::IPCResult RecvRenderLayers(const bool& aEnabled,
                                            const bool& aForceRepaint,
                                            const LayersObserverEpoch& aEpoch);
 
-  mozilla::ipc::IPCResult RecvNavigateByKey(const bool& aForward,
-                                            const bool& aForDocumentNavigation);
-
-  mozilla::ipc::IPCResult RecvActivate();
-
-  mozilla::ipc::IPCResult RecvDeactivate();
-
   void ActorDestroy(ActorDestroyReason aWhy) override;
 
  private:
   ~BrowserBridgeParent();
 
   RefPtr<TabParent> mTabParent;
   bool mIPCOpen;
 };
--- a/dom/ipc/PBrowserBridge.ipdl
+++ b/dom/ipc/PBrowserBridge.ipdl
@@ -20,49 +20,23 @@ namespace dom {
  * PBrowserBridge corresponds to a remote iframe.
  */
 async protocol PBrowserBridge {
   manager PBrowser;
 
 child:
   async SetLayersId(LayersId layersId);
 
-  /**
-   * Request that the IPC child / Web parent process move focus to the
-   * browser's frame. If canRaise is true, the window can be raised if
-   * it is inactive.
-   */
-  async RequestFocus(bool canRaise);
-
-  /**
-   * When IPC parent / Web child sends this message, the IPC child / Web parent
-   * should move focus to the next or previous focusable element or document.
-   */
-  async MoveFocus(bool forward, bool forDocumentNavigation);
-
 parent:
   // Destroy the remote web browser due to the nsFrameLoader going away.
   async __delete__();
 
   // DocShell messaging.
   async LoadURL(nsCString aSpec);
 
   // Out of process rendering.
   async Show(ScreenIntSize size, bool parentIsActive, nsSizeMode sizeMode);
   async UpdateDimensions(DimensionInfo dimensions) compressall;
   async RenderLayers(bool aEnabled, bool aForceRepaint, LayersObserverEpoch aEpoch);
-
-  /**
-   * Navigate by key (Tab/Shift+Tab/F6/Shift+f6).
-   */
-  async NavigateByKey(bool aForward, bool aForDocumentNavigation);
-
-  /**
-   * Sending an activate message moves focus to the iframe.
-   */
-  async Activate();
-
-  async Deactivate();
-
 };
 
 }  // namespace dom
 }  // namespace mozilla
--- a/dom/ipc/TabParent.cpp
+++ b/dom/ipc/TabParent.cpp
@@ -494,23 +494,16 @@ void TabParent::ActorDestroy(ActorDestro
                         "ipc:browser-destroyed", nullptr);
   }
 }
 
 mozilla::ipc::IPCResult TabParent::RecvMoveFocus(
     const bool& aForward, const bool& aForDocumentNavigation) {
   LOGBROWSERFOCUS(("RecvMoveFocus %p, aForward: %d, aForDocumentNavigation: %d",
                    this, aForward, aForDocumentNavigation));
-  BrowserBridgeParent* bridgeParent = GetBrowserBridgeParent();
-  if (bridgeParent) {
-    mozilla::Unused << bridgeParent->SendMoveFocus(aForward,
-                                                   aForDocumentNavigation);
-    return IPC_OK();
-  }
-
   nsCOMPtr<nsIFocusManager> fm = nsFocusManager::GetFocusManager();
   if (fm) {
     RefPtr<Element> dummy;
 
     uint32_t type =
         aForward
             ? (aForDocumentNavigation
                    ? static_cast<uint32_t>(
@@ -1919,22 +1912,16 @@ mozilla::ipc::IPCResult TabParent::RecvO
   bool consumed = (rv == NS_SUCCESS_EVENT_CONSUMED);
   HandledWindowedPluginKeyEvent(aKeyEventData, consumed);
 
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult TabParent::RecvRequestFocus(const bool& aCanRaise) {
   LOGBROWSERFOCUS(("RecvRequestFocus %p, aCanRaise: %d", this, aCanRaise));
-  BrowserBridgeParent* bridgeParent = GetBrowserBridgeParent();
-  if (bridgeParent) {
-    mozilla::Unused << bridgeParent->SendRequestFocus(aCanRaise);
-    return IPC_OK();
-  }
-
   nsCOMPtr<nsIFocusManager> fm = nsFocusManager::GetFocusManager();
   if (!fm) {
     return IPC_OK();
   }
 
   if (!mFrameElement || !mFrameElement->OwnerDoc()) {
     return IPC_OK();
   }
--- a/gfx/layers/apz/src/FocusTarget.cpp
+++ b/gfx/layers/apz/src/FocusTarget.cpp
@@ -1,24 +1,23 @@
 /* -*- 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 "mozilla/layers/FocusTarget.h"
 
-#include "mozilla/dom/BrowserBridgeChild.h"  // for BrowserBridgeChild
-#include "mozilla/dom/EventTarget.h"         // for EventTarget
-#include "mozilla/dom/TabParent.h"           // for TabParent
-#include "mozilla/EventDispatcher.h"         // for EventDispatcher
-#include "mozilla/layout/RenderFrame.h"      // For RenderFrame
-#include "nsIContentInlines.h"               // for nsINode::IsEditable()
-#include "nsIPresShell.h"                    // for nsIPresShell
-#include "nsLayoutUtils.h"                   // for nsLayoutUtils
+#include "mozilla/dom/EventTarget.h"     // for EventTarget
+#include "mozilla/dom/TabParent.h"       // for TabParent
+#include "mozilla/EventDispatcher.h"     // for EventDispatcher
+#include "mozilla/layout/RenderFrame.h"  // For RenderFrame
+#include "nsIContentInlines.h"           // for nsINode::IsEditable()
+#include "nsIPresShell.h"                // for nsIPresShell
+#include "nsLayoutUtils.h"               // for nsLayoutUtils
 
 #define ENABLE_FT_LOGGING 0
 // #define ENABLE_FT_LOGGING 1
 
 #if ENABLE_FT_LOGGING
 #  define FT_LOG(FMT, ...)         \
     printf_stderr("FT (%s): " FMT, \
                   XRE_IsParentProcess() ? "chrome" : "content", __VA_ARGS__)
@@ -175,27 +174,16 @@ FocusTarget::FocusTarget(nsIPresShell* a
 
     FT_LOG("Creating nil target with seq=%" PRIu64
            ", kl=%d (remote browser missing layers id)\n",
            aFocusSequenceNumber, mFocusHasKeyEventListeners);
 
     return;
   }
 
-  // Check if the key event target is a remote browser
-  if (BrowserBridgeChild* bbc = BrowserBridgeChild::GetFrom(keyEventTarget)) {
-    FT_LOG("Creating oopif reflayer target with seq=%" PRIu64
-           ", kl=%d, lt=%" PRIu64 "\n",
-           aFocusSequenceNumber, mFocusHasKeyEventListeners,
-           bbc->GetLayersId());
-
-    mData = AsVariant<LayersId>(bbc->GetLayersId());
-    return;
-  }
-
   // The content to scroll is either the focused element or the focus node of
   // the selection. It's difficult to determine if an element is an interactive
   // element requiring async keyboard scrolling to be disabled. So we only
   // allow async key scrolling based on the selection, which doesn't have
   // this problem and is more common.
   if (focusedContent) {
     FT_LOG("Creating nil target with seq=%" PRIu64
            ", kl=%d (disabling for focusing an element)\n",