1469746 - Crash in MergeState::HasMatchingItemInOldList when plugins are added to the page

Status: RESOLVED FIXED

(Core :: Web Painting, defect, P2)

Description

[Matt Woodrow (:mattwoodrow)]

See URL, reported in bug 1466010.

We crash when we toggle the value of nsDisplayListBuilder::WillComputePluginGeometry() between builds, since it changes the behaviour of BuildDisplayListForStackingContext (with opacity:0) without an invalidation.

Comment 1

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1469746 - Rebuild display lists in full when we start/stop computing visibility for plugins.

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

Comment 2

[Miko Mynttinen]

Comment on attachment 8986348 [details]
Bug 1469746 - Rebuild display lists in full when we start/stop computing visibility for plugins.

https://reviewboard.mozilla.org/r/251716/#review258102

LGTM.

::: layout/base/nsLayoutUtils.cpp:3603
(Diff revision 1)
>    if ((aFlags & PaintFrameFlags::PAINT_WIDGET_LAYERS) &&
>        !(aFlags & PaintFrameFlags::PAINT_DOCUMENT_RELATIVE) &&
>        rootPresContext->NeedToComputePluginGeometryUpdates()) {
>      builder.SetWillComputePluginGeometry(true);
> +
> +    // Disable partial updates for this paint as the list we're about to

Great comments!

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/97d477bf9b2c
Rebuild display lists in full when we start/stop computing visibility for plugins. r=miko

Comment 4

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8986348 [details]
Bug 1469746 - Rebuild display lists in full when we start/stop computing visibility for plugins.

Approval Request Comment
[Feature/Bug causing the regression]: RDL
[User impact if declined]: Assertion crashes in Nightly/Dev Edition. Specific to opacity:0 around plugins, so nothing to really render wrong in release, probably unaffected.
[Is this code covered by automated tests?]: No (our coverage of windowed plugins isn't great)
[Has the fix been verified in Nightly?]: No.
[Needs manual test from QE? If yes, steps to reproduce]: Maybe? I've verified the fix locally in my builds. The github URL has a link that crashes ~50% of the time for me. 
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Just another RDL fallback, super low risk.
[String changes made/needed]: None.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8986348 [details]
Bug 1469746 - Rebuild display lists in full when we start/stop computing visibility for plugins.

Too late for the Fx61 release at this point (we're already building RCs), but I'm leaving it on the radar for possible dot release inclusion.

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/97d477bf9b2c

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8986348 [details]
Bug 1469746 - Rebuild display lists in full when we start/stop computing visibility for plugins.

We're going to be spinning another Fx61 RC build. Let's take this in it so users getting the graduated rollout of RDL benefit from the fix sooner.

Comment 8

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-release/rev/61ce1b424ccc