From e3503a74b925d9ecbeaaacfa045018f8bb1218a4 Mon Sep 17 00:00:00 2001 From: Brian Hackett Date: Wed, 31 Oct 2018 14:57:01 -1000 Subject: [PATCH] Bug 1503750 - Add preference to allow crashing on repaint failures, r=mccr8. --HG-- extra : rebase_source : df926fba3f291e2a56975936d1090c7d0ce54c60 --- modules/libpref/init/all.js | 1 + toolkit/recordreplay/ProcessRedirect.cpp | 9 ++++++--- toolkit/recordreplay/ProcessRewind.cpp | 8 ++++---- toolkit/recordreplay/ipc/ChildIPC.cpp | 25 ++++++++++++++++++++++++ toolkit/recordreplay/ipc/ChildInternal.h | 7 ++++--- 5 files changed, 40 insertions(+), 10 deletions(-) diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 1ac07c5168a6..431412a5eb70 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -1110,6 +1110,7 @@ pref("devtools.recordreplay.enableRewinding", true); pref("devtools.recordreplay.mvp.enabled", false); pref("devtools.recordreplay.timeline.enabled", false); +pref("devtools.recordreplay.allowRepaintFailures", true); // view source pref("view_source.syntax_highlight", true); diff --git a/toolkit/recordreplay/ProcessRedirect.cpp b/toolkit/recordreplay/ProcessRedirect.cpp index 8d27457efb3a..3ed662c8575e 100644 --- a/toolkit/recordreplay/ProcessRedirect.cpp +++ b/toolkit/recordreplay/ProcessRedirect.cpp @@ -8,6 +8,7 @@ #include "InfallibleVector.h" #include "MiddlemanCall.h" +#include "ipc/ChildInternal.h" #include "ipc/ParentInternal.h" #include "mozilla/Sprintf.h" @@ -100,9 +101,11 @@ RecordReplayInterceptCall(int aCallId, CallArguments* aArguments) } } - if (parent::InRepaintStressMode()) { - // We're about to crash, so print out the name of the call that failed. - Print("Could not perform middleman call: %s\n", redirection.mName); + if (child::CurrentRepaintCannotFail()) { + // EnsureNotDivergedFromRecording is going to force us to crash, so fail + // earlier with a more helpful error message. + child::ReportFatalError(Nothing(), "Could not perform middleman call: %s\n", + redirection.mName); } // Calling any redirection which performs the standard steps will cause diff --git a/toolkit/recordreplay/ProcessRewind.cpp b/toolkit/recordreplay/ProcessRewind.cpp index d0cd978b5ea3..715b543bd26f 100644 --- a/toolkit/recordreplay/ProcessRewind.cpp +++ b/toolkit/recordreplay/ProcessRewind.cpp @@ -237,10 +237,10 @@ EnsureNotDivergedFromRecording() if (HasDivergedFromRecording()) { MOZ_RELEASE_ASSERT(gUnhandledDivergeAllowed); - // Crash instead of rewinding in the painting stress mode, for finding - // areas where middleman calls do not cover all painting logic. - if (parent::InRepaintStressMode()) { - MOZ_CRASH("Recording divergence in repaint stress mode"); + // Crash instead of rewinding if a repaint is about to fail and is not + // allowed. + if (child::CurrentRepaintCannotFail()) { + MOZ_CRASH("Recording divergence while repainting"); } PrintSpew("Unhandled recording divergence, restoring checkpoint...\n"); diff --git a/toolkit/recordreplay/ipc/ChildIPC.cpp b/toolkit/recordreplay/ipc/ChildIPC.cpp index ce90329cf461..bb1421a0e50a 100644 --- a/toolkit/recordreplay/ipc/ChildIPC.cpp +++ b/toolkit/recordreplay/ipc/ChildIPC.cpp @@ -475,6 +475,9 @@ static Atomic gNumPendi // ID of the compositor thread. static Atomic gCompositorThreadId; +// Whether repaint failures are allowed, or if the process should crash. +static bool gAllowRepaintFailures; + already_AddRefed DrawTargetForRemoteDrawing(LayoutDeviceIntSize aSize) { @@ -522,6 +525,17 @@ NotifyPaintStart() { MOZ_RELEASE_ASSERT(NS_IsMainThread()); + // Initialize state on the first paint. + static bool gPainted; + if (!gPainted) { + gPainted = true; + + // Repaint failures are not allowed in the repaint stress mode. + gAllowRepaintFailures = + Preferences::GetBool("devtools.recordreplay.allowRepaintFailures") && + !parent::InRepaintStressMode(); + } + // A new paint cannot be triggered until the last one finishes and has been // sent to the middleman. MOZ_RELEASE_ASSERT(HasDivergedFromRecording() || !gHasActivePaint); @@ -571,6 +585,9 @@ NotifyPaintComplete() // Whether we have repainted since diverging from the recording. static bool gDidRepaint; +// Whether we are currently repainting. +static bool gRepainting; + void Repaint(size_t* aWidth, size_t* aHeight) { @@ -588,6 +605,7 @@ Repaint(size_t* aWidth, size_t* aHeight) // case the last graphics we sent will still be correct. if (!gDidRepaint) { gDidRepaint = true; + gRepainting = true; // Allow other threads to diverge from the recording so the compositor can // perform any paint we are about to trigger, or finish any in flight paint @@ -611,6 +629,7 @@ Repaint(size_t* aWidth, size_t* aHeight) } Thread::WaitForIdleThreads(); + gRepainting = false; } if (gDrawTargetBuffer) { @@ -623,6 +642,12 @@ Repaint(size_t* aWidth, size_t* aHeight) } } +bool +CurrentRepaintCannotFail() +{ + return gRepainting && !gAllowRepaintFailures; +} + /////////////////////////////////////////////////////////////////////////////// // Checkpoint Messages /////////////////////////////////////////////////////////////////////////////// diff --git a/toolkit/recordreplay/ipc/ChildInternal.h b/toolkit/recordreplay/ipc/ChildInternal.h index 719d7bc20bf6..2f3c9bfb3df1 100644 --- a/toolkit/recordreplay/ipc/ChildInternal.h +++ b/toolkit/recordreplay/ipc/ChildInternal.h @@ -115,9 +115,6 @@ void NotifyFlushedRecording(); // Notify the middleman about an AlwaysMarkMajorCheckpoints directive. void NotifyAlwaysMarkMajorCheckpoints(); -// Report a fatal error to the middleman process. -void ReportFatalError(const char* aFormat, ...); - // Mark a time span when the main thread is idle. void BeginIdleTime(); void EndIdleTime(); @@ -130,6 +127,10 @@ void SendMiddlemanCallRequest(const char* aInputData, size_t aInputSize, InfallibleVector* aOutputData); void SendResetMiddlemanCalls(); +// Return whether a repaint is in progress and is not allowed to trigger an +// unhandled recording divergence per preferences. +bool CurrentRepaintCannotFail(); + } // namespace child } // namespace recordreplay