New issue
Advanced search Search tips

Issue 872364 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Cleanup: remove coordinator_handles_offset from composited_layer_mapping.cc

Project Member Reported by pdr@chromium.org, Aug 8

Issue description

With recent refactoring in this area, "bool coordinator_handles_offset" is always true:
https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc?l=1687

Both scrolling_coordinator and scrollable_area will always exist (due to https://bit.ly/root-layer-scrolling, though the details are not important).

We can refactor this code from the following:
------------------8<------------------
bool coordinator_handles_offset = false;
auto* scrolling_coordinator = owning_layer_.GetScrollingCoordinator();
auto* scrollable_area = owning_layer_.GetScrollableArea();
if (scrolling_coordinator && scrollable_area) {
  coordinator_handles_offset =
      scrolling_coordinator->ScrollableAreaScrollLayerDidChange(
          scrollable_area);
}
scrolling_contents_layer_->SetPosition(
    coordinator_handles_offset ? FloatPoint()
                               : FloatPoint(-ToFloatSize(scroll_position)));
------------------8<------------------

To:
------------------8<------------------
auto* scrolling_coordinator = owning_layer_.GetScrollingCoordinator();
scrolling_coordinator->ScrollableAreaScrollLayerDidChange(scrollable_area);
scrolling_contents_layer_->SetPosition(
    FloatPoint(-ToFloatSize(scroll_position)));
------------------8<------------------

 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/280f8b99783b1aeb5ebf24b5b811073d6d1b7c46

commit 280f8b99783b1aeb5ebf24b5b811073d6d1b7c46
Author: Mason Freed <masonfreed@google.com>
Date: Fri Aug 10 04:07:41 2018

Refactor composite layer mapping to remove redundant conditional.

Bug:  872364 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ib45390dab6120e74d51564760433972f17dae9dc
Reviewed-on: https://chromium-review.googlesource.com/1167882
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582046}
[modify] https://crrev.com/280f8b99783b1aeb5ebf24b5b811073d6d1b7c46/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.cc
[modify] https://crrev.com/280f8b99783b1aeb5ebf24b5b811073d6d1b7c46/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.h
[modify] https://crrev.com/280f8b99783b1aeb5ebf24b5b811073d6d1b7c46/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc

Cc: -masonfreed@google.com pdr@chromium.org
Owner: masonfreed@chromium.org
Status: Fixed (was: Assigned)
Woohoo!

Sign in to add a comment