1413546 - Enable retained display lists for parent process

Status: RESOLVED FIXED

(Core :: Web Painting, enhancement, P2)

Description

[Miko Mynttinen]

At the moment we are only using display list retaining in content processes.

Enabling retained display lists for parent process crashes when using popups or hovering over some UI elements.

This bug tracks fixing those problems and enabling retained display lists for parent process.

Comment 1

[Jet Villegas (inactive)]

Miko: can you land a patch to turn this into a pref? Thx!
https://searchfox.org/mozilla-central/source/layout/base/nsLayoutUtils.cpp#3671

Comment 2

[Matt Woodrow (:mattwoodrow)]

Attachment: Add a pref to enable retained-dl for chrome

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e505ce45eec155292008a9d77310ef2a871e3c69

Comment 3

[Jet Villegas (inactive)]

Comment on attachment 8937554 [details] [diff] [review]
Add a pref to enable retained-dl for chrome

layout.display-list.retain.chrome

LGTM. Thx! r+

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/26ee6030db34
Add pref to allow retained display lists within the parent process. r=jet

Comment 5

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/26ee6030db34

Comment 6

[mdew]

Could we temporarily revert this patch, so it can be improved/less buggy?

Comment 7

[Matt Woodrow (:mattwoodrow)]

Alice, is there a specific reason you marked this as a blocker instead of bug 1344971?

Comment 8

[Alice0775 White]

(In reply to Matt Woodrow (:mattwoodrow) from comment #7)
> Alice, is there a specific reason you marked this as a blocker instead of
> bug 1344971?

no.

Comment 9

[Miko Mynttinen]

Attachment: Bug 1413546 - Fix e10s logic in nsLayoutUtils::AreRetainedDisplayListsEnabled() and set layout.display-list.retain.chrome to true

Comment 10

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8998627 [details]
Bug 1413546 - Fix e10s logic in nsLayoutUtils::AreRetainedDisplayListsEnabled() and set layout.display-list.retain.chrome to true

Matt Woodrow (:mattwoodrow) has approved the revision.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b4b3be038ab8
Fix e10s logic in nsLayoutUtils::AreRetainedDisplayListsEnabled() and set layout.display-list.retain.chrome to true r=mattwoodrow

Comment 12

[Andreea Pavel [:apavel]]

Backed out for failing win  testing\firefox-ui\tests\functional\sessionstore\test_restore_windows_after_windows_shutdown.py 

Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=b4b3be038ab8e3f6c8aa4c3bcfdba2e581d65a13

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=192935582&repo=autoland&lineNumber=64571

Backout: https://hg.mozilla.org/integration/autoland/rev/3202d623ead82c71bb1790ecfc5c3d99493d2e1e

Comment 13

[Miko Mynttinen]

It seems that test_restore_windows_after_windows_shutdown.py is failing because we have a dangling pointer to RenderFrameParent in nsDisplayRemote. The cause for this is either a missing invalidation for nsDisplayRemote, or a bug in the test that uses a special shutdown mechanic.

Comment 14

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

It's possible that there is something up with the test, but I'd expect the same issue to come up with a ordinary OS shutdown, this is afaik the only test that exercises that shutdown path.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/716d49302a28
Fix e10s logic in nsLayoutUtils::AreRetainedDisplayListsEnabled() and set layout.display-list.retain.chrome to true r=mattwoodrow

Comment 16

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

https://hg.mozilla.org/mozilla-central/rev/716d49302a28

Comment 17

[Alex Thayer [:alexical] (she/her)]

Should we reopen this given that we have since disabled it?

Comment 18

[Miko Mynttinen]

This was disabled in bug 1496491, because a Windows specific bug with dragging regions prevented users from closing tabs.

Comment 19

[Miko Mynttinen]

Attachment: Bug 1413546 - Enable retained display lists for parent process r=mattwoodrow

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/87409f291fa6
Enable retained display lists for parent process r=mattwoodrow

Comment 21

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/87409f291fa6

Comment 22

[Alexandru Ionescu (needinfo me) [:alexandrui]]

We noticed this performance improvements:
== Change summary for alert #21276 (as of Tue, 04 Jun 2019 07:38:06 GMT) ==

Improvements:

5% tart windows10-64-shippable opt e10s stylo 2.68 -> 2.53
4% tart windows7-32-shippable opt e10s stylo 2.65 -> 2.54

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=21276

Comment 23

[Alexandru Ionescu (needinfo me) [:alexandrui]]

Hi, the improvements below came with your patch:
== Change summary for alert #21276 (as of Tue, 04 Jun 2019 07:38:06 GMT) ==

Improvements:

5% tart windows10-64-shippable opt e10s stylo 2.68 -> 2.53
4% tart windows7-32-shippable opt e10s stylo 2.65 -> 2.54
2% tart linux64-shippable opt e10s stylo 2.04 -> 2.00

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=21276