Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2822603002:
Remove UpdateScrollTree (Closed)
DescriptionRemove draw_property_utils's UpdateScrollTree
This function was added in [1] but is not needed. Removing this function
lets us remove one more user of scroll_node's owning_layer_id.
[1] https://chromium.googlesource.com/chromium/src/+/1c0802c3c4c03b43efea23b82d7e624f1d8c7b57
BUG=693740
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Review-Url: https://codereview.chromium.org/2822603002
Cr-Commit-Position: refs/heads/master@{#465342}
Committed: https://chromium.googlesource.com/chromium/src/+/a91f5b16d907a71eb79896a54fc8c368d11dc061
Patch Set 1 #
Messages
Total messages: 19 (11 generated)
Description was changed from ========== Remove UpdateScrollTree BUG=693740 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ========== to ========== Remove UpdateScrollTree BUG=693740 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ==========
The CQ bit was checked by pdr@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Remove UpdateScrollTree BUG=693740 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ========== to ========== Remove draw_property_utils's UpdateScrollTree This function was added in [1] but is not needed. Removing this function lets us remove one more user of scroll_node's owning_layer_id. [1] https://chromium.googlesource.com/chromium/src/+/1c0802c3c4c03b43efea23b82d7e... BUG=693740 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ==========
pdr@chromium.org changed reviewers: + weiliangc@chromium.org
pdr@chromium.org changed reviewers: + enne@chromium.org
lgtm
The CQ bit was checked by pdr@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1492541135492740, "parent_rev":
"83c086319c32d59aa423af87326140530a4e2df4", "commit_rev":
"a91f5b16d907a71eb79896a54fc8c368d11dc061"}
Message was sent while issue was closed.
Description was changed from ========== Remove draw_property_utils's UpdateScrollTree This function was added in [1] but is not needed. Removing this function lets us remove one more user of scroll_node's owning_layer_id. [1] https://chromium.googlesource.com/chromium/src/+/1c0802c3c4c03b43efea23b82d7e... BUG=693740 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ========== to ========== Remove draw_property_utils's UpdateScrollTree This function was added in [1] but is not needed. Removing this function lets us remove one more user of scroll_node's owning_layer_id. [1] https://chromium.googlesource.com/chromium/src/+/1c0802c3c4c03b43efea23b82d7e... BUG=693740 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2822603002 Cr-Commit-Position: refs/heads/master@{#465342} Committed: https://chromium.googlesource.com/chromium/src/+/a91f5b16d907a71eb79896a54fc8... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/a91f5b16d907a71eb79896a54fc8...
Message was sent while issue was closed.
Oops my bad, I didn't add test/comment for what this code is for and I forgot what it is for. So this is for when we have a scroll clip layer that doesn't mask to bounds but has bounds change. This implies changes in scroll clip layer bounds. For having a scroll clip layer w/o mask to bounds seems to be possibly a setting for WebView. (here https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/Vis...). Or maybe here when the container_layer is not clipping? (https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/page/scro...). Maybe instead of set is_scoll_clip_layer_ we could set ElementId of what this scroll clip layer is for? That way we can directly find the scroll node and update scroll clip layer bounds there.
Message was sent while issue was closed.
On 2017/04/18 at 21:25:24, weiliangc wrote: > Oops my bad, I didn't add test/comment for what this code is for and I forgot what it is for. > > So this is for when we have a scroll clip layer that doesn't mask to bounds but has bounds change. This implies changes in scroll clip layer bounds. > > For having a scroll clip layer w/o mask to bounds seems to be possibly a setting for WebView. (here https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/Vis...). > > Or maybe here when the container_layer is not clipping? (https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/page/scro...). > > Maybe instead of set is_scoll_clip_layer_ we could set ElementId of what this scroll clip layer is for? That way we can directly find the scroll node and update scroll clip layer bounds there. Would you be up for writing a small testcase for me? I can then fix the code.
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in https://codereview.chromium.org/2830473002/ by pdr@chromium.org. The reason for reverting is: Patch failed to handle changes to scroll clip layer bounds.. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
