SubtreeReasonsForCompositing should not promote position: fixed elements unless they move with scroll |
||||||||||
Issue descriptionSee https://codereview.chromium.org/2754983002/ for the discussion on position: fixed elements. Currently SubtreeReasonsForCompositing in CompositingRequirementsUpdater.cpp will promote *any* fixed position elements. This is overzealous - unless the fixed element can scroll, it doesn't need a layer.
,
May 5 2017
,
Jul 17 2017
This behavior was reverted in https://codereview.chromium.org/2977273002/ due to issue 742213. It is not the root cause of that bug, but reverting this was the quickest way to fix the stable-blocking bug. I'm re-opening this issue, and will mark it blocked on the root cause fix once I open a bug for that.
,
Jul 18 2017
,
Apr 25 2018
I am not currently working on this, dropping to Available
,
Apr 25 2018
,
Jun 22 2018
It appears that the downstream issue with WebView and FullScreen has been fixed (we think; no longer able to reproduce), so I'm going to try re-landing this.
,
Jun 28 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9ca72e1af3da3d582edd7e35b12881acd32eeff5 commit 9ca72e1af3da3d582edd7e35b12881acd32eeff5 Author: Stephen McGruer <smcgruer@chromium.org> Date: Thu Jun 28 15:52:58 2018 Don't promote position: fixed elements with composited descendants if they don't scroll This is a re-land of an old CL, which was reverted in https://codereview.chromium.org/2977273002 due to a WebView issue. We believe the underlying issue may have been resolved, so should try re-landing this. Bug: 715699 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I0da6ab6f55d0fb4ba0760378f182217e8c4546c1 Reviewed-on: https://chromium-review.googlesource.com/1112387 Reviewed-by: Robert Flack <flackr@chromium.org> Commit-Queue: Robert Flack <flackr@chromium.org> Cr-Commit-Position: refs/heads/master@{#571143} [modify] https://crrev.com/9ca72e1af3da3d582edd7e35b12881acd32eeff5/third_party/blink/renderer/core/paint/compositing/compositing_requirements_updater.cc
,
Jul 10
,
Aug 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/365973dba9c887bb8cfd8b0d1edba41166fb094b commit 365973dba9c887bb8cfd8b0d1edba41166fb094b Author: Stephen McGruer <smcgruer@chromium.org> Date: Fri Aug 17 15:53:08 2018 Revert "Don't promote position: fixed elements with composited descendants if they don't scroll" This reverts commit 9ca72e1af3da3d582edd7e35b12881acd32eeff5. Reason for revert: Suspected cause of crbug.com/863361 - it seems that scrolling the viewport is still possible. Bug: 715699, 863361 Original change's description: > Don't promote position: fixed elements with composited descendants if they don't scroll > > This is a re-land of an old CL, which was reverted in https://codereview.chromium.org/2977273002 > due to a WebView issue. We believe the underlying issue may have been resolved, so should try > re-landing this. > > Bug: 715699 > Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel > Change-Id: I0da6ab6f55d0fb4ba0760378f182217e8c4546c1 > Reviewed-on: https://chromium-review.googlesource.com/1112387 > Reviewed-by: Robert Flack <flackr@chromium.org> > Commit-Queue: Robert Flack <flackr@chromium.org> > Cr-Commit-Position: refs/heads/master@{#571143} TBR=flackr@chromium.org,smcgruer@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 715699 Change-Id: I6a9e969a4118b75819dcd297d18d9a2ae1c9abca Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Reviewed-on: https://chromium-review.googlesource.com/1179921 Reviewed-by: Stephen McGruer <smcgruer@chromium.org> Commit-Queue: Stephen McGruer <smcgruer@chromium.org> Cr-Commit-Position: refs/heads/master@{#584087} [modify] https://crrev.com/365973dba9c887bb8cfd8b0d1edba41166fb094b/third_party/blink/renderer/core/paint/compositing/compositing_requirements_updater.cc
,
Aug 20
,
Aug 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5525c8022f28870d0165ef34582a3ea12873622d commit 5525c8022f28870d0165ef34582a3ea12873622d Author: Stephen McGruer <smcgruer@chromium.org> Date: Mon Aug 20 18:25:21 2018 Revert "Don't promote position: fixed elements with composited descendants if they don't scroll" This reverts commit 9ca72e1af3da3d582edd7e35b12881acd32eeff5. Reason for revert: Suspected cause of crbug.com/863361 - it seems that scrolling the viewport is still possible. Bug: 715699, 863361 Original change's description: > Don't promote position: fixed elements with composited descendants if they don't scroll > > This is a re-land of an old CL, which was reverted in https://codereview.chromium.org/2977273002 > due to a WebView issue. We believe the underlying issue may have been resolved, so should try > re-landing this. > > Bug: 715699 > Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel > Change-Id: I0da6ab6f55d0fb4ba0760378f182217e8c4546c1 > Reviewed-on: https://chromium-review.googlesource.com/1112387 > Reviewed-by: Robert Flack <flackr@chromium.org> > Commit-Queue: Robert Flack <flackr@chromium.org> > Cr-Commit-Position: refs/heads/master@{#571143} TBR=flackr@chromium.org,smcgruer@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 715699 Change-Id: I6a9e969a4118b75819dcd297d18d9a2ae1c9abca Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Reviewed-on: https://chromium-review.googlesource.com/1179921 Reviewed-by: Stephen McGruer <smcgruer@chromium.org> Commit-Queue: Stephen McGruer <smcgruer@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#584087}(cherry picked from commit 365973dba9c887bb8cfd8b0d1edba41166fb094b) Reviewed-on: https://chromium-review.googlesource.com/1181801 Reviewed-by: Alex Mineer <amineer@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#719} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/5525c8022f28870d0165ef34582a3ea12873622d/third_party/blink/renderer/core/paint/compositing/compositing_requirements_updater.cc
,
Aug 21
Sadly, looks like the underlying code wasn't fixed yet, and so this had to be reverted again to avoid breaking WebView :(. See issue 863361 for the latest reproduction (on cnn.com). I'm opening this up to available as I won't be working on it, but there is real benefit to this change it seems. The evidence is these three separate memory perf regression bugs that I had opened against me when the above revert went in! Issue 876037, 876101, 876036.
,
Aug 22
Make that four memory perf regressions from the revert: issue 876022
,
Sep 6
Make that five memory perf regressions from the revert: issue 880899 |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by bugdroid1@chromium.org
, May 5 2017