1490370 - Trying to add non-existent scrollbars disables retained display lists

Status: VERIFIED FIXED

(Core :: Web Painting, defect, P1)

Description

[Miko Mynttinen]

This breaks retained display lists at least on Searchfox, Youtube, and Facebook.

Comment 1

[Miko Mynttinen]

Attachment: Bug 1490370 - Exit early when there is no scrollbar content to add

Comment 2

[Markus Stange [:mstange]]

Comment on attachment 9008097 [details]
Bug 1490370 - Exit early when there is no scrollbar content to add

Markus Stange [:mstange] has approved the revision.

Comment 3

[Miko Mynttinen]

Attachment: dl-test-modified.html

Testcase

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1eee82c8caa8
Exit early when there is no scrollbar content to add r=mstange

Comment 5

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/1eee82c8caa8

Comment 6

[Miko Mynttinen]

Comment on attachment 9008097 [details]
Bug 1490370 - Exit early when there is no scrollbar content to add

Approval Request Comment
[Feature/Bug causing the regression]: Overlay scrollbars disable retained display lists even when they are not visible.
[User impact if declined]: Retained display lists do not work on some sites, such as Youtube and Facebook, when system uses overlay scrollbars (Mac).
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Yes. I have verified the fix in Nightly.
[Needs manual test from QE? If yes, steps to reproduce]: Yes.
STR:
1) Open the attached testcase https://bug1490370.bmoattachments.org/attachment.cgi?id=9008104
2) Set pref layers.acceleration.draw-fps to true
3) Toggle pref layout.display-list.retain between true and false and compare the number "[Content] DL". This is display list build time, and it should be noticeably lower when layout.display-list.retain = true.

[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: Not risky.
[Why is the change risky/not risky?]: The change is small and easy to reason about - we do not need to disable display list retaining, if we did not build scrollbar content.
[String changes made/needed]: None.

Comment 7

[Pascal Chevrel:pascalc]

Comment on attachment 9008097 [details]
Bug 1490370 - Exit early when there is no scrollbar content to add

Approved for 63 Beta 8, thanks.

Comment 8

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/releases/mozilla-beta/rev/9a52b350ee5c

Comment 9

[Cristian Fogel [:cfogel]]

Have tried to reproduce and verify (on Win10) using the steps from comment 6. 
However I've noticed that the difference in values was about the same even before the fix:

  true || false       version:
4.2-4.4||9-10         63.0a1 (2018-08-01)    
3.1-3.2||7.6-8.6      beta7                      
3.6-4.2||8.2-8.6      nightly (version with the fix)                

Is it something of concern?

Comment 10

[Miko Mynttinen]

(In reply to Cristi Fogel [:cfogel] from comment #9)
> Have tried to reproduce and verify (on Win10) using the steps from comment
> 6. 
> However I've noticed that the difference in values was about the same even
> before the fix:
> 
>   true || false       version:
> 4.2-4.4||9-10         63.0a1 (2018-08-01)    
> 3.1-3.2||7.6-8.6      beta7                      
> 3.6-4.2||8.2-8.6      nightly (version with the fix)                
> 
> Is it something of concern?

This was a macOS specific fix, so this is expected.

Comment 11

[Cristian Fogel [:cfogel]]

Indeed, on macOS(10.13.6) there is a good difference between the values.
Marking this verified for 64.0a1(2018-09-19).

Comment 12

[Cristian Fogel [:cfogel]]

Verified on 63.0b8 as well.