Closed
Bug 1490370
Opened 6 years ago
Closed 6 years ago
Trying to add non-existent scrollbars disables retained display lists
Categories
(Core :: Web Painting, defect, P1)
Core
Web Painting
Tracking
()
VERIFIED
FIXED
mozilla64
People
(Reporter: mikokm, Assigned: mikokm)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
46 bytes,
text/x-phabricator-request
|
mstange
:
review+
pascalc
:
approval-mozilla-beta+
|
Details | Review |
2.99 KB,
text/html
|
Details |
This breaks retained display lists at least on Searchfox, Youtube, and Facebook.
Assignee | ||
Comment 1•6 years ago
|
||
Comment 2•6 years ago
|
||
Comment on attachment 9008097 [details] Bug 1490370 - Exit early when there is no scrollbar content to add Markus Stange [:mstange] has approved the revision.
Attachment #9008097 -
Flags: review+
Assignee | ||
Comment 3•6 years ago
|
||
Testcase
Pushed by mikokm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/1eee82c8caa8 Exit early when there is no scrollbar content to add r=mstange
Comment 5•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1eee82c8caa8
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox64:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Assignee | ||
Comment 6•6 years ago
|
||
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.
Attachment #9008097 -
Flags: approval-mozilla-beta?
Comment 7•6 years ago
|
||
Comment on attachment 9008097 [details] Bug 1490370 - Exit early when there is no scrollbar content to add Approved for 63 Beta 8, thanks.
Attachment #9008097 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•6 years ago
|
Flags: qe-verify+
Comment 8•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/9a52b350ee5c
status-firefox63:
--- → fixed
Comment 9•6 years ago
|
||
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?
Flags: needinfo?(mikokm)
Assignee | ||
Comment 10•6 years ago
|
||
(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.
Flags: needinfo?(mikokm)
Comment 11•6 years ago
|
||
Indeed, on macOS(10.13.6) there is a good difference between the values. Marking this verified for 64.0a1(2018-09-19).
Status: RESOLVED → VERIFIED
Comment 12•6 years ago
|
||
Verified on 63.0b8 as well.
Updated•6 years ago
|
Flags: qe-verify+
You need to log in
before you can comment on or make changes to this bug.
Description
•