Bug 1443329 - Ensure we always call NotifyActivity on ForcePaint BHR r?mconley
MozReview-Commit-ID: GomQWREr7vG
--- a/dom/ipc/ProcessHangMonitor.cpp
+++ b/dom/ipc/ProcessHangMonitor.cpp
@@ -92,16 +92,22 @@ class HangMonitorChild
void NotifyPluginHang(uint32_t aPluginId);
void NotifyPluginHangAsync(uint32_t aPluginId);
void ClearHang();
void ClearHangAsync();
void ClearForcePaint(uint64_t aLayerObserverEpoch);
+ // MaybeStartForcePaint will notify the background hang monitor of activity
+ // if this is the first time calling it since ClearForcePaint. It should be
+ // callable from any thread, but you must be holding mMonitor if using it off
+ // the main thread, since it could race with ClearForcePaint.
+ void MaybeStartForcePaint();
+
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;
@@ -136,16 +142,20 @@ class HangMonitorChild
bool mForcePaint;
TabId mForcePaintTab;
MOZ_INIT_OUTSIDE_CTOR uint64_t mForcePaintEpoch;
JSContext* mContext;
bool mShutdownDone;
// This field is only accessed on the hang thread.
bool mIPCOpen;
+
+ // Allows us to ensure we NotifyActivity only once, allowing
+ // either thread to do so.
+ Atomic<bool> mBHRMonitorActive;
};
Atomic<HangMonitorChild*> HangMonitorChild::sInstance;
/* Parent process objects */
class HangMonitorParent;
@@ -425,43 +435,60 @@ HangMonitorChild::RecvForcePaint(const T
{
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();
+ MaybeStartForcePaint();
mForcePaint = true;
mForcePaintTab = aTabId;
mForcePaintEpoch = aLayerObserverEpoch;
}
JS_RequestInterruptCallback(mContext);
return IPC_OK();
}
void
+HangMonitorChild::MaybeStartForcePaint()
+{
+ if (!NS_IsMainThread()) {
+ mMonitor.AssertCurrentThreadOwns();
+ }
+
+ if (!mBHRMonitorActive.exchange(true)) {
+ mForcePaintMonitor->NotifyActivity();
+ }
+}
+
+void
HangMonitorChild::ClearForcePaint(uint64_t aLayerObserverEpoch)
{
MOZ_RELEASE_ASSERT(NS_IsMainThread());
MOZ_RELEASE_ASSERT(XRE_IsContentProcess());
{
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();
+
+ // ClearForcePaint must be called on the main thread, and the
+ // hang monitor thread only sets this with mMonitor held, so there
+ // should be no risk of missing NotifyActivity calls here.
+ mBHRMonitorActive = false;
}
}
void
HangMonitorChild::Bind(Endpoint<PProcessHangMonitorChild>&& aEndpoint)
{
MOZ_RELEASE_ASSERT(IsOnThread());
@@ -1376,8 +1403,19 @@ ProcessHangMonitor::ClearForcePaint(uint
{
MOZ_RELEASE_ASSERT(NS_IsMainThread());
MOZ_RELEASE_ASSERT(XRE_IsContentProcess());
if (HangMonitorChild* child = HangMonitorChild::Get()) {
child->ClearForcePaint(aLayerObserverEpoch);
}
}
+
+/* static */ void
+ProcessHangMonitor::MaybeStartForcePaint()
+{
+ MOZ_RELEASE_ASSERT(NS_IsMainThread());
+ MOZ_RELEASE_ASSERT(XRE_IsContentProcess());
+
+ if (HangMonitorChild* child = HangMonitorChild::Get()) {
+ child->MaybeStartForcePaint();
+ }
+}
--- a/dom/ipc/ProcessHangMonitor.h
+++ b/dom/ipc/ProcessHangMonitor.h
@@ -44,16 +44,17 @@ class ProcessHangMonitor final
static void RemoveProcess(PProcessHangMonitorParent* aParent);
static void ClearHang();
static void ForcePaint(PProcessHangMonitorParent* aParent,
dom::TabParent* aTab,
uint64_t aLayerObserverEpoch);
static void ClearForcePaint(uint64_t aLayerObserverEpoch);
+ static void MaybeStartForcePaint();
enum SlowScriptAction {
Continue,
Terminate,
StartDebugger,
TerminateGlobal,
};
SlowScriptAction NotifySlowScript(nsITabChild* aTabChild,
--- a/dom/ipc/TabChild.cpp
+++ b/dom/ipc/TabChild.cpp
@@ -2577,16 +2577,20 @@ TabChild::RecvRenderLayers(const bool& a
// 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(mLayerObserverEpoch);
}
});
+ if (aEnabled) {
+ ProcessHangMonitor::MaybeStartForcePaint();
+ }
+
if (mCompositorOptions) {
MOZ_ASSERT(mPuppetWidget);
RefPtr<LayerManager> lm = mPuppetWidget->GetLayerManager();
MOZ_ASSERT(lm);
// We send the current layer observer epoch to the compositor so that
// TabParent knows whether a layer update notification corresponds to the
// latest RecvRenderLayers request that was made.