1411132 - Enabling layout.display-list.retain causes tabs to crash in nsDisplayBackgroundColor::GetOpaqueRegion const

Status: VERIFIED FIXED

(Core :: Web Painting, defect)

Description

[overdodactyl]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:52.0) Gecko/20100101 Firefox/52.0
Build ID: 20100101

Steps to reproduce:

Set the value of layout.display-list.retain to true in about:config

Visit any web page (either new ones or ones open prior to changing the setting).


Actual results:

Each tab will crash independently after spending 5-10 seconds on the site.

Reproducible in a new profile.

A few sample crashes ID's are listed below:

bp-6815eff5-ed87-4a66-b95f-f475e0171024

bp-5c69d538-430e-41ba-af03-bf8190171024

Comment 1

[Arjen de Jager]

Slightly different crash signature: [@ nsDisplayBackgroundColor::GetOpaqueRegion ]

bp-3bf85398-9597-4fdd-b420-7ce350171024
bp-8ce21f3b-8b03-4915-aad7-4bfd20171024
bp-ef27b04d-af07-40a0-98f5-d3e8f0171024

Comment 2

[Miko Mynttinen]

Thank you for the report. As building a partial display list requires information that is stored in-between paints, there is no easy/efficient way to support changing this preference on the fly.

The attached patch changes the pref update policy to Once (which it should have been from the beginning), meaning changing it requires a restart.

Comment 3

[Miko Mynttinen]

Attachment: Bug 1411132 - Change layout.display-list.retain UpdatePolicy to Once

Review commit: https://reviewboard.mozilla.org/r/192538/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/192538/

Comment 4

[Marcia Knous [:marcia]]

Adding additional signatures in Socorro so that they refer back to this bug.

Comment 5

[Marcia Knous [:marcia]]

(In reply to Marcia Knous [:marcia - use ni] from comment #4)
> Adding additional signatures in Socorro so that they refer back to this bug.

There are a fair number of people hitting some of these signatures starting with the 10-24 build. Are all of these people enabling the pref?

Comment 6

[Miko Mynttinen]

(In reply to Marcia Knous [:marcia - use ni] from comment #5)
> (In reply to Marcia Knous [:marcia - use ni] from comment #4)
> > Adding additional signatures in Socorro so that they refer back to this bug.
> 
> There are a fair number of people hitting some of these signatures starting
> with the 10-24 build. Are all of these people enabling the pref?

The pref is disabled in all.js, so this should be the case. The pref was advertised on reddit yesterday, which would explain the numbers (in tens/hundreds).

Comment 7

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8921511 [details]
Bug 1411132 - Change layout.display-list.retain UpdatePolicy to Once

https://reviewboard.mozilla.org/r/192538/#review197732

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cb757b237396
Change layout.display-list.retain UpdatePolicy to Once r=mattwoodrow

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/cb757b237396

Comment 10

[Calixte Denizet (:calixte)]

The crash signatures:
 - "nsDisplayBackgroundColor::GetOpaqueRegion" (28 crashes, see [1]);
 - "nsDisplayBackgroundColor::GetOpaqueRegion const" (14 crashes, see [2]);
 - "nsStyleImage::IsResolved" (4 crashes, see [3])
are still there in nightly 58 with buildid 20171025100449.

[1] http://bit.ly/2h9vLpB
[2] http://bit.ly/2i4rpje
[3] http://bit.ly/2gKmx6D

Comment 11

[John Doe]

Reproduced on a clean profile.

1. Open the following website:
https://www.neowin.net/news/iphone-x-shows-up-in-new-video-showing-off-its-stutter-free-app-switching
2. Play the linked twitter video
3. Flickering, followed by a crash. If it doesn't try moving the mouse on and off the video.

Crash report id: c9466721-695b-456b-afb7-6b5c50171026

Comment 12

[Calixte Denizet (:calixte)]

:mattwoodrow, could you investigate please ?

Comment 13

[Nicholas Nethercote [inactive]]

This is the #2 Mac topcrash in Nightly 20171027100103, with 18 occurrences across several installations, which is pretty high for Mac Nightly.

Comment 14

[Nicholas Nethercote [inactive]]

And the #3 Linux topcrash in the same Nightly.

Comment 15

[Matt Woodrow (:mattwoodrow)]

Attachment: Recurse into merging for reused old items

We need to make sure we run the merge algorithm for reused old items, even when they don't have a matching new item. There can be an item in the sublist that is invalidated (and didn't get a new matching item created), and we need to make sure we find and remove it.

This fixes the crash, and bug 1411881 fixes the flicking.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f26525123e6a
Always recurse into merging for sub-lists, even when there isn't a matching new item, so that we find all items that need invalidating. r=ethanlin

Comment 17

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/b666ea1ac903 because nothing could figure out what you were talking about with 'aStats', https://treeherder.mozilla.org/logviewer.html#?job_id=140616728&repo=mozilla-inbound

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd40c8a5af54
Always recurse into merging for sub-lists, even when there isn't a matching new item, so that we find all items that need invalidating. r=ethanlin

Comment 19

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/dd40c8a5af54

Comment 20

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/dd40c8a5af54

Comment 21

[Emil Ghitta, QA [:emilghitta]]

I have managed to reproduce the issue described in comment 0 by following the steps provided in comment 11 using Firefox 58.0a1 (BuildId:20171023220222)

This issue is verified fixed using Firefox 58.0b5 (BuildId:20171120142222) on Windows 10 64bit, macOS 10.12.6 and Ubuntu 16.04 64bit.