Bug 1321275 - Fix reference counting of PVRLayerChild when GPU process has been terminated draft
authorKearwood "Kip" Gilbert <kgilbert@mozilla.com>
Wed, 12 Jul 2017 16:05:40 -0700
changeset 609115 8cbc18e032b7d86d376ffa18f367d1ef7c577093
parent 609112 2908b6171cb8df85fc1f0985e2bf94ce3b04a522
child 609165 8822f5146ac305e51d84163fa535da5fe1973ae0
push id68504
push userkgilbert@mozilla.com
push dateFri, 14 Jul 2017 20:22:02 +0000
bugs1321275
milestone56.0a1
Bug 1321275 - Fix reference counting of PVRLayerChild when GPU process has been terminated - In VRManagerChild::DeallocPVRLayerChild, we delete the PVRLayerChild, rather than doing proper reference counting on the VRLayerChild object which has been deallocated but still referenced by VRDisplayPresentation::mLayers. This happens when the GPU process is terminated. - When the user tries to stop the VR presentation or change the URL, PVRLayerChild::SendDestroy is called by VRDisplayPresentation::DestroyLayers. This results in the assertion about "invalid actor state". - This patch fixes this by following the same pattern used by TextureClient for its PTextureChild. MozReview-Commit-ID: 327cdb4s7kl
gfx/vr/ipc/VRLayerChild.cpp
gfx/vr/ipc/VRLayerChild.h
gfx/vr/ipc/VRManagerChild.cpp
--- a/gfx/vr/ipc/VRLayerChild.cpp
+++ b/gfx/vr/ipc/VRLayerChild.cpp
@@ -10,23 +10,23 @@
 #include "SharedSurfaceGL.h"              // for SharedSurface
 #include "mozilla/layers/LayersMessages.h" // for TimedTexture
 #include "nsICanvasRenderingContextInternal.h"
 #include "mozilla/dom/HTMLCanvasElement.h"
 
 namespace mozilla {
 namespace gfx {
 
-VRLayerChild::VRLayerChild(uint32_t aVRDisplayID, VRManagerChild* aVRManagerChild)
-  : mVRDisplayID(aVRDisplayID)
-  , mCanvasElement(nullptr)
+VRLayerChild::VRLayerChild()
+  : mCanvasElement(nullptr)
   , mShSurfClient(nullptr)
   , mFront(nullptr)
-  , mIPCOpen(true)
+  , mIPCOpen(false)
 {
+  MOZ_COUNT_CTOR(VRLayerChild);
 }
 
 VRLayerChild::~VRLayerChild()
 {
   if (mCanvasElement) {
     mCanvasElement->StopVRPresentation();
   }
 
@@ -89,10 +89,39 @@ VRLayerChild::ClearSurfaces()
 }
 
 void
 VRLayerChild::ActorDestroy(ActorDestroyReason aWhy)
 {
   mIPCOpen = false;
 }
 
+// static
+PVRLayerChild*
+VRLayerChild::CreateIPDLActor()
+{
+  VRLayerChild* c = new VRLayerChild();
+  c->AddIPDLReference();
+  return c;
+}
+
+// static
+bool
+VRLayerChild::DestroyIPDLActor(PVRLayerChild* actor)
+{
+  static_cast<VRLayerChild*>(actor)->ReleaseIPDLReference();
+  return true;
+}
+
+void
+VRLayerChild::AddIPDLReference() {
+  MOZ_ASSERT(mIPCOpen == false);
+  mIPCOpen = true;
+  AddRef();
+}
+void
+VRLayerChild::ReleaseIPDLReference() {
+  MOZ_ASSERT(mIPCOpen == false);
+  Release();
+}
+
 } // namespace gfx
 } // namespace mozilla
--- a/gfx/vr/ipc/VRLayerChild.h
+++ b/gfx/vr/ipc/VRLayerChild.h
@@ -27,30 +27,38 @@ namespace gl {
 class SurfaceFactory;
 }
 namespace gfx {
 
 class VRLayerChild : public PVRLayerChild {
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(VRLayerChild)
 
 public:
-  VRLayerChild(uint32_t aVRDisplayID, VRManagerChild* aVRManagerChild);
+  static PVRLayerChild* CreateIPDLActor();
+  static bool DestroyIPDLActor(PVRLayerChild* actor);
+
   void Initialize(dom::HTMLCanvasElement* aCanvasElement);
   void SubmitFrame();
   bool IsIPCOpen();
 
-protected:
+private:
+  VRLayerChild();
   virtual ~VRLayerChild();
   void ClearSurfaces();
   virtual void ActorDestroy(ActorDestroyReason aWhy) override;
 
-  uint32_t mVRDisplayID;
-
   RefPtr<dom::HTMLCanvasElement> mCanvasElement;
   RefPtr<layers::SharedSurfaceTextureClient> mShSurfClient;
   RefPtr<layers::TextureClient> mFront;
   bool mIPCOpen;
+
+  // AddIPDLReference and ReleaseIPDLReference are only to be called by CreateIPDLActor
+  // and DestroyIPDLActor, respectively. We intentionally make them private to prevent misuse.
+  // The purpose of these methods is to be aware of when the IPC system around this
+  // actor goes down: mIPCOpen is then set to false.
+  void AddIPDLReference();
+  void ReleaseIPDLReference();
 };
 
 } // namespace gfx
 } // namespace mozilla
 
 #endif
--- a/gfx/vr/ipc/VRManagerChild.cpp
+++ b/gfx/vr/ipc/VRManagerChild.cpp
@@ -187,25 +187,23 @@ VRManagerChild::AllocPVRLayerChild(const
                                    const float& aLeftEyeWidth,
                                    const float& aLeftEyeHeight,
                                    const float& aRightEyeX,
                                    const float& aRightEyeY,
                                    const float& aRightEyeWidth,
                                    const float& aRightEyeHeight,
                                    const uint32_t& aGroup)
 {
-  RefPtr<VRLayerChild> layer = new VRLayerChild(aDisplayID, this);
-  return layer.forget().take();
+  return VRLayerChild::CreateIPDLActor();
 }
 
 bool
 VRManagerChild::DeallocPVRLayerChild(PVRLayerChild* actor)
 {
-  delete actor;
-  return true;
+  return VRLayerChild::DestroyIPDLActor(actor);
 }
 
 void
 VRManagerChild::UpdateDisplayInfo(nsTArray<VRDisplayInfo>& aDisplayUpdates)
 {
   nsTArray<uint32_t> disconnectedDisplays;
   nsTArray<uint32_t> connectedDisplays;