Closed Bug 1503750 Opened 2 years ago Closed 2 years ago

Add preference to allow crashing on repaint failures

Categories

(Core Graveyard :: Web Replay, enhancement)

enhancement
Not set
normal

Tracking

(firefox65 fixed)

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: bhackett1024, Assigned: bhackett1024)

Details

Attachments

(1 file)

Attached patch patchSplinter Review
When a repaint fails while using Web Replay, we show the last painted graphics with a red "PAINT FAILURE" message scribbled in the upper left.  While nice for stability this isn't very helpful for figuring out why the failure occurred, and especially after bug 1503639 we'll want to find the causes of these and drive them to zero.  Still, for now I'm hesitant about always crashing on repaint failures.  The attached patch adds a preference, devtools.recordreplay.allowRepaintFailures, to control this behavior.
Attachment #9021716 - Flags: review?(continuation)
Comment on attachment 9021716 [details] [diff] [review]
patch

Review of attachment 9021716 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/recordreplay/ipc/ChildIPC.cpp
@@ +529,5 @@
> +  static bool gPainted;
> +  if (!gPainted) {
> +    gPainted = true;
> +
> +    gAllowRepaintFailures = Preferences::GetBool("devtools.recordreplay.allowRepaintFailures");

Doing the pref like this means users won't be able to update it while the browser is running, but I guess for a very experimental thing like this that is fine.

@@ +532,5 @@
> +
> +    gAllowRepaintFailures = Preferences::GetBool("devtools.recordreplay.allowRepaintFailures");
> +
> +    // Repaint failures are not allowed in the repaint stress mode.
> +    if (parent::InRepaintStressMode()) {

This feels like a slightly weird way to combine these two.

Maybe something like
gAllowRepaintFailures =
  Preferences::GetBool("devtools.recordreplay.allowRepaintFailures") &&
  !parent::InRepaintStressMode();
instead? I suppose that's an overly long expression.
Attachment #9021716 - Flags: review?(continuation) → review+
Pushed by bhackett@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/584368614d2c
Add preference to allow crashing on repaint failures, r=mccr8.
https://hg.mozilla.org/mozilla-central/rev/584368614d2c
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.