Bug 1280715 - Move FixedSizeShmemAllocator to CompositorBridgeChild to allow for a more sane destruction sequence r=nical

gfx/layers/ipc/CompositorBridgeChild.cpp

@@ -59,6 +59,7 @@ CompositorBridgeChild::CompositorBridgeChild(ClientLayerManager *aLayerManager)
   , mCanSend(false)
   , mFwdTransactionId(0)
   , mMessageLoop(MessageLoop::current())
+  , mSectionAllocator(nullptr)
 {
   MOZ_ASSERT(NS_IsMainThread());
 }
@@ -100,6 +101,11 @@ CompositorBridgeChild::Destroy()
     mTexturePools[i]->Destroy();
   }
 
+  if (mSectionAllocator) {
+    delete mSectionAllocator;
+    mSectionAllocator = nullptr;
+  }
+
   // Destroying the layer manager may cause all sorts of things to happen, so
   // let's make sure there is still a reference to keep this alive whatever
   // happens.
@@ -960,6 +966,21 @@ CompositorBridgeChild::ClearTexturePool()
   }
 }
 
+FixedSizeSmallShmemSectionAllocator*
+CompositorBridgeChild::GetTileLockAllocator()
+{
+  MOZ_ASSERT(IPCOpen());
+  if (!IPCOpen()) {
+    return nullptr;
+  }
+
+  if (!mSectionAllocator) {
+    mSectionAllocator = new FixedSizeSmallShmemSectionAllocator(this);
+  }
+  return mSectionAllocator;
+}
+
+
 PTextureChild*
 CompositorBridgeChild::CreateTexture(const SurfaceDescriptor& aSharedData,
                                      LayersBackend aLayersBackend,

gfx/layers/ipc/CompositorBridgeChild.h

@@ -194,6 +194,8 @@ public:
                                     TextureFlags aFlags);
   void ClearTexturePool();
 
+  virtual FixedSizeSmallShmemSectionAllocator* GetTileLockAllocator() override;
+
   void HandleMemoryPressure();
 
   virtual MessageLoop* GetMessageLoop() const override { return mMessageLoop; }
@@ -312,6 +314,8 @@ private:
   AutoTArray<RefPtr<TextureClientPool>,2> mTexturePools;
 
   uint64_t mProcessToken;
+
+  FixedSizeSmallShmemSectionAllocator* mSectionAllocator;
 };
 
 } // namespace layers

gfx/layers/ipc/ISurfaceAllocator.h

@@ -316,26 +316,7 @@ public:
 
   ShmemAllocator* GetShmAllocator() { return mShmProvider->AsShmemAllocator(); }
 
-  /**
-    * In order to avoid shutdown crashes, we need to test for mShmProvider->AsShmemAllocator()
-    * here. Basically, there's a case where we have the following class hierarchy:
-    *
-    * ClientIPCAllocator -> TextureForwarder -> CompositableForwarder -> ShadowLayerForwarder
-    *
-    * In ShadowLayerForwarder's dtor, we tear down the actor and close the IPC channel.
-    * In TextureForwarder's dtor, we destroy the FixedSizeSmallShmemAllocator and that in turn calls
-    * ClientIPCAllocator::IPCOpen() to determine whether we can dealloc some shmem regions.
-    *
-    * This does not work. In the above class diagram, as the ShadowLayerForwarder's dtor has run
-    * its course, the ClientIPCAllocator object we're holding on to is now just a plain
-    * ClientIPCAllocator and so we call ClientIPCAllocator's IPCOpen() which unconditionally
-    * returns true. We therefore have to rely on AsShmemAllocator() to determine whether we can
-    * do these deallocs as ClientIPCAllocator::AsShmemAllocator() returns nullptr.
-    *
-    * Ideally, we should move a lot of this destruction work into non-destructor Destroy() methods
-    * which do cleanup before we destroy the objects.
-    */
-  bool IPCOpen() const { return mShmProvider->AsShmemAllocator() && mShmProvider->IPCOpen(); }
+  bool IPCOpen() const { return mShmProvider->IPCOpen(); }
 
 protected:
   std::vector<mozilla::ipc::Shmem> mUsedShmems;

gfx/layers/ipc/TextureForwarder.cpp

@@ -1,40 +0,0 @@
-/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*-
- * vim: sw=2 ts=8 et :
- */
-/* 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/TextureForwarder.h"
-
-namespace mozilla {
-namespace layers {
-
-TextureForwarder::TextureForwarder()
-  : mSectionAllocator(nullptr)
-{
-}
-
-TextureForwarder::~TextureForwarder()
-{
-  if (mSectionAllocator) {
-    delete mSectionAllocator;
-  }
-}
-
-FixedSizeSmallShmemSectionAllocator*
-TextureForwarder::GetTileLockAllocator()
-{
-  MOZ_ASSERT(IPCOpen());
-  if (!IPCOpen()) {
-    return nullptr;
-  }
-
-  if (!mSectionAllocator) {
-    mSectionAllocator = new FixedSizeSmallShmemSectionAllocator(this);
-  }
-  return mSectionAllocator;
-}
-
-} // namespace layers
-} // namespace mozilla

gfx/layers/ipc/TextureForwarder.h

@@ -22,10 +22,6 @@ namespace layers {
 class TextureForwarder : public ClientIPCAllocator
 {
 public:
-  TextureForwarder();
-
-  virtual ~TextureForwarder();
-
   /**
    * Create a TextureChild/Parent pair as as well as the TextureHost on the parent side.
    */
@@ -37,10 +33,7 @@ public:
 
   virtual TextureForwarder* AsTextureForwarder() override { return this; }
 
-  virtual FixedSizeSmallShmemSectionAllocator* GetTileLockAllocator();
-
-private:
-  FixedSizeSmallShmemSectionAllocator* mSectionAllocator;
+  virtual FixedSizeSmallShmemSectionAllocator* GetTileLockAllocator() { return nullptr; }
 };
 
 } // namespace layers

gfx/layers/moz.build

@@ -374,7 +374,6 @@ UNIFIED_SOURCES += [
     'ipc/SharedBufferManagerParent.cpp',
     'ipc/SharedPlanarYCbCrImage.cpp',
     'ipc/SharedRGBImage.cpp',
-    'ipc/TextureForwarder.cpp',
     'LayerScope.cpp',
     'LayersLogging.cpp',
     'LayerSorter.cpp',