Bug 1543560 - Move NotifyWebRenderError() to CompositorManagerParent r=nical
authorsotaro <sotaro.ikeda.g@gmail.com>
Tue, 16 Apr 2019 07:45:00 +0000
changeset 469644 719725292e4c
parent 469643 4c15baa5aa95
child 469645 978eb962906f
push id35878
push userapavel@mozilla.com
push dateTue, 16 Apr 2019 15:43:40 +0000
treeherdermozilla-central@258af4e91151 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnical
bugs1543560
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 1543560 - Move NotifyWebRenderError() to CompositorManagerParent r=nical NotifyWebRenderError is sent via CompositorBridgeParent/CompositorBridgeChild. It is not good, since they are re-created for each nsBaseWidget::CreateCompositorSession() call. Then there could be a case that the NotifyWebRenderError is not notified to GPUProcessManager and a crash was caused by NotifyWebRenderError message during ipc close. CompositorManagerParent/CompositorManagerChild are stable for sending NotifyWebRenderError. Differential Revision: https://phabricator.services.mozilla.com/D27030
gfx/layers/ipc/CompositorBridgeChild.cpp
gfx/layers/ipc/CompositorBridgeChild.h
gfx/layers/ipc/CompositorBridgeParent.cpp
gfx/layers/ipc/CompositorBridgeParent.h
gfx/layers/ipc/CompositorManagerChild.cpp
gfx/layers/ipc/CompositorManagerChild.h
gfx/layers/ipc/CompositorManagerParent.cpp
gfx/layers/ipc/CompositorManagerParent.h
gfx/layers/ipc/PCompositorBridge.ipdl
gfx/layers/ipc/PCompositorManager.ipdl
gfx/webrender_bindings/RendererOGL.cpp
--- a/gfx/layers/ipc/CompositorBridgeChild.cpp
+++ b/gfx/layers/ipc/CompositorBridgeChild.cpp
@@ -804,23 +804,16 @@ mozilla::ipc::IPCResult CompositorBridge
 
   if (RefPtr<dom::TabParent> tab =
           dom::TabParent::GetTabParentFromLayersId(aLayersId)) {
     tab->LayerTreeUpdate(aEpoch, aActive);
   }
   return IPC_OK();
 }
 
-mozilla::ipc::IPCResult CompositorBridgeChild::RecvNotifyWebRenderError(
-    const WebRenderError& aError) {
-  MOZ_ASSERT(XRE_IsParentProcess());
-  GPUProcessManager::Get()->NotifyWebRenderError(aError);
-  return IPC_OK();
-}
-
 void CompositorBridgeChild::HoldUntilCompositableRefReleasedIfNecessary(
     TextureClient* aClient) {
   if (!aClient) {
     return;
   }
 
   if (!(aClient->GetFlags() & TextureFlags::RECYCLE)) {
     return;
--- a/gfx/layers/ipc/CompositorBridgeChild.h
+++ b/gfx/layers/ipc/CompositorBridgeChild.h
@@ -277,19 +277,16 @@ class CompositorBridgeChild final : publ
       const ViewID& aId, const uint32_t& aAPZCId);
 
   mozilla::ipc::IPCResult RecvRemotePaintIsReady();
 
   mozilla::ipc::IPCResult RecvObserveLayersUpdate(
       const LayersId& aLayersId, const LayersObserverEpoch& aEpoch,
       const bool& aActive);
 
-  mozilla::ipc::IPCResult RecvNotifyWebRenderError(
-      const WebRenderError& aError);
-
   uint64_t GetNextResourceId();
 
   void ClearSharedFrameMetricsData(LayersId aLayersId);
 
   // Class used to store the shared FrameMetrics, mutex, and APZCId  in a hash
   // table
   class SharedFrameMetricsData final {
    public:
--- a/gfx/layers/ipc/CompositorBridgeParent.cpp
+++ b/gfx/layers/ipc/CompositorBridgeParent.cpp
@@ -2267,21 +2267,16 @@ PTextureParent* CompositorBridgeParent::
 bool CompositorBridgeParent::DeallocPTextureParent(PTextureParent* actor) {
   return TextureHost::DestroyIPDLActor(actor);
 }
 
 bool CompositorBridgeParent::IsSameProcess() const {
   return OtherPid() == base::GetCurrentProcId();
 }
 
-void CompositorBridgeParent::NotifyWebRenderError(wr::WebRenderError aError) {
-  MOZ_ASSERT(CompositorLoop() == MessageLoop::current());
-  Unused << SendNotifyWebRenderError(aError);
-}
-
 void CompositorBridgeParent::NotifyWebRenderContextPurge() {
   MOZ_ASSERT(CompositorLoop() == MessageLoop::current());
   RefPtr<wr::WebRenderAPI> api =
       mWrBridge->GetWebRenderAPI(wr::RenderRoot::Default);
   api->ClearAllCaches();
 }
 
 #if defined(XP_WIN) || defined(MOZ_WIDGET_GTK)
--- a/gfx/layers/ipc/CompositorBridgeParent.h
+++ b/gfx/layers/ipc/CompositorBridgeParent.h
@@ -386,17 +386,16 @@ class CompositorBridgeParent final : pub
       const SurfaceDescriptor& aSharedData, const ReadLockDescriptor& aReadLock,
       const LayersBackend& aLayersBackend, const TextureFlags& aFlags,
       const LayersId& aId, const uint64_t& aSerial,
       const wr::MaybeExternalImageId& aExternalImageId) override;
   bool DeallocPTextureParent(PTextureParent* actor) override;
 
   bool IsSameProcess() const override;
 
-  void NotifyWebRenderError(wr::WebRenderError aError);
   void NotifyWebRenderContextPurge();
   void NotifyPipelineRendered(const wr::PipelineId& aPipelineId,
                               const wr::Epoch& aEpoch,
                               const VsyncId& aCompositeStartId,
                               TimeStamp& aCompositeStart,
                               TimeStamp& aRenderStart, TimeStamp& aCompositeEnd,
                               wr::RendererStats* aStats = nullptr);
   void NotifyDidSceneBuild(const nsTArray<wr::RenderRoot>& aRenderRoots,
--- a/gfx/layers/ipc/CompositorManagerChild.cpp
+++ b/gfx/layers/ipc/CompositorManagerChild.cpp
@@ -278,10 +278,18 @@ bool CompositorManagerChild::ShouldConti
   if (XRE_IsParentProcess()) {
     gfxCriticalNote << "Killing GPU process due to IPC reply timeout";
     MOZ_DIAGNOSTIC_ASSERT(GPUProcessManager::Get()->GetGPUChild());
     GPUProcessManager::Get()->KillProcess();
   }
   return false;
 }
 
+mozilla::ipc::IPCResult CompositorManagerChild::RecvNotifyWebRenderError(
+    const WebRenderError&& aError) {
+  MOZ_ASSERT(XRE_IsParentProcess());
+  MOZ_ASSERT(NS_IsMainThread());
+  GPUProcessManager::Get()->NotifyWebRenderError(aError);
+  return IPC_OK();
+}
+
 }  // namespace layers
 }  // namespace mozilla
--- a/gfx/layers/ipc/CompositorManagerChild.h
+++ b/gfx/layers/ipc/CompositorManagerChild.h
@@ -78,16 +78,19 @@ class CompositorManagerChild : public PC
 
   PCompositorBridgeChild* AllocPCompositorBridgeChild(
       const CompositorBridgeOptions& aOptions);
 
   bool DeallocPCompositorBridgeChild(PCompositorBridgeChild* aActor);
 
   bool ShouldContinueFromReplyTimeout() override;
 
+  mozilla::ipc::IPCResult RecvNotifyWebRenderError(
+      const WebRenderError&& aError);
+
  private:
   static StaticRefPtr<CompositorManagerChild> sInstance;
 
   CompositorManagerChild(CompositorManagerParent* aParent,
                          uint64_t aProcessToken, uint32_t aNamespace);
 
   CompositorManagerChild(Endpoint<PCompositorManagerChild>&& aEndpoint,
                          uint64_t aProcessToken, uint32_t aNamespace);
--- a/gfx/layers/ipc/CompositorManagerParent.cpp
+++ b/gfx/layers/ipc/CompositorManagerParent.cpp
@@ -6,16 +6,17 @@
 
 #include "mozilla/layers/CompositorManagerParent.h"
 #include "mozilla/gfx/GPUParent.h"
 #include "mozilla/webrender/RenderThread.h"
 #include "mozilla/layers/CompositorBridgeParent.h"
 #include "mozilla/layers/ContentCompositorBridgeParent.h"
 #include "mozilla/layers/CompositorThread.h"
 #include "mozilla/layers/SharedSurfacesParent.h"
+#include "mozilla/Unused.h"
 #include "nsAutoPtr.h"
 #include "VsyncSource.h"
 
 namespace mozilla {
 namespace layers {
 
 StaticRefPtr<CompositorManagerParent> CompositorManagerParent::sInstance;
 StaticMutex CompositorManagerParent::sMutex;
@@ -123,16 +124,19 @@ void CompositorManagerParent::Bind(
   if (NS_WARN_IF(!aEndpoint.Bind(this))) {
     return;
   }
 
   BindComplete();
 }
 
 void CompositorManagerParent::BindComplete() {
+  MOZ_ASSERT(CompositorThreadHolder::IsInCompositorThread() ||
+             NS_IsMainThread());
+
   // Add the IPDL reference to ourself, so we can't get freed until IPDL is
   // done with us.
   AddRef();
 
   StaticMutexAutoLock lock(sMutex);
   if (OtherPid() == base::GetCurrentProcId()) {
     sInstance = this;
   }
@@ -317,10 +321,21 @@ mozilla::ipc::IPCResult CompositorManage
       },
       [](bool) {
         MOZ_ASSERT_UNREACHABLE("MemoryReport promises are never rejected");
       });
 
   return IPC_OK();
 }
 
+/* static */
+void CompositorManagerParent::NotifyWebRenderError(wr::WebRenderError aError) {
+  MOZ_ASSERT(CompositorThreadHolder::IsInCompositorThread());
+
+  StaticMutexAutoLock lock(sMutex);
+  if (NS_WARN_IF(!sInstance)) {
+    return;
+  }
+  Unused << sInstance->SendNotifyWebRenderError(aError);
+}
+
 }  // namespace layers
 }  // namespace mozilla
--- a/gfx/layers/ipc/CompositorManagerParent.h
+++ b/gfx/layers/ipc/CompositorManagerParent.h
@@ -52,16 +52,18 @@ class CompositorManagerParent final : pu
 
   void BindComplete();
   void ActorDestroy(ActorDestroyReason aReason) override;
 
   bool DeallocPCompositorBridgeParent(PCompositorBridgeParent* aActor);
   PCompositorBridgeParent* AllocPCompositorBridgeParent(
       const CompositorBridgeOptions& aOpt);
 
+  static void NotifyWebRenderError(wr::WebRenderError aError);
+
  private:
   static StaticRefPtr<CompositorManagerParent> sInstance;
   static StaticMutex sMutex;
 
 #ifdef COMPOSITOR_MANAGER_PARENT_EXPLICIT_SHUTDOWN
   static StaticAutoPtr<nsTArray<CompositorManagerParent*>> sActiveActors;
   static void ShutdownInternal();
 #endif
--- a/gfx/layers/ipc/PCompositorBridge.ipdl
+++ b/gfx/layers/ipc/PCompositorBridge.ipdl
@@ -36,17 +36,16 @@ using mozilla::LayoutDeviceIntSize from 
 using class mozilla::TimeStamp from "mozilla/TimeStamp.h";
 using class mozilla::layers::FrameUniformityData from "mozilla/layers/FrameUniformityData.h";
 using mozilla::layers::TextureFlags from "mozilla/layers/CompositorTypes.h";
 using mozilla::layers::CompositorOptions from "mozilla/layers/CompositorOptions.h";
 using mozilla::wr::PipelineId from "mozilla/webrender/WebRenderTypes.h";
 using mozilla::wr::IdNamespace from "mozilla/webrender/WebRenderTypes.h";
 using base::ProcessId from "base/process.h";
 using mozilla::wr::MaybeExternalImageId from "mozilla/webrender/WebRenderTypes.h";
-using mozilla::wr::WebRenderError from "mozilla/webrender/WebRenderTypes.h";
 using mozilla::layers::LayersObserverEpoch from "mozilla/layers/LayersTypes.h";
 using mozilla::layers::TransactionId from "mozilla/layers/LayersTypes.h";
 
 namespace mozilla {
 namespace layers {
 
 struct FrameStats {
   TransactionId id;
@@ -267,13 +266,12 @@ parent:
   async BeginRecording(TimeStamp aRecordingStart);
   async EndRecording();
 
 child:
   // Send back Compositor Frame Metrics from APZCs so tiled layers can
   // update progressively.
   async SharedCompositorFrameMetrics(Handle metrics, CrossProcessMutexHandle mutex, LayersId aLayersId, uint32_t aAPZCId);
   async ReleaseSharedCompositorFrameMetrics(ViewID aId, uint32_t aAPZCId);
-  async NotifyWebRenderError(WebRenderError error);
 };
 
 } // layers
 } // mozilla
--- a/gfx/layers/ipc/PCompositorManager.ipdl
+++ b/gfx/layers/ipc/PCompositorManager.ipdl
@@ -13,16 +13,17 @@ include "mozilla/layers/WebRenderMessage
 using struct mozilla::void_t from "ipc/IPCMessageUtils.h";
 using mozilla::TimeDuration from "mozilla/TimeStamp.h";
 using mozilla::CSSToLayoutDeviceScale from "Units.h";
 using mozilla::gfx::IntSize from "mozilla/gfx/2D.h";
 using mozilla::ipc::SharedMemoryBasic::Handle from "mozilla/ipc/SharedMemoryBasic.h";
 using mozilla::layers::CompositorOptions from "mozilla/layers/CompositorOptions.h";
 using mozilla::wr::ExternalImageId from "mozilla/webrender/WebRenderTypes.h";
 using mozilla::wr::MemoryReport from "mozilla/webrender/WebRenderTypes.h";
+using mozilla::wr::WebRenderError from "mozilla/webrender/WebRenderTypes.h";
 using mozilla::layers::SharedSurfacesMemoryReport from "mozilla/layers/SharedSurfacesMemoryReport.h";
 
 namespace mozilla {
 namespace layers {
 
 struct WidgetCompositorOptions {
   CSSToLayoutDeviceScale scale;
   TimeDuration vsyncRate;
@@ -77,12 +78,15 @@ parent:
 
   async AddSharedSurface(ExternalImageId aId, SurfaceDescriptorShared aDesc);
   async RemoveSharedSurface(ExternalImageId aId);
   async ReportSharedSurfacesMemory() returns (SharedSurfacesMemoryReport aReport);
 
   async NotifyMemoryPressure();
 
   async ReportMemory() returns (MemoryReport aReport);
+
+child:
+  async NotifyWebRenderError(WebRenderError error);
 };
 
 } // layers
 } // mozilla
--- a/gfx/webrender_bindings/RendererOGL.cpp
+++ b/gfx/webrender_bindings/RendererOGL.cpp
@@ -5,16 +5,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "RendererOGL.h"
 #include "GLContext.h"
 #include "mozilla/gfx/Logging.h"
 #include "mozilla/gfx/gfxVars.h"
 #include "mozilla/gfx/Types.h"
 #include "mozilla/layers/CompositorBridgeParent.h"
+#include "mozilla/layers/CompositorManagerParent.h"
 #include "mozilla/layers/CompositorThread.h"
 #include "mozilla/layers/LayersTypes.h"
 #include "mozilla/webrender/RenderCompositor.h"
 #include "mozilla/webrender/RenderTextureHost.h"
 #include "mozilla/widget/CompositorWidget.h"
 
 namespace mozilla {
 namespace wr {
@@ -209,21 +210,19 @@ void RendererOGL::AccumulateMemoryReport
   // Assume BGRA8 for the format since it's not exposed anywhere,
   // and all compositor backends should be using that.
   uintptr_t swapChainSize = size.width * size.height *
                             BytesPerPixel(SurfaceFormat::B8G8R8A8) *
                             (mCompositor->UseTripleBuffering() ? 3 : 2);
   aReport->swap_chain += swapChainSize;
 }
 
-static void DoNotifyWebRenderError(layers::CompositorBridgeParent* aBridge,
-                                   WebRenderError aError) {
-  aBridge->NotifyWebRenderError(aError);
+static void DoNotifyWebRenderError(WebRenderError aError) {
+  layers::CompositorManagerParent::NotifyWebRenderError(aError);
 }
 
 void RendererOGL::NotifyWebRenderError(WebRenderError aError) {
-  layers::CompositorThreadHolder::Loop()->PostTask(
-      NewRunnableFunction("DoNotifyWebRenderErrorRunnable",
-                          &DoNotifyWebRenderError, mBridge, aError));
+  layers::CompositorThreadHolder::Loop()->PostTask(NewRunnableFunction(
+      "DoNotifyWebRenderErrorRunnable", &DoNotifyWebRenderError, aError));
 }
 
 }  // namespace wr
 }  // namespace mozilla