Add preference to allow crashing on repaint failures

RESOLVED FIXED in Firefox 65

Status

()

enhancement
RESOLVED FIXED
8 months ago
7 months ago

People

(Reporter: bhackett, Assigned: bhackett)

Tracking

unspecified
mozilla65
Points:
---

Firefox Tracking Flags

(firefox65 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

8 months ago
Posted 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+

Comment 2

7 months ago
Pushed by bhackett@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/584368614d2c
Add preference to allow crashing on repaint failures, r=mccr8.

Comment 3

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/584368614d2c
Status: NEW → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in before you can comment on or make changes to this bug.