Bug 1500257 part 7 - Modify RenderFrame to hold onto TabParent instead of nsFrameLoader. r=aosmond
☠☠ backed out by 2b27038adbe4 ☠ ☠
authorRyan Hunt <rhunt@eqrion.net>
Wed, 23 Jan 2019 09:52:30 -0600
changeset 458342 4c85fb68f2ed297828bf4646301c2d80d1c8e0a1
parent 458341 ba62cc27c32f4d8a3fefff8eee5bf47d270130bc
child 458343 1e1b5cd23412a85fad19ab8ec5aacf31b3a9c9b6
push id111812
push userrhunt@eqrion.net
push dateSat, 09 Feb 2019 06:41:25 +0000
treeherdermozilla-inbound@335ddf6a213a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaosmond
bugs1500257
milestone67.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 1500257 part 7 - Modify RenderFrame to hold onto TabParent instead of nsFrameLoader. r=aosmond A TabParent for a remote subframe will have the same owner content as the top-level remote browser. This means that 'TabParent::GetFrameLoader()' will return the frame loader of the top-level remote browser. This is fine for getting the layer manager and compositor for connecting layer trees, but the frame loader is also used to acquire a TabParent for its process ID. This is incorrect in the remote subframe case, and will lead to the compositor rejecting layer transactions for the remote subframe because it will only accept them from the top-level remote browser's process. This commit switches RenderFrame to just hold on to TabParent, and acquire the nsFrameLoader as necessary. Another change is to RenderFrame::SetOwnerContent. Previously this method would take the new owner content and check an assertion. I don't see much value in the assertion, so I've removed it. Additionally, now that we acquire the owner content, and therefore the layer manager, from TabParent, we need to ensure that RenderFrame::SetOwnerContent is ran after the TabParent has had it's owner content updated. So the callsite has been moved into TabParent. This resolved a test failure with frame loader swapping. Differential Revision: https://phabricator.services.mozilla.com/D17447
dom/base/nsFrameLoader.cpp
dom/ipc/TabParent.cpp
layout/ipc/RenderFrame.cpp
layout/ipc/RenderFrame.h
--- a/dom/base/nsFrameLoader.cpp
+++ b/dom/base/nsFrameLoader.cpp
@@ -1769,20 +1769,16 @@ void nsFrameLoader::SetOwnerContent(Elem
 
   JS::RootedObject wrapper(jsapi.cx(), GetWrapper());
   if (wrapper) {
     JSAutoRealm ar(jsapi.cx(), wrapper);
     IgnoredErrorResult rv;
     UpdateReflectorGlobal(jsapi.cx(), wrapper, rv);
     Unused << NS_WARN_IF(rv.Failed());
   }
-
-  if (RenderFrame* rfp = GetCurrentRenderFrame()) {
-    rfp->OwnerContentChanged(aContent);
-  }
 }
 
 bool nsFrameLoader::OwnerIsMozBrowserFrame() {
   nsCOMPtr<nsIMozBrowserFrame> browserFrame = do_QueryInterface(mOwnerContent);
   return browserFrame ? browserFrame->GetReallyIsBrowser() : false;
 }
 
 bool nsFrameLoader::OwnerIsIsolatedMozBrowserFrame() {
--- a/dom/ipc/TabParent.cpp
+++ b/dom/ipc/TabParent.cpp
@@ -299,16 +299,20 @@ void TabParent::SetOwnerElement(Element*
   nsCOMPtr<nsIWidget> widget = GetTopLevelWidget();
   if (widget) {
     WindowsHandle widgetNativeData = reinterpret_cast<WindowsHandle>(
         widget->GetNativeData(NS_NATIVE_SHAREABLE_WINDOW));
     if (widgetNativeData) {
       Unused << SendSetWidgetNativeData(widgetNativeData);
     }
   }
+
+  if (mRenderFrame.IsInitialized()) {
+    mRenderFrame.OwnerContentChanged();
+  }
 }
 
 NS_IMETHODIMP TabParent::GetOwnerElement(Element** aElement) {
   *aElement = do_AddRef(GetOwnerElement()).take();
   return NS_OK;
 }
 
 void TabParent::AddWindowListeners() {
@@ -612,26 +616,18 @@ void TabParent::LoadURL(nsIURI* aURI) {
     mDelayedURL = spec;
     return;
   }
 
   Unused << SendLoadURL(spec, GetShowInfo());
 }
 
 void TabParent::InitRendering() {
-  RefPtr<nsFrameLoader> frameLoader = GetFrameLoader();
-
   MOZ_ASSERT(!mRenderFrame.IsInitialized());
-  MOZ_ASSERT(frameLoader);
-
-  if (!frameLoader) {
-    return;
-  }
-
-  mRenderFrame.Initialize(frameLoader);
+  mRenderFrame.Initialize(this);
   MOZ_ASSERT(mRenderFrame.IsInitialized());
 
   layers::LayersId layersId = mRenderFrame.GetLayersId();
   AddTabParentToTable(layersId, this);
 
   TextureFactoryIdentifier textureFactoryIdentifier;
   mRenderFrame.GetTextureFactoryIdentifier(&textureFactoryIdentifier);
   Unused << SendInitRendering(textureFactoryIdentifier, layersId,
--- a/layout/ipc/RenderFrame.cpp
+++ b/layout/ipc/RenderFrame.cpp
@@ -23,116 +23,99 @@
 
 using namespace mozilla::dom;
 using namespace mozilla::gfx;
 using namespace mozilla::layers;
 
 namespace mozilla {
 namespace layout {
 
-static already_AddRefed<LayerManager> GetLayerManager(
-    nsFrameLoader* aFrameLoader) {
-  if (nsIContent* content = aFrameLoader->GetOwnerContent()) {
-    RefPtr<LayerManager> lm = nsContentUtils::LayerManagerForContent(content);
-    if (lm) {
-      return lm.forget();
-    }
+static already_AddRefed<LayerManager> GetLayerManager(TabParent* aTabParent) {
+  if (Element* element = aTabParent->GetOwnerElement()) {
+    return nsContentUtils::LayerManagerForContent(element);
   }
-
-  Document* doc = aFrameLoader->GetOwnerDoc();
-  if (!doc) {
-    return nullptr;
-  }
-  return nsContentUtils::LayerManagerForDocument(doc);
+  return nullptr;
 }
 
 RenderFrame::RenderFrame()
     : mLayersId{0},
-      mFrameLoader(nullptr),
+      mTabParent(nullptr),
       mLayerManager(nullptr),
       mInitialized(false),
       mLayersConnected(false) {}
 
 RenderFrame::~RenderFrame() {}
 
-bool RenderFrame::Initialize(nsFrameLoader* aFrameLoader) {
-  if (mInitialized || !aFrameLoader) {
+bool RenderFrame::Initialize(TabParent* aTabParent) {
+  if (mInitialized || !aTabParent) {
     return false;
   }
 
-  mFrameLoader = aFrameLoader;
-  RefPtr<LayerManager> lm = GetLayerManager(mFrameLoader);
+  mTabParent = aTabParent;
+  RefPtr<LayerManager> lm = GetLayerManager(mTabParent);
   PCompositorBridgeChild* compositor =
       lm ? lm->GetCompositorBridgeChild() : nullptr;
-
-  TabParent* browser = TabParent::GetFrom(aFrameLoader);
-  mTabProcessId = browser->Manager()->AsContentParent()->OtherPid();
+  mTabProcessId = mTabParent->Manager()->AsContentParent()->OtherPid();
 
   // Our remote frame will push layers updates to the compositor,
   // and we'll keep an indirect reference to that tree.
   GPUProcessManager* gpm = GPUProcessManager::Get();
   mLayersConnected = gpm->AllocateAndConnectLayerTreeId(
       compositor, mTabProcessId, &mLayersId, &mCompositorOptions);
 
   mInitialized = true;
   return true;
 }
 
 void RenderFrame::Destroy() {
   if (mLayersId.IsValid()) {
     GPUProcessManager::Get()->UnmapLayerTreeId(mLayersId, mTabProcessId);
   }
 
-  mFrameLoader = nullptr;
+  mTabParent = nullptr;
   mLayerManager = nullptr;
 }
 
 void RenderFrame::EnsureLayersConnected(CompositorOptions* aCompositorOptions) {
-  RefPtr<LayerManager> lm = GetLayerManager(mFrameLoader);
+  RefPtr<LayerManager> lm = GetLayerManager(mTabParent);
   if (!lm) {
     return;
   }
 
   if (!lm->GetCompositorBridgeChild()) {
     return;
   }
 
   mLayersConnected = lm->GetCompositorBridgeChild()->SendNotifyChildRecreated(
       mLayersId, &mCompositorOptions);
   *aCompositorOptions = mCompositorOptions;
 }
 
 LayerManager* RenderFrame::AttachLayerManager() {
   RefPtr<LayerManager> lm;
-  if (mFrameLoader) {
-    lm = GetLayerManager(mFrameLoader);
+  if (mTabParent) {
+    lm = GetLayerManager(mTabParent);
   }
 
   // Perhaps the document containing this frame currently has no presentation?
   if (lm && lm->GetCompositorBridgeChild() && lm != mLayerManager) {
     mLayersConnected =
         lm->GetCompositorBridgeChild()->SendAdoptChild(mLayersId);
     FrameLayerBuilder::InvalidateAllLayers(lm);
   }
 
   mLayerManager = lm.forget();
   return mLayerManager;
 }
 
-void RenderFrame::OwnerContentChanged(nsIContent* aContent) {
-  MOZ_ASSERT(!mFrameLoader || mFrameLoader->GetOwnerContent() == aContent,
-             "Don't build new map if owner is same!");
-
-  Unused << AttachLayerManager();
-}
+void RenderFrame::OwnerContentChanged() { Unused << AttachLayerManager(); }
 
 void RenderFrame::GetTextureFactoryIdentifier(
     TextureFactoryIdentifier* aTextureFactoryIdentifier) const {
-  RefPtr<LayerManager> lm =
-      mFrameLoader ? GetLayerManager(mFrameLoader) : nullptr;
+  RefPtr<LayerManager> lm = mTabParent ? GetLayerManager(mTabParent) : nullptr;
   // Perhaps the document containing this frame currently has no presentation?
   if (lm) {
     *aTextureFactoryIdentifier = lm->GetTextureFactoryIdentifier();
   } else {
     *aTextureFactoryIdentifier = TextureFactoryIdentifier();
   }
 }
 
--- a/layout/ipc/RenderFrame.h
+++ b/layout/ipc/RenderFrame.h
@@ -16,16 +16,20 @@
 #include "mozilla/layers/LayersTypes.h"
 #include "nsDisplayList.h"
 
 class nsFrameLoader;
 class nsSubDocumentFrame;
 
 namespace mozilla {
 
+namespace dom {
+class TabParent;
+}  // namespace dom
+
 namespace layers {
 struct TextureFactoryIdentifier;
 }  // namespace layers
 
 namespace layout {
 
 /**
  * RenderFrame connects and manages layer trees for remote frames. It is
@@ -36,24 +40,23 @@ class RenderFrame final {
   typedef mozilla::layers::LayerManager LayerManager;
   typedef mozilla::layers::LayersId LayersId;
   typedef mozilla::layers::TextureFactoryIdentifier TextureFactoryIdentifier;
 
  public:
   RenderFrame();
   virtual ~RenderFrame();
 
-  bool Initialize(nsFrameLoader* aFrameLoader);
+  bool Initialize(dom::TabParent* aTabParent);
   void Destroy();
 
   void EnsureLayersConnected(CompositorOptions* aCompositorOptions);
   LayerManager* AttachLayerManager();
-  void OwnerContentChanged(nsIContent* aContent);
+  void OwnerContentChanged();
 
-  nsFrameLoader* GetFrameLoader() const { return mFrameLoader; }
   LayersId GetLayersId() const { return mLayersId; }
   CompositorOptions GetCompositorOptions() const { return mCompositorOptions; }
 
   void GetTextureFactoryIdentifier(
       TextureFactoryIdentifier* aTextureFactoryIdentifier) const;
 
   bool IsInitialized() const { return mInitialized; }
   bool IsLayersConnected() const { return mLayersConnected; }
@@ -64,17 +67,17 @@ class RenderFrame final {
   base::ProcessId mTabProcessId;
   // The layers id of the remote frame.
   LayersId mLayersId;
   // The compositor options for this layers id. This is only meaningful if
   // the compositor actually knows about this layers id (i.e. when
   // mLayersConnected is true).
   CompositorOptions mCompositorOptions;
 
-  RefPtr<nsFrameLoader> mFrameLoader;
+  dom::TabParent* mTabParent;
   RefPtr<LayerManager> mLayerManager;
 
   bool mInitialized;
   // A flag that indicates whether or not the compositor knows about the
   // layers id. In some cases this RenderFrame is not connected to the
   // compositor and so this flag is false.
   bool mLayersConnected;
 };