Bug 1443329 - Don't notify BHR if ForcePaint loses its race r?mconley draft
authorDoug Thayer <dothayer@mozilla.com>
Mon, 05 Mar 2018 15:29:34 -0800
changeset 769528 aa07caa8a38da941b0d8aed609f20edae3cec07f
parent 768133 748f4a1baef345a1db72774393b3e45fcd40a9a1
child 769529 f2098f068a503271cd5af2c766ab55d4fca350e5
child 769542 977153f5813c5657053d0df57558efb0d4b41e0d
child 769590 475c3eda424602f6466dad8ff4c1352466166600
push id103159
push userbmo:dothayer@mozilla.com
push dateMon, 19 Mar 2018 18:58:49 +0000
reviewersmconley
bugs1443329
milestone61.0a1
Bug 1443329 - Don't notify BHR if ForcePaint loses its race r?mconley MozReview-Commit-ID: 3QnYqjifBiE
dom/ipc/ProcessHangMonitor.cpp
dom/ipc/ProcessHangMonitor.h
dom/ipc/TabChild.cpp
--- a/dom/ipc/ProcessHangMonitor.cpp
+++ b/dom/ipc/ProcessHangMonitor.cpp
@@ -90,17 +90,17 @@ class HangMonitorChild
 
   bool IsDebuggerStartupComplete();
 
   void NotifyPluginHang(uint32_t aPluginId);
   void NotifyPluginHangAsync(uint32_t aPluginId);
 
   void ClearHang();
   void ClearHangAsync();
-  void ClearForcePaint();
+  void ClearForcePaint(uint64_t aLayerObserverEpoch);
 
   mozilla::ipc::IPCResult RecvTerminateScript(const bool& aTerminateGlobal) override;
   mozilla::ipc::IPCResult RecvBeginStartingDebugger() override;
   mozilla::ipc::IPCResult RecvEndStartingDebugger() override;
 
   mozilla::ipc::IPCResult RecvForcePaint(const TabId& aTabId, const uint64_t& aLayerObserverEpoch) override;
 
   void ActorDestroy(ActorDestroyReason aWhy) override;
@@ -417,37 +417,52 @@ HangMonitorChild::RecvEndStartingDebugge
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 HangMonitorChild::RecvForcePaint(const TabId& aTabId, const uint64_t& aLayerObserverEpoch)
 {
   MOZ_RELEASE_ASSERT(IsOnThread());
 
-  mForcePaintMonitor->NotifyActivity();
-
   {
     MonitorAutoLock lock(mMonitor);
+    // If we lose our race, and the main thread has already painted,
+    // the NotifyActivity call below would result in an indefinite
+    // hang, since it wouldn't have a matching NotifyWait()
+    if (mForcePaintEpoch >= aLayerObserverEpoch) {
+      return IPC_OK();
+    }
+    mForcePaintMonitor->NotifyActivity();
     mForcePaint = true;
     mForcePaintTab = aTabId;
     mForcePaintEpoch = aLayerObserverEpoch;
   }
 
   JS_RequestInterruptCallback(mContext);
 
   return IPC_OK();
 }
 
 void
-HangMonitorChild::ClearForcePaint()
+HangMonitorChild::ClearForcePaint(uint64_t aLayerObserverEpoch)
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
   MOZ_RELEASE_ASSERT(XRE_IsContentProcess());
 
-  mForcePaintMonitor->NotifyWait();
+  {
+    MonitorAutoLock lock(mMonitor);
+    // Set the epoch, so that if the forcepaint loses its race, it
+    // knows it and can exit appropriately. However, ensure we don't
+    // overwrite an even newer mForcePaintEpoch which could have
+    // come in in a ForcePaint notification while we were painting.
+    if (aLayerObserverEpoch > mForcePaintEpoch) {
+      mForcePaintEpoch = aLayerObserverEpoch;
+    }
+    mForcePaintMonitor->NotifyWait();
+  }
 }
 
 void
 HangMonitorChild::Bind(Endpoint<PProcessHangMonitorChild>&& aEndpoint)
 {
   MOZ_RELEASE_ASSERT(IsOnThread());
 
   MOZ_ASSERT(!sInstance);
@@ -1352,17 +1367,17 @@ ProcessHangMonitor::ForcePaint(PProcessH
                                uint64_t aLayerObserverEpoch)
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
   auto parent = static_cast<HangMonitorParent*>(aParent);
   parent->ForcePaint(aTabParent, aLayerObserverEpoch);
 }
 
 /* static */ void
-ProcessHangMonitor::ClearForcePaint()
+ProcessHangMonitor::ClearForcePaint(uint64_t aLayerObserverEpoch)
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
   MOZ_RELEASE_ASSERT(XRE_IsContentProcess());
 
   if (HangMonitorChild* child = HangMonitorChild::Get()) {
-    child->ClearForcePaint();
+    child->ClearForcePaint(aLayerObserverEpoch);
   }
 }
--- a/dom/ipc/ProcessHangMonitor.h
+++ b/dom/ipc/ProcessHangMonitor.h
@@ -43,17 +43,17 @@ class ProcessHangMonitor final
   static PProcessHangMonitorParent* AddProcess(dom::ContentParent* aContentParent);
   static void RemoveProcess(PProcessHangMonitorParent* aParent);
 
   static void ClearHang();
 
   static void ForcePaint(PProcessHangMonitorParent* aParent,
                          dom::TabParent* aTab,
                          uint64_t aLayerObserverEpoch);
-  static void ClearForcePaint();
+  static void ClearForcePaint(uint64_t aLayerObserverEpoch);
 
   enum SlowScriptAction {
     Continue,
     Terminate,
     StartDebugger,
     TerminateGlobal,
   };
   SlowScriptAction NotifySlowScript(nsITabChild* aTabChild,
--- a/dom/ipc/TabChild.cpp
+++ b/dom/ipc/TabChild.cpp
@@ -2573,17 +2573,17 @@ TabChild::RecvRenderLayers(const bool& a
 
   auto clearForcePaint = MakeScopeExit([&] {
     // We might force a paint, or we might already have painted and this is a
     // no-op. In either case, once we exit this scope, we need to alert the
     // ProcessHangMonitor that we've finished responding to what might have
     // been a request to force paint. This is so that the BackgroundHangMonitor
     // for force painting can be made to wait again.
     if (aEnabled) {
-      ProcessHangMonitor::ClearForcePaint();
+      ProcessHangMonitor::ClearForcePaint(mLayerObserverEpoch);
     }
   });
 
   if (mCompositorOptions) {
     MOZ_ASSERT(mPuppetWidget);
     RefPtr<LayerManager> lm = mPuppetWidget->GetLayerManager();
     MOZ_ASSERT(lm);