New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 693740 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 723263

Blocking:
issue 471333
issue 667946
issue 712298



Sign in to add a comment

Remove owning_layer_id from cc::ScrollNode

Project Member Reported by pdr@chromium.org, Feb 17 2017

Issue description

We should use element ids as the stable mapping between scroll nodes and layers instead of using layer ids.
 

Comment 1 by pdr@chromium.org, Feb 20 2017

Cc: ajuma@chromium.org khushals...@chromium.org
The goal of this refactoring is to remove ScrollNode's owning_layer_id which maps from ScrollNode to Layer. We will still have a mapping from Layer to ScrollNode, just not the other way around. Here are the big users of owning_layer_id:

1) Scroll offsets (e.g., ScrollTree::ScrollBy):
* ScrollTree::layer_id_to_scroll_offset_map_
* ScrollTree::layer_id_to_synced_scroll_offset_map_
I think we should move ScrollOffset and SyncedScrollOffset to ScrollNode. Existing Layer->Offset callsites would be updated to go through ScrollNode (i.e., Layer->ScrollNode->Offset).

2) Tracking the current scrolling layer. We can refactor all of these cases to just track the current scroll node.

3) Testing if a scroll is for one of the viewport layers. These cases can be refactored to check the viewport layer's scroll node instead of the scroll node's layer.

4) There are various callsites in layer_tree_host_impl.cc such as FindScrollLayerForDeviceViewportPoint. Some of these cases look gnarly but I think they can all be switched to using the scroll node itself instead of the layer. This will require that we move non-fast-scrolling-regions to the scroll tree before doing this refactor.
I think you will have to re-work the way scroll sync happens if SyncedScrollOffset is moved to ScrollNode. Right now we simply override copying the maps when copying PropertyTrees from main to pending and pending to active (https://cs.chromium.org/chromium/src/cc/trees/property_tree.cc?l=1121) and do a pass after this step.

One way I could think of doing this was to do a copy when going from main to pending. And then as a second step set SyncedProperty using the ScrollNodes from the active PropertyTrees and calling PushFromMainThread there. But this won't work if we are committing to the active tree...
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 22 2017

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

commit 5aa2bdd883754f0d62a743c528e2c69107c3550d
Author: pdr <pdr@chromium.org>
Date: Wed Feb 22 18:23:23 2017

Remove ScrollNode's layer_id_to_scrollbars_enabled_map

This patch removes cc::ScrollNode::layer_id_to_scrollbars_enabled_map_
which was added in [1] but is not used. This is part of a larger
refactoring to remove Layer information from ScrollNode.

[1] https://codereview.chromium.org/2453553003

BUG= 693740 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2702053002
Cr-Commit-Position: refs/heads/master@{#452137}

[modify] https://crrev.com/5aa2bdd883754f0d62a743c528e2c69107c3550d/cc/trees/property_tree.cc
[modify] https://crrev.com/5aa2bdd883754f0d62a743c528e2c69107c3550d/cc/trees/property_tree.h

Project Member

Comment 4 by bugdroid1@chromium.org, Feb 22 2017

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

commit 014861b417eb3532456a982f1d81a384358fc859
Author: pdr <pdr@chromium.org>
Date: Wed Feb 22 18:44:39 2017

Refactor scroll chaining to use ScrollNodes without going through LayerImpl

Before this patch, ScrollState::DistributeToScrollChainDescendant kept
a list of const ScrollNode pointers and used the node's owning_layer_id to
lookup a non-const version of the ScrollNode through LayerImpl's
DistributeScroll. This indirection is not needed and we can simply use a
non-const ScrollNode list, bypassing LayerImpl::DistributeScroll.

It can be verified that this is equivalent by adding the following DHCECK
to ScrollState::DistributeToScrollChainDescendant:
DCHECK_EQ(scroll_tree.Node(layer_tree_impl_->LayerById(next->owning_layer_id)->scroll_tree_index()), next);

This patch lets us remove one use of ScrollNode's owning_layer_id.

BUG= 693740 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2708493002
Cr-Commit-Position: refs/heads/master@{#452143}

[modify] https://crrev.com/014861b417eb3532456a982f1d81a384358fc859/cc/input/scroll_state.cc
[modify] https://crrev.com/014861b417eb3532456a982f1d81a384358fc859/cc/input/scroll_state.h
[modify] https://crrev.com/014861b417eb3532456a982f1d81a384358fc859/cc/layers/layer_impl.cc
[modify] https://crrev.com/014861b417eb3532456a982f1d81a384358fc859/cc/layers/layer_impl.h
[modify] https://crrev.com/014861b417eb3532456a982f1d81a384358fc859/cc/trees/layer_tree_host_impl.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Feb 22 2017

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

commit dd7a9eede66b8ab6d1b76c24d01010d7ad06cafa
Author: pdr <pdr@chromium.org>
Date: Wed Feb 22 19:22:48 2017

Refactor viewport scrolling decisions to use ScrollNodes over layer ids

Viewport scrolling has special-casing that previously used ScrollNode's
owning_layer_id but can be refactored to use ScrollNodes directly.
There should be no change in behavior and this gets us a little closer
to removing owning_layer_id.

BUG= 693740 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2707783003
Cr-Commit-Position: refs/heads/master@{#452161}

[modify] https://crrev.com/dd7a9eede66b8ab6d1b76c24d01010d7ad06cafa/cc/trees/layer_tree_host_impl.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Feb 22 2017

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

commit 5b0518aededd5260a9fde6669b690ef705ef5234
Author: pdr <pdr@chromium.org>
Date: Wed Feb 22 23:30:08 2017

Reconstitute const-ref parameter to set_scroll_chain_and_layer_tree

This was mistakenly removed in https://codereview.chromium.org/2708493002

BUG= 693740 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2713713002
Cr-Commit-Position: refs/heads/master@{#452270}

[modify] https://crrev.com/5b0518aededd5260a9fde6669b690ef705ef5234/cc/input/scroll_state.h
[modify] https://crrev.com/5b0518aededd5260a9fde6669b690ef705ef5234/cc/trees/layer_tree_host_impl.cc

Comment 7 by pdr@chromium.org, Feb 23 2017

Thanks Khushal, that does seem like an issue in the proposal. Ali and I chatted about this today and he suggested using an auxiliary map. I poked around and it looks like we could add scroll_id_to_scroll_offset_map_ (or similar) to PropertyTrees, and then replace getters/setters of layer_id_to_scroll_offset_map_[layer->id()] with scroll_id_to_scroll_offset_map_[layer->scroll_tree_index()]. Do you think this would work?
The impl thread has state associated with scrolling of a particular element, so you need something more persistent to map this state to these elements than scroll_tree_index. If PropertyTrees are re-built on the main thread, then the same layer on the main and impl thread will have different scroll_tree_indexes.

How about an element_id_to_scroll_offset_map_? That makes this mapping much more obvious too.
Project Member

Comment 9 by bugdroid1@chromium.org, Feb 23 2017

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

commit 87f502185cad0c3cbdd7019790a7cff3d37bbf5b
Author: pdr <pdr@chromium.org>
Date: Thu Feb 23 19:37:01 2017

Refactor IsScrolledBy and IsClosestScrollAncestor to use ScrollNode ids

This patch switches IsScrolledBy and IsClosestScrollAncestor to use
ScrollNode indexes instead of ScrollNode's owning_layer_id. There should
be no change in behavior and this gets us a little closer to removing
owning_layer_id.

BUG= 693740 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2702143002
Cr-Commit-Position: refs/heads/master@{#452589}

[modify] https://crrev.com/87f502185cad0c3cbdd7019790a7cff3d37bbf5b/cc/trees/layer_tree_host_impl.cc

Comment 10 by pdr@chromium.org, Feb 23 2017

Cc: wkorman@chromium.org
Thinking about this more, I'm starting to second-guess the approach I took in https://codereview.chromium.org/2702143002.

In https://codereview.chromium.org/2702143002, I depend on there being "owning layers" for a given scroll node, and this concept is used in IsScrolledBy and IsClosestScrollLayer. What is this "owning layer" concept really for? I think we can change the callers of these functions to work with scroll nodes instead of layers instead, and then the need for "owning layer" goes away.
Project Member

Comment 11 by bugdroid1@chromium.org, Feb 24 2017

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

commit abe15527a4e00dfaefe168f02e387c8c66773285
Author: pdr <pdr@chromium.org>
Date: Fri Feb 24 19:15:56 2017

Rename cc::ScrollNode's inner and outer viewport members

ScrollNode has members for is_{inner, outer}_viewport_scroll_layer but
these names do not make sense without ScrollNode's owning_layer_id
concept. This patch renames these members to scrolls_inner_viewport
and scrolls_outer_viewport.

In addition, this patch cleans up viewport layer detection added in
https://codereview.chromium.org/2707783003 to use these new members.

BUG= 693740 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2714713003
Cr-Commit-Position: refs/heads/master@{#452897}

[modify] https://crrev.com/abe15527a4e00dfaefe168f02e387c8c66773285/cc/trees/layer_tree_host_common_unittest.cc
[modify] https://crrev.com/abe15527a4e00dfaefe168f02e387c8c66773285/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/abe15527a4e00dfaefe168f02e387c8c66773285/cc/trees/property_tree.cc
[modify] https://crrev.com/abe15527a4e00dfaefe168f02e387c8c66773285/cc/trees/property_tree_builder.cc
[modify] https://crrev.com/abe15527a4e00dfaefe168f02e387c8c66773285/cc/trees/scroll_node.cc
[modify] https://crrev.com/abe15527a4e00dfaefe168f02e387c8c66773285/cc/trees/scroll_node.h

Project Member

Comment 12 by bugdroid1@chromium.org, Feb 28 2017

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

commit 47fcc62bde25cb7275e56c3b256c0d0dc75aefaa
Author: pdr <pdr@chromium.org>
Date: Tue Feb 28 02:22:22 2017

Refactor unreliable hit test code to avoid owning_layers

In [1] we switched the unreliable hit test code from using ScrollNode's
owning_layer to using the ScrollTree's layer_id_to_scroll_node_index.
This patch improves on [1] to not use layer_id_to_scroll_node_index
either, which turned out to be owning_layer in disguise.

The existing IsClosestScrollAncestor code has been refactored into a
helper (IsInitialScrollHitTestReliable) that makes the main thread
fallback cases more obvious (see: [2,3]). A test has been added for
the failure case noticed by Ali Juma in [1] where a drawn scrollbar
is the child of a scrolling layer but itself does not scroll.

[1] https://codereview.chromium.org/2702143002
[2] https://codereview.chromium.org/238803005
[3] https://codereview.chromium.org/1878323002

BUG= 693740 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2718833002
Cr-Commit-Position: refs/heads/master@{#453462}

[modify] https://crrev.com/47fcc62bde25cb7275e56c3b256c0d0dc75aefaa/cc/layers/layer_impl.h
[modify] https://crrev.com/47fcc62bde25cb7275e56c3b256c0d0dc75aefaa/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/47fcc62bde25cb7275e56c3b256c0d0dc75aefaa/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/47fcc62bde25cb7275e56c3b256c0d0dc75aefaa/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/47fcc62bde25cb7275e56c3b256c0d0dc75aefaa/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/47fcc62bde25cb7275e56c3b256c0d0dc75aefaa/cc/trees/layer_tree_impl.h

Project Member

Comment 13 by bugdroid1@chromium.org, Feb 28 2017

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

commit b615d9eab4718e5a8f14ac44e6f2f5639c829a9f
Author: pdr <pdr@chromium.org>
Date: Tue Feb 28 06:01:27 2017

Revert of Refactor unreliable hit test code to avoid owning_layers (patchset #3 id:40001 of https://codereview.chromium.org/2718833002/ )

Reason for revert:
May have caused RenderWidgetHostViewChildFrameTest.SwapCompositorFrame to fail on windows.

Original issue's description:
> Refactor unreliable hit test code to avoid owning_layers
>
> In [1] we switched the unreliable hit test code from using ScrollNode's
> owning_layer to using the ScrollTree's layer_id_to_scroll_node_index.
> This patch improves on [1] to not use layer_id_to_scroll_node_index
> either, which turned out to be owning_layer in disguise.
>
> The existing IsClosestScrollAncestor code has been refactored into a
> helper (IsInitialScrollHitTestReliable) that makes the main thread
> fallback cases more obvious (see: [2,3]). A test has been added for
> the failure case noticed by Ali Juma in [1] where a drawn scrollbar
> is the child of a scrolling layer but itself does not scroll.
>
> [1] https://codereview.chromium.org/2702143002
> [2] https://codereview.chromium.org/238803005
> [3] https://codereview.chromium.org/1878323002
>
> BUG= 693740 
> CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
>
> Review-Url: https://codereview.chromium.org/2718833002
> Cr-Commit-Position: refs/heads/master@{#453462}
> Committed: https://chromium.googlesource.com/chromium/src/+/47fcc62bde25cb7275e56c3b256c0d0dc75aefaa

TBR=ajuma@chromium.org,bokan@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 693740 

Review-Url: https://codereview.chromium.org/2723553003
Cr-Commit-Position: refs/heads/master@{#453516}

[modify] https://crrev.com/b615d9eab4718e5a8f14ac44e6f2f5639c829a9f/cc/layers/layer_impl.h
[modify] https://crrev.com/b615d9eab4718e5a8f14ac44e6f2f5639c829a9f/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/b615d9eab4718e5a8f14ac44e6f2f5639c829a9f/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/b615d9eab4718e5a8f14ac44e6f2f5639c829a9f/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/b615d9eab4718e5a8f14ac44e6f2f5639c829a9f/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/b615d9eab4718e5a8f14ac44e6f2f5639c829a9f/cc/trees/layer_tree_impl.h

Labels: BugSource-Team PaintTeamTriaged-20170217
Project Member

Comment 15 by bugdroid1@chromium.org, Feb 28 2017

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

commit dbe75656dc50037306c8a3b6968a5592f4b17106
Author: pdr <pdr@chromium.org>
Date: Tue Feb 28 20:23:05 2017

Reland: Refactor unreliable hit test code to avoid owning_layers

In [1] we switched the unreliable hit test code from using ScrollNode's
owning_layer to using the ScrollTree's layer_id_to_scroll_node_index.
This patch improves on [1] to not use layer_id_to_scroll_node_index
either, which turned out to be owning_layer in disguise.

The existing IsClosestScrollAncestor code has been refactored into a
helper (IsInitialScrollHitTestReliable) that makes the main thread
fallback cases more obvious (see: [2,3]). A test has been added for
the failure case noticed by Ali Juma in [1] where a drawn scrollbar
is the child of a scrolling layer but itself does not scroll.

[1] https://codereview.chromium.org/2702143002
[2] https://codereview.chromium.org/238803005
[3] https://codereview.chromium.org/1878323002

This is a reland of https://codereview.chromium.org/2718833002 which
was rolled out but turned out to not be the actual cause of
 https://crbug.com/696919 .

TBR=ajuma@chromium.org
BUG= 693740 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2720713004
Cr-Commit-Position: refs/heads/master@{#453686}

[modify] https://crrev.com/dbe75656dc50037306c8a3b6968a5592f4b17106/cc/layers/layer_impl.h
[modify] https://crrev.com/dbe75656dc50037306c8a3b6968a5592f4b17106/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/dbe75656dc50037306c8a3b6968a5592f4b17106/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/dbe75656dc50037306c8a3b6968a5592f4b17106/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/dbe75656dc50037306c8a3b6968a5592f4b17106/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/dbe75656dc50037306c8a3b6968a5592f4b17106/cc/trees/layer_tree_impl.h

Project Member

Comment 17 by bugdroid1@chromium.org, Mar 1 2017

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

commit b2d4d4ba555e4658e5b2ea2ba4cbd9b95ac84442
Author: pdr <pdr@chromium.org>
Date: Wed Mar 01 19:56:45 2017

Track the currently scrolling ScrollNode instead of the scrolling layer

This patch removes CurrentlyScrollingLayer functions from LayerTreeImpl
and LayerTreeHostImpl, replacing them with calls to CurrentlyScrollingNode.
This removes two more uses of ScrollNode's owning_layer_id and sets us
up to remove the ScrollbarAnimationController dependency on layer ids
as well (left out of this patch to keep it simple).

BUG= 693740 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2720183003
Cr-Commit-Position: refs/heads/master@{#454006}

[modify] https://crrev.com/b2d4d4ba555e4658e5b2ea2ba4cbd9b95ac84442/cc/trees/layer_tree_host_common.cc
[modify] https://crrev.com/b2d4d4ba555e4658e5b2ea2ba4cbd9b95ac84442/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/b2d4d4ba555e4658e5b2ea2ba4cbd9b95ac84442/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/b2d4d4ba555e4658e5b2ea2ba4cbd9b95ac84442/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/b2d4d4ba555e4658e5b2ea2ba4cbd9b95ac84442/cc/trees/layer_tree_host_unittest_scroll.cc
[modify] https://crrev.com/b2d4d4ba555e4658e5b2ea2ba4cbd9b95ac84442/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/b2d4d4ba555e4658e5b2ea2ba4cbd9b95ac84442/cc/trees/layer_tree_impl.h
[modify] https://crrev.com/b2d4d4ba555e4658e5b2ea2ba4cbd9b95ac84442/cc/trees/tree_synchronizer_unittest.cc

Project Member

Comment 18 by bugdroid1@chromium.org, Mar 2 2017

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

commit c4e18068559bd3637c76ac46ead252b1d343098e
Author: pdr <pdr@chromium.org>
Date: Thu Mar 02 19:49:29 2017

Use element ids as a stable identifier tracking scroll nodes across updates

Before the pending tree is swapped to the active tree we need to record
the currently scrolling node. Once the pending tree has been pushed to
the active tree, the currently scrolling node then needs to be set.
Because the topology of the scroll tree can change, this is not as
simple as tracking the scroll node index.

Before this patch, layers were used as the stable identifier across
scroll tree updates. This patch switches to using element ids which
lets us remove another user of ScrollNode's owning_layer_id.

BUG= 693740 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2723023004
Cr-Commit-Position: refs/heads/master@{#454349}

[modify] https://crrev.com/c4e18068559bd3637c76ac46ead252b1d343098e/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/c4e18068559bd3637c76ac46ead252b1d343098e/cc/trees/tree_synchronizer_unittest.cc

Project Member

Comment 19 by bugdroid1@chromium.org, Mar 3 2017

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

commit c793ec77b26b49928278bb852a7691d36c8f7544
Author: pdr <pdr@chromium.org>
Date: Fri Mar 03 23:45:31 2017

Make LayerTreeImpl::SetCurrentlyScrollingNode use element ids over layer ids

This patch updates LayerTreeImpl::SetCurrentlyScrollingNode to use
element ids instead of layer ids. A TODO has been added to remove
the use of LayerIdByElementId for the scrollbar animation controller
which is a very large patch.

Tests have been updated to have the scroll element ids set.

BUG= 693740 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2729253003
Cr-Commit-Position: refs/heads/master@{#454713}

[modify] https://crrev.com/c793ec77b26b49928278bb852a7691d36c8f7544/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/c793ec77b26b49928278bb852a7691d36c8f7544/cc/trees/layer_tree_host_unittest_scroll.cc
[modify] https://crrev.com/c793ec77b26b49928278bb852a7691d36c8f7544/cc/trees/layer_tree_impl.cc

Project Member

Comment 20 by bugdroid1@chromium.org, Mar 6 2017

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

commit 84d15f489b981ad9f33301abf3ee78f989aa20bf
Author: pdr <pdr@chromium.org>
Date: Mon Mar 06 18:28:16 2017

Do not ignore SolidColorScrollbarLayer's scroll_layer_id ctor argument

SolidColorScrollbarLayer's constructor takes a scroll_layer_id argument
similar to the other ScrollbarLayerInterface subclasses, but this
argument was ignored. This patch fixes this bug which lets us remove
some redundant code in tests.

BUG= 693740 

Review-Url: https://codereview.chromium.org/2734583002
Cr-Commit-Position: refs/heads/master@{#454894}

[modify] https://crrev.com/84d15f489b981ad9f33301abf3ee78f989aa20bf/cc/layers/scrollbar_layer_unittest.cc
[modify] https://crrev.com/84d15f489b981ad9f33301abf3ee78f989aa20bf/cc/layers/solid_color_scrollbar_layer.cc
[modify] https://crrev.com/84d15f489b981ad9f33301abf3ee78f989aa20bf/cc/trees/layer_tree_host_unittest_damage.cc

Project Member

Comment 21 by bugdroid1@chromium.org, Mar 7 2017

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

commit a20ad732333dc9db2b545886acd16a11abdc5842
Author: pdr <pdr@chromium.org>
Date: Tue Mar 07 17:11:15 2017

Remove indirection: setup scrollbar scroll layers in the scrollbar constructor

Scrollbar layers keep track of the layer id (soon: element id) that they
actually scroll. Before this patch, scrollbar layers were constructed
with no scroll layer id, then later updated to set the scroll layer id.
This patch removes this indirection and directly sets the scroll layer
in the constructor.

Removed functions:
  VisualViewport::setScrollLayerOnScrollbars
  WebScrollbarLayer::setScrollLayer
  WebScrollbarLayerImpl::setScrollLayer
  PaintedOverlayScrollbarLayer::SetScrollLayer
  PaintedScrollbarLayer::SetScrollLayer
  SolidColorScrollbarLayer::SetScrollLayer

BUG= 693740 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2728253002
Cr-Commit-Position: refs/heads/master@{#455105}

[modify] https://crrev.com/a20ad732333dc9db2b545886acd16a11abdc5842/cc/blink/web_compositor_support_impl.cc
[modify] https://crrev.com/a20ad732333dc9db2b545886acd16a11abdc5842/cc/blink/web_compositor_support_impl.h
[modify] https://crrev.com/a20ad732333dc9db2b545886acd16a11abdc5842/cc/blink/web_scrollbar_layer_impl.cc
[modify] https://crrev.com/a20ad732333dc9db2b545886acd16a11abdc5842/cc/blink/web_scrollbar_layer_impl.h
[modify] https://crrev.com/a20ad732333dc9db2b545886acd16a11abdc5842/cc/layers/painted_overlay_scrollbar_layer.cc
[modify] https://crrev.com/a20ad732333dc9db2b545886acd16a11abdc5842/cc/layers/painted_overlay_scrollbar_layer.h
[modify] https://crrev.com/a20ad732333dc9db2b545886acd16a11abdc5842/cc/layers/painted_scrollbar_layer.cc
[modify] https://crrev.com/a20ad732333dc9db2b545886acd16a11abdc5842/cc/layers/painted_scrollbar_layer.h
[modify] https://crrev.com/a20ad732333dc9db2b545886acd16a11abdc5842/cc/layers/scrollbar_layer_interface.h
[modify] https://crrev.com/a20ad732333dc9db2b545886acd16a11abdc5842/cc/layers/scrollbar_layer_unittest.cc
[modify] https://crrev.com/a20ad732333dc9db2b545886acd16a11abdc5842/cc/layers/solid_color_scrollbar_layer.cc
[modify] https://crrev.com/a20ad732333dc9db2b545886acd16a11abdc5842/cc/layers/solid_color_scrollbar_layer.h
[modify] https://crrev.com/a20ad732333dc9db2b545886acd16a11abdc5842/third_party/WebKit/Source/core/frame/VisualViewport.cpp
[modify] https://crrev.com/a20ad732333dc9db2b545886acd16a11abdc5842/third_party/WebKit/Source/core/frame/VisualViewport.h
[modify] https://crrev.com/a20ad732333dc9db2b545886acd16a11abdc5842/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp
[modify] https://crrev.com/a20ad732333dc9db2b545886acd16a11abdc5842/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.h
[modify] https://crrev.com/a20ad732333dc9db2b545886acd16a11abdc5842/third_party/WebKit/Source/platform/testing/TestingPlatformSupport.cpp
[modify] https://crrev.com/a20ad732333dc9db2b545886acd16a11abdc5842/third_party/WebKit/Source/platform/testing/TestingPlatformSupport.h
[modify] https://crrev.com/a20ad732333dc9db2b545886acd16a11abdc5842/third_party/WebKit/Source/web/WebViewImpl.cpp
[modify] https://crrev.com/a20ad732333dc9db2b545886acd16a11abdc5842/third_party/WebKit/public/platform/WebCompositorSupport.h
[modify] https://crrev.com/a20ad732333dc9db2b545886acd16a11abdc5842/third_party/WebKit/public/platform/WebScrollbarLayer.h

Project Member

Comment 22 by bugdroid1@chromium.org, Mar 9 2017

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

commit 53707d796144fb0c60f081e4477b31a7adab4e6d
Author: pdr <pdr@chromium.org>
Date: Thu Mar 09 19:30:09 2017

Revert of Remove indirection: setup scrollbar scroll layers in the scrollbar constructor (patchset #5 id:80001 of https://codereview.chromium.org/2728253002/ )

Reason for revert:
Regressed scrolling performance:  https://crbug.com/699705 

Original issue's description:
> Remove indirection: setup scrollbar scroll layers in the scrollbar constructor
>
> Scrollbar layers keep track of the layer id (soon: element id) that they
> actually scroll. Before this patch, scrollbar layers were constructed
> with no scroll layer id, then later updated to set the scroll layer id.
> This patch removes this indirection and directly sets the scroll layer
> in the constructor.
>
> Removed functions:
>   VisualViewport::setScrollLayerOnScrollbars
>   WebScrollbarLayer::setScrollLayer
>   WebScrollbarLayerImpl::setScrollLayer
>   PaintedOverlayScrollbarLayer::SetScrollLayer
>   PaintedScrollbarLayer::SetScrollLayer
>   SolidColorScrollbarLayer::SetScrollLayer
>
> BUG= 693740 
> CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
>
> Review-Url: https://codereview.chromium.org/2728253002
> Cr-Commit-Position: refs/heads/master@{#455105}
> Committed: https://chromium.googlesource.com/chromium/src/+/a20ad732333dc9db2b545886acd16a11abdc5842

TBR=ajuma@chromium.org,bokan@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 693740 

Review-Url: https://codereview.chromium.org/2743653003
Cr-Commit-Position: refs/heads/master@{#455828}

[modify] https://crrev.com/53707d796144fb0c60f081e4477b31a7adab4e6d/cc/blink/web_compositor_support_impl.cc
[modify] https://crrev.com/53707d796144fb0c60f081e4477b31a7adab4e6d/cc/blink/web_compositor_support_impl.h
[modify] https://crrev.com/53707d796144fb0c60f081e4477b31a7adab4e6d/cc/blink/web_scrollbar_layer_impl.cc
[modify] https://crrev.com/53707d796144fb0c60f081e4477b31a7adab4e6d/cc/blink/web_scrollbar_layer_impl.h
[modify] https://crrev.com/53707d796144fb0c60f081e4477b31a7adab4e6d/cc/layers/painted_overlay_scrollbar_layer.cc
[modify] https://crrev.com/53707d796144fb0c60f081e4477b31a7adab4e6d/cc/layers/painted_overlay_scrollbar_layer.h
[modify] https://crrev.com/53707d796144fb0c60f081e4477b31a7adab4e6d/cc/layers/painted_scrollbar_layer.cc
[modify] https://crrev.com/53707d796144fb0c60f081e4477b31a7adab4e6d/cc/layers/painted_scrollbar_layer.h
[modify] https://crrev.com/53707d796144fb0c60f081e4477b31a7adab4e6d/cc/layers/scrollbar_layer_interface.h
[modify] https://crrev.com/53707d796144fb0c60f081e4477b31a7adab4e6d/cc/layers/scrollbar_layer_unittest.cc
[modify] https://crrev.com/53707d796144fb0c60f081e4477b31a7adab4e6d/cc/layers/solid_color_scrollbar_layer.cc
[modify] https://crrev.com/53707d796144fb0c60f081e4477b31a7adab4e6d/cc/layers/solid_color_scrollbar_layer.h
[modify] https://crrev.com/53707d796144fb0c60f081e4477b31a7adab4e6d/third_party/WebKit/Source/core/frame/VisualViewport.cpp
[modify] https://crrev.com/53707d796144fb0c60f081e4477b31a7adab4e6d/third_party/WebKit/Source/core/frame/VisualViewport.h
[modify] https://crrev.com/53707d796144fb0c60f081e4477b31a7adab4e6d/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp
[modify] https://crrev.com/53707d796144fb0c60f081e4477b31a7adab4e6d/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.h
[modify] https://crrev.com/53707d796144fb0c60f081e4477b31a7adab4e6d/third_party/WebKit/Source/platform/testing/TestingPlatformSupport.cpp
[modify] https://crrev.com/53707d796144fb0c60f081e4477b31a7adab4e6d/third_party/WebKit/Source/platform/testing/TestingPlatformSupport.h
[modify] https://crrev.com/53707d796144fb0c60f081e4477b31a7adab4e6d/third_party/WebKit/Source/web/WebViewImpl.cpp
[modify] https://crrev.com/53707d796144fb0c60f081e4477b31a7adab4e6d/third_party/WebKit/public/platform/WebCompositorSupport.h
[modify] https://crrev.com/53707d796144fb0c60f081e4477b31a7adab4e6d/third_party/WebKit/public/platform/WebScrollbarLayer.h

Project Member

Comment 23 by bugdroid1@chromium.org, Mar 17 2017

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

commit 8ffd86e6c9e28bcab01a3f59fb288a7f20df2090
Author: pdr <pdr@chromium.org>
Date: Fri Mar 17 18:50:06 2017

Use Layer::INVALID_ID instead of 0 for invalid layers in WebScrollbarLayerImpl

This patch fixes a bug where 0, a valid layer id, was used in place of an
invalid layer id in the constructor of WebScrollbarLayerImpl. This has
been split out of https://codereview.chromium.org/2728253002 which was
reverted.

BUG= 693740 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2751973007
Cr-Commit-Position: refs/heads/master@{#457831}

[modify] https://crrev.com/8ffd86e6c9e28bcab01a3f59fb288a7f20df2090/cc/blink/web_scrollbar_layer_impl.cc

Project Member

Comment 24 by bugdroid1@chromium.org, Mar 17 2017

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

commit 9655bcad1aa09a1dc837192866ca8110c0d68563
Author: nektar <nektar@chromium.org>
Date: Fri Mar 17 19:50:48 2017

Revert of Use Layer::INVALID_ID instead of 0 for invalid layers in WebScrollbarLayerImpl (patchset #1 id:1 of https://codereview.chromium.org/2751973007/ )

Reason for revert:
Possibly affecting CC_Unittests on Android.

TextureLayerChangeInvisibleMailboxTest.RunMultiThread_DelegatingRenderer
https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.linux%2FAndroid_Tests%2F39440%2F%2B%2Frecipes%2Fsteps%2Fcc_unittests_on_Android%2F0%2Flogs%2FTextureLayerChangeInvisibleMailboxTest.RunMultiThread_DelegatingRenderer%2F0

[ RUN      ] TextureLayerChangeInvisibleMailboxTest.RunMultiThread_DelegatingRenderer
../../cc/layers/texture_layer_unittest.cc:1195: Failure
Value of: 5
Expected: commit_count_
Which is: 4
[  FAILED  ] TextureLayerChangeInvisibleMailboxTest.RunMultiThread_DelegatingRenderer (25 ms)
[----------] 1 test from TextureLayerChangeInvisibleMailboxTest (25 ms total)

Original issue's description:
> Use Layer::INVALID_ID instead of 0 for invalid layers in WebScrollbarLayerImpl
>
> This patch fixes a bug where 0, a valid layer id, was used in place of an
> invalid layer id in the constructor of WebScrollbarLayerImpl. This has
> been split out of https://codereview.chromium.org/2728253002 which was
> reverted.
>
> BUG= 693740 
> CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
>
> Review-Url: https://codereview.chromium.org/2751973007
> Cr-Commit-Position: refs/heads/master@{#457831}
> Committed: https://chromium.googlesource.com/chromium/src/+/8ffd86e6c9e28bcab01a3f59fb288a7f20df2090

TBR=ajuma@chromium.org,pdr@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 693740 

Review-Url: https://codereview.chromium.org/2757973002
Cr-Commit-Position: refs/heads/master@{#457863}

[modify] https://crrev.com/9655bcad1aa09a1dc837192866ca8110c0d68563/cc/blink/web_scrollbar_layer_impl.cc

Project Member

Comment 25 by bugdroid1@chromium.org, Mar 18 2017

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

commit 4bed6671a93fb6d88d49cb8cd4136eb0257f1751
Author: pdr <pdr@chromium.org>
Date: Sat Mar 18 05:12:53 2017

Reland of Use Layer::INVALID_ID instead of 0 for invalid layers in WebScrollbarLayerImpl (patchset #1 id:1 of https://codereview.chromium.org/2757973002/ )

Reason for revert:
Reverting this revert as the root cause was  crbug.com/702868 

Original issue's description:
> Revert of Use Layer::INVALID_ID instead of 0 for invalid layers in WebScrollbarLayerImpl (patchset #1 id:1 of https://codereview.chromium.org/2751973007/ )
>
> Reason for revert:
> Possibly affecting CC_Unittests on Android.
>
> TextureLayerChangeInvisibleMailboxTest.RunMultiThread_DelegatingRenderer
> https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.linux%2FAndroid_Tests%2F39440%2F%2B%2Frecipes%2Fsteps%2Fcc_unittests_on_Android%2F0%2Flogs%2FTextureLayerChangeInvisibleMailboxTest.RunMultiThread_DelegatingRenderer%2F0
>
> [ RUN      ] TextureLayerChangeInvisibleMailboxTest.RunMultiThread_DelegatingRenderer
> ../../cc/layers/texture_layer_unittest.cc:1195: Failure
> Value of: 5
> Expected: commit_count_
> Which is: 4
> [  FAILED  ] TextureLayerChangeInvisibleMailboxTest.RunMultiThread_DelegatingRenderer (25 ms)
> [----------] 1 test from TextureLayerChangeInvisibleMailboxTest (25 ms total)
>
>
>
> Original issue's description:
> > Use Layer::INVALID_ID instead of 0 for invalid layers in WebScrollbarLayerImpl
> >
> > This patch fixes a bug where 0, a valid layer id, was used in place of an
> > invalid layer id in the constructor of WebScrollbarLayerImpl. This has
> > been split out of https://codereview.chromium.org/2728253002 which was
> > reverted.
> >
> > BUG= 693740 
> > CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
> >
> > Review-Url: https://codereview.chromium.org/2751973007
> > Cr-Commit-Position: refs/heads/master@{#457831}
> > Committed: https://chromium.googlesource.com/chromium/src/+/8ffd86e6c9e28bcab01a3f59fb288a7f20df2090
>
> TBR=ajuma@chromium.org,pdr@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG= 693740 
>
> Review-Url: https://codereview.chromium.org/2757973002
> Cr-Commit-Position: refs/heads/master@{#457863}
> Committed: https://chromium.googlesource.com/chromium/src/+/9655bcad1aa09a1dc837192866ca8110c0d68563

TBR=ajuma@chromium.org,nektar@chromium.org
BUG= 693740 

Review-Url: https://codereview.chromium.org/2756343002
Cr-Commit-Position: refs/heads/master@{#457958}

[modify] https://crrev.com/4bed6671a93fb6d88d49cb8cd4136eb0257f1751/cc/blink/web_scrollbar_layer_impl.cc

Comment 26 by pdr@chromium.org, Apr 18 2017

Description: Show this description
Project Member

Comment 27 by bugdroid1@chromium.org, Apr 18 2017

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

commit a91f5b16d907a71eb79896a54fc8c368d11dc061
Author: pdr <pdr@chromium.org>
Date: Tue Apr 18 20:03:52 2017

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/+/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}

[modify] https://crrev.com/a91f5b16d907a71eb79896a54fc8c368d11dc061/cc/trees/draw_property_utils.cc

Project Member

Comment 28 by bugdroid1@chromium.org, Apr 18 2017

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

commit 2501a834815e3a9e460051bd98f1eb49c7357e7b
Author: pdr <pdr@chromium.org>
Date: Tue Apr 18 21:57:37 2017

Revert of Remove UpdateScrollTree (patchset #1 id:1 of https://codereview.chromium.org/2822603002/ )

Reason for revert:
Patch failed to handle changes to scroll clip layer bounds.

Original issue's description:
> 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/+/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

TBR=weiliangc@chromium.org,enne@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 693740 

Review-Url: https://codereview.chromium.org/2830473002
Cr-Commit-Position: refs/heads/master@{#465390}

[modify] https://crrev.com/2501a834815e3a9e460051bd98f1eb49c7357e7b/cc/trees/draw_property_utils.cc

Project Member

Comment 29 by bugdroid1@chromium.org, Apr 18 2017

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

commit 315f18b6f11170412ef3e9057cb84eeaf24322b6
Author: pdr <pdr@chromium.org>
Date: Tue Apr 18 22:30:54 2017

Update element_id class comment to reflect how element id works

This patch updates the element_id comment to describe the modern use
of element ids in the compositor and blink.

BUG= 693740 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2826703003
Cr-Commit-Position: refs/heads/master@{#465400}

[modify] https://crrev.com/315f18b6f11170412ef3e9057cb84eeaf24322b6/cc/trees/element_id.h

Comment 30 by pdr@chromium.org, Apr 19 2017

Blocking: 712298
Project Member

Comment 31 by bugdroid1@chromium.org, Apr 19 2017

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

commit faf2ee225610f256279fe77bf1413bb512dec3c2
Author: pdr <pdr@chromium.org>
Date: Wed Apr 19 03:11:04 2017

Replace layer id with Element id for tracking scrollbar animation controllers

This patch tracks scrollbar animation controllers with ElementIds instead
of layer ids. This requires tracking the scrolling element id in
scrollbar layers in addition to the scrolling layer id. A future patch
will remove scrolling_layer_id from ScrollbarLayerInterface.

This patch get us closer to removing scroll_node's owning_layer_id by
removing 3 callers of owning_layer_id (in LayerTreeHostImpl) and 2 callers
of LayerIdByElementId (in LayerTreeImpl).

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/2816063003
Cr-Commit-Position: refs/heads/master@{#465480}

[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/blink/web_scrollbar_layer_impl.cc
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/input/scrollbar_animation_controller_unittest.cc
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/input/single_scrollbar_animation_controller_thinning_unittest.cc
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/layers/layer.cc
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/layers/layer.h
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/layers/layer_impl.cc
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/layers/layer_impl.h
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/layers/painted_overlay_scrollbar_layer.cc
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/layers/painted_overlay_scrollbar_layer.h
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/layers/painted_scrollbar_layer.cc
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/layers/painted_scrollbar_layer.h
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/layers/scrollbar_layer_impl_base.cc
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/layers/scrollbar_layer_impl_base.h
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/layers/scrollbar_layer_interface.h
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/layers/scrollbar_layer_unittest.cc
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/layers/solid_color_scrollbar_layer.cc
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/layers/solid_color_scrollbar_layer.h
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/test/fake_painted_scrollbar_layer.cc
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/test/fake_painted_scrollbar_layer.h
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/trees/layer_tree_host_common.cc
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/trees/layer_tree_host_common.h
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/trees/layer_tree_host_unittest.cc
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/trees/layer_tree_host_unittest_context.cc
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/trees/layer_tree_host_unittest_damage.cc
[modify] https://crrev.com/faf2ee225610f256279fe77bf1413bb512dec3c2/cc/trees/layer_tree_impl.cc

Project Member

Comment 32 by bugdroid1@chromium.org, Apr 19 2017

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

commit dfd089f1bd1aa1d8512741f2a103d0b87684f2e4
Author: pdr <pdr@chromium.org>
Date: Wed Apr 19 18:29:30 2017

Refactor FindScrollLayerForDeviceViewportPoint to use scroll nodes over layer ids

This patch refactors FindScrollLayerForDeviceViewportPoint as
FindScrollNodeForDeviceViewportPoint which both simplifies code and lets
us remove two users of ScrollNode's owning_layer_id.

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/2824683002
Cr-Commit-Position: refs/heads/master@{#465677}

[modify] https://crrev.com/dfd089f1bd1aa1d8512741f2a103d0b87684f2e4/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/dfd089f1bd1aa1d8512741f2a103d0b87684f2e4/cc/trees/layer_tree_host_impl.h

Project Member

Comment 33 by bugdroid1@chromium.org, Apr 19 2017

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

commit 7b8ed8a28833335aebe45f6e2c4193c7ee058515
Author: pdr <pdr@chromium.org>
Date: Wed Apr 19 21:24:27 2017

Refactor LayerTreeImpl's scrollbar map to be keyed on element ids

An element can have up to two scrollbars and we can switch LayerTreeImpl's
scrollbar tracking map from layer_id->scrollbar_layer_id to
element_id->scrollbar_layer_id. This patch gets us a little closer to
making the cc scrollbar code just depend on element ids instead of layer
ids.

With this patch we can remove the layer_id param from
LayerTreeHostImpl::RegisterScrollbarAnimationController which will make
it easy to remove layer ids from the scrollbar layers in a followup (i.e.,
track just scroll_element_id instead of scroll_layer_id and scroll_element_id).

This patch is primarily a switch from layer_id to element_id. Some tests
needed updates to ensure the scrolling layer's element id is set.

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/2824693002
Cr-Commit-Position: refs/heads/master@{#465754}

[modify] https://crrev.com/7b8ed8a28833335aebe45f6e2c4193c7ee058515/cc/input/scrollbar_animation_controller.cc
[modify] https://crrev.com/7b8ed8a28833335aebe45f6e2c4193c7ee058515/cc/input/scrollbar_animation_controller.h
[modify] https://crrev.com/7b8ed8a28833335aebe45f6e2c4193c7ee058515/cc/input/scrollbar_animation_controller_unittest.cc
[modify] https://crrev.com/7b8ed8a28833335aebe45f6e2c4193c7ee058515/cc/input/single_scrollbar_animation_controller_thinning.cc
[modify] https://crrev.com/7b8ed8a28833335aebe45f6e2c4193c7ee058515/cc/input/single_scrollbar_animation_controller_thinning.h
[modify] https://crrev.com/7b8ed8a28833335aebe45f6e2c4193c7ee058515/cc/input/single_scrollbar_animation_controller_thinning_unittest.cc
[modify] https://crrev.com/7b8ed8a28833335aebe45f6e2c4193c7ee058515/cc/layers/scrollbar_layer_unittest.cc
[modify] https://crrev.com/7b8ed8a28833335aebe45f6e2c4193c7ee058515/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/7b8ed8a28833335aebe45f6e2c4193c7ee058515/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/7b8ed8a28833335aebe45f6e2c4193c7ee058515/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/7b8ed8a28833335aebe45f6e2c4193c7ee058515/cc/trees/layer_tree_host_unittest_damage.cc
[modify] https://crrev.com/7b8ed8a28833335aebe45f6e2c4193c7ee058515/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/7b8ed8a28833335aebe45f6e2c4193c7ee058515/cc/trees/layer_tree_impl.h

Project Member

Comment 34 by bugdroid1@chromium.org, Apr 20 2017

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

commit 7868a2fd251dc273fedd774f50a3b06fb8d81f9c
Author: pdr <pdr@chromium.org>
Date: Thu Apr 20 19:04:23 2017

Add ScrollTree::FindNodeFromElementId

The transform tree and effect trees both have this function and the
scroll tree feels left out. This patch adds FindNodeFromElementId to
the scroll tree and uses it in LayerTreeImpl::PushPropertiesTo. A
future patch will add more callers.

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/2832743002
Cr-Commit-Position: refs/heads/master@{#466087}

[modify] https://crrev.com/7868a2fd251dc273fedd774f50a3b06fb8d81f9c/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/7868a2fd251dc273fedd774f50a3b06fb8d81f9c/cc/trees/property_tree.cc
[modify] https://crrev.com/7868a2fd251dc273fedd774f50a3b06fb8d81f9c/cc/trees/property_tree.h

Project Member

Comment 35 by bugdroid1@chromium.org, Apr 20 2017

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

commit 75a6cc514e06cd92ccf0689898e15c6d2b6fca16
Author: pdr <pdr@chromium.org>
Date: Thu Apr 20 19:21:14 2017

Remove scroll layer ids from scrollbar layers

This patch removes scroll layer ids from scrollbar layers so scrollbars only
track the scrolling element id. The big changes in this patch are:
1) ScrollbarLayerInterface no longer has a scroll_layer_id and instead just
uses scroll_element_id. SetScrollInfo has been renamed SetScrollElementId.
2) Similarly, ScrollbarLayerImplBase no longer has a scroll_layer_id and
SetScrollInfo has been renamed SetScrollElementId.
3) Regrettably, two instances of LayerIdByElementId have been added in
RegisterScrollbar and CanScrollOrientation. These uses of LayerIdByElementId
require a 1:1 mapping between layers and element ids which is not necessary.
TODOs have been added to remove these.

This patch also fixes a bug in ScrollbarLayerImplBase::SetScrollLayerId where
scroll_element_id was compared to itself instead of scroll_element_id_.

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/2827163005
Cr-Commit-Position: refs/heads/master@{#466091}

[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/blink/web_scrollbar_layer_impl.cc
[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/input/scrollbar_animation_controller_unittest.cc
[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/input/single_scrollbar_animation_controller_thinning_unittest.cc
[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/layers/painted_overlay_scrollbar_layer.cc
[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/layers/painted_overlay_scrollbar_layer.h
[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/layers/painted_scrollbar_layer.cc
[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/layers/painted_scrollbar_layer.h
[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/layers/painted_scrollbar_layer_unittest.cc
[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/layers/scrollbar_layer_impl_base.cc
[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/layers/scrollbar_layer_impl_base.h
[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/layers/scrollbar_layer_interface.h
[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/layers/scrollbar_layer_unittest.cc
[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/layers/solid_color_scrollbar_layer.cc
[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/layers/solid_color_scrollbar_layer.h
[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/test/fake_painted_scrollbar_layer.cc
[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/test/fake_painted_scrollbar_layer.h
[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/trees/layer_tree_host_pixeltest_scrollbars.cc
[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/trees/layer_tree_host_unittest.cc
[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/trees/layer_tree_host_unittest_context.cc
[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/trees/layer_tree_host_unittest_damage.cc
[modify] https://crrev.com/75a6cc514e06cd92ccf0689898e15c6d2b6fca16/cc/trees/layer_tree_impl.cc

Project Member

Comment 36 by bugdroid1@chromium.org, Apr 25 2017

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

commit 23d381fd827c8880b85fe5794627e98af07454cc
Author: pdr <pdr@chromium.org>
Date: Tue Apr 25 21:06:31 2017

Move LayerImpl's bounds_delta to property trees

This patch fully moves LayerImpl::bounds_delta_ to PropertyTrees which
frees storage on LayerImpl and unifies logic.  PropertyTrees already
tracked these deltas in "*bounds_delta_" members, so they were stored
in two places. With the bounds deltas only on PropertyTrees,
LayerTreeImpl::UpdatePropertyTreesForBoundsDelta can be removed since
it is redundant.

As part of this refactoring, the bounds delta concept has been renamed
"ViewportBoundsDelta" to make it clear that it is only used for special
viewport layers, and to differentiate it from scroll deltas.

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/2840433002
Cr-Commit-Position: refs/heads/master@{#467111}

[modify] https://crrev.com/23d381fd827c8880b85fe5794627e98af07454cc/cc/layers/layer_impl.cc
[modify] https://crrev.com/23d381fd827c8880b85fe5794627e98af07454cc/cc/layers/layer_impl.h
[modify] https://crrev.com/23d381fd827c8880b85fe5794627e98af07454cc/cc/layers/layer_impl_unittest.cc
[modify] https://crrev.com/23d381fd827c8880b85fe5794627e98af07454cc/cc/layers/layer_position_constraint_unittest.cc
[modify] https://crrev.com/23d381fd827c8880b85fe5794627e98af07454cc/cc/test/fake_layer_tree_host.cc
[modify] https://crrev.com/23d381fd827c8880b85fe5794627e98af07454cc/cc/trees/layer_tree_host_common_unittest.cc
[modify] https://crrev.com/23d381fd827c8880b85fe5794627e98af07454cc/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/23d381fd827c8880b85fe5794627e98af07454cc/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/23d381fd827c8880b85fe5794627e98af07454cc/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/23d381fd827c8880b85fe5794627e98af07454cc/cc/trees/layer_tree_impl.h

Project Member

Comment 37 by bugdroid1@chromium.org, Apr 27 2017

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

commit ce1b52c931d839be349c13b0d4256ead6075336f
Author: pdr <pdr@chromium.org>
Date: Thu Apr 27 01:24:28 2017

Remove SetFixedContainerSizeDelta from LayerPositionConstraintUnittest

This function just wrapped simple logic and we can explicitly
call SetViewportBoundsDelta on the correct layer instead. This
is similar to https://codereview.chromium.org/2839533002 which
removed FixedContainerSizeDelta.

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/2843253002
Cr-Commit-Position: refs/heads/master@{#467539}

[modify] https://crrev.com/ce1b52c931d839be349c13b0d4256ead6075336f/cc/layers/layer_position_constraint_unittest.cc

Project Member

Comment 38 by bugdroid1@chromium.org, Apr 27 2017

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

commit 8f08ca970029c2cf5ba9235d14efed69dcda91f7
Author: pdr <pdr@chromium.org>
Date: Thu Apr 27 01:25:40 2017

Remove Layer/LayerImpl's FixedContainerSizeDelta

LayerImpl::FixedContainerSizeDelta was only needed for tests and these
tests can be simplified to just access the appropriate container
viewport bounds directly off the property trees.

Layer::FixedContainerSizeDelta has been removed because it is not used.

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/2839533002
Cr-Commit-Position: refs/heads/master@{#467540}

[modify] https://crrev.com/8f08ca970029c2cf5ba9235d14efed69dcda91f7/cc/layers/layer.h
[modify] https://crrev.com/8f08ca970029c2cf5ba9235d14efed69dcda91f7/cc/layers/layer_impl.cc
[modify] https://crrev.com/8f08ca970029c2cf5ba9235d14efed69dcda91f7/cc/layers/layer_impl.h
[modify] https://crrev.com/8f08ca970029c2cf5ba9235d14efed69dcda91f7/cc/trees/layer_tree_host_impl_unittest.cc

Project Member

Comment 39 by bugdroid1@chromium.org, May 10 2017

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

commit a2fe572b1f2bb6e2f9c59227de08a738eed0c00e
Author: pdr <pdr@chromium.org>
Date: Wed May 10 15:20:22 2017

Cleanup SetViewportLayersFromIds calls in LayerTreeImplTest

https://crrev.com/76ddc8770c3b80af44f97ddaa32d14098beb0ff8 added a call
to SetViewportLayersFromIds with layer ids of 0 instead of INVALID_ID,
but these 0 layer ids are not valid layers. This patch switches these
0's to real invalid layer ids. In addition, there is no need to call
SetViewportLayersFromIds twice so the two calls have been unified.

BUG= 693740 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2868583002
Cr-Commit-Position: refs/heads/master@{#470578}

[modify] https://crrev.com/a2fe572b1f2bb6e2f9c59227de08a738eed0c00e/cc/trees/layer_tree_impl_unittest.cc

Project Member

Comment 40 by bugdroid1@chromium.org, May 12 2017

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

commit b0fbfe1542e538079f74439a578a2b08cb8a88ba
Author: pdr <pdr@chromium.org>
Date: Fri May 12 22:22:17 2017

Remove one use of ScrollNode's owning_layer_id for StickyPositionOffset

This patch removes one more owning_layer_id. This is the first of many
related to scroll offsets.

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/2874793004
Cr-Commit-Position: refs/heads/master@{#471465}

[modify] https://crrev.com/b0fbfe1542e538079f74439a578a2b08cb8a88ba/cc/trees/property_tree.cc

Project Member

Comment 41 by bugdroid1@chromium.org, May 12 2017

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

commit 64b691c1dfc3e667a1c615e29abf08304c8d5734
Author: pdr <pdr@chromium.org>
Date: Fri May 12 23:37:57 2017

Cleanup the Layer::SetScrollOffset{FromImplSide} property tree fast-path

This patch updates Layer::SetScrollOffset{FromImplSide} to clarify how the
scroll offset fast-path works:
1) Existing property nodes are updated if they exist without invalidating
the property trees.
2) If no existing property nodes exist, the property trees should already be
marked as needig a full rebuild which will update the scroll offset.

DCHECKs have been added to clarify:
* SetScrollOffset should not be called for non-scrollable Layers.
* The scroll transform node does not need to be looked up using layer ids
because it is available via the Layer's transform_node_index().

This patch removes two users of transform node's owning layer id.

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/2877673002
Cr-Commit-Position: refs/heads/master@{#471490}

[modify] https://crrev.com/64b691c1dfc3e667a1c615e29abf08304c8d5734/cc/layers/layer.cc
[modify] https://crrev.com/64b691c1dfc3e667a1c615e29abf08304c8d5734/cc/layers/layer.h
[modify] https://crrev.com/64b691c1dfc3e667a1c615e29abf08304c8d5734/cc/layers/scrollbar_layer_unittest.cc
[modify] https://crrev.com/64b691c1dfc3e667a1c615e29abf08304c8d5734/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositorTest.cpp

Project Member

Comment 42 by bugdroid1@chromium.org, May 16 2017

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

commit 80ecb98035a6654fff02e86877be39f51fcc3d54
Author: pdr <pdr@chromium.org>
Date: Tue May 16 03:40:09 2017

Harmonize LayerTreeHost/LayerTreeHostImpl synchronization steps

Both LayerTreeHost::FinishCommitOnImpl and LayerTreeHostImpl::ActivateSyncTree
follow these rough steps to synchronize with the LayerTreeHostImpl:
1) Update property trees.
2) Update layer properties, possibly with side effects.
3) Push layer tree host properties (e.g., viewport types and page scale factor).

These steps have implicit dependencies: layer property updates may require
property trees, and the viewport layer type update may require layer properties.

Before this patch, LayerTreeHost and LayerTreeHostImpl followed these steps
in different orders. This patch refactors the synchronization code to use
the same ordering, and adds DCHECKs to enforce dependencies.

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/2873313004
Cr-Commit-Position: refs/heads/master@{#472004}

[modify] https://crrev.com/80ecb98035a6654fff02e86877be39f51fcc3d54/cc/layers/layer.cc
[modify] https://crrev.com/80ecb98035a6654fff02e86877be39f51fcc3d54/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/80ecb98035a6654fff02e86877be39f51fcc3d54/cc/trees/layer_tree_host.h
[modify] https://crrev.com/80ecb98035a6654fff02e86877be39f51fcc3d54/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/80ecb98035a6654fff02e86877be39f51fcc3d54/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/80ecb98035a6654fff02e86877be39f51fcc3d54/cc/trees/layer_tree_impl.h

Project Member

Comment 43 by bugdroid1@chromium.org, May 16 2017

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

commit b4d70c2b28f321a94a52615c95692b6da8738783
Author: pdr <pdr@chromium.org>
Date: Tue May 16 17:48:24 2017

Remove redundant update of viewport layer types

LayerImpl's viewport layer types depend on (1) LayerTreeImpl's viewport
layers and (2) LayerImpl's scroll_clip_layer. [1] added code to update
the layer type when either (1) or (2) change.

With the refactoring of [2], LayerImpl's properties such as
scroll_clip_layer are always set before LayerTreeImpl's viewport layers
are updated. This patch removes an unnecessary call to
LayerTreeImpl::UpdateViewportLayerTypes from LayerImpl::SetScrollClipLayer
and proves it is safe with a new DCHECK.

Add a cache of LayerImpl's viewport layer type
[1] https://chromium.googlesource.com/chromium/src/+/6280cc1d5a1dccfbe23ca438eed862e245f18614

Harmonize LayerTreeHost/LayerTreeHostImpl synchronization steps
[2] https://chromium.googlesource.com/chromium/src/+/80ecb98035a6654fff02e86877be39f51fcc3d54

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/2882403002
Cr-Commit-Position: refs/heads/master@{#472150}

[modify] https://crrev.com/b4d70c2b28f321a94a52615c95692b6da8738783/cc/layers/layer_impl.cc
[modify] https://crrev.com/b4d70c2b28f321a94a52615c95692b6da8738783/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/b4d70c2b28f321a94a52615c95692b6da8738783/cc/trees/layer_tree_impl.h

Comment 44 by pdr@chromium.org, May 17 2017

Blockedon: 723263
Project Member

Comment 45 by bugdroid1@chromium.org, Jun 9 2017

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

commit 54d0381911a5e03dbaaf549e0afe2dba19a6943a
Author: pdr <pdr@chromium.org>
Date: Fri Jun 09 00:44:02 2017

Key scroll offsets on ElementIds instead of layer ids

Scroll offsets are stored on the ScrollTree but were keyed on layer
ids. This patch switches the scroll offsets to be keyed on ElementIds
which gets us closer to removing the Layer dependency from scrolling.

This patch removes 10 users of ScrollNode's owning_layer_id.

Bug:  693740 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ib6b9681518748fc223d19adce336ff9a889c9325
Reviewed-on: https://chromium-review.googlesource.com/527346
Commit-Queue: Philip Rogers <pdr@chromium.org>
Reviewed-by: Chris harrelson <chrishtr@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#478140}
[modify] https://crrev.com/54d0381911a5e03dbaaf549e0afe2dba19a6943a/cc/layers/layer.cc
[modify] https://crrev.com/54d0381911a5e03dbaaf549e0afe2dba19a6943a/cc/layers/layer_impl.cc
[modify] https://crrev.com/54d0381911a5e03dbaaf549e0afe2dba19a6943a/cc/layers/layer_impl_unittest.cc
[modify] https://crrev.com/54d0381911a5e03dbaaf549e0afe2dba19a6943a/cc/layers/layer_position_constraint_unittest.cc
[modify] https://crrev.com/54d0381911a5e03dbaaf549e0afe2dba19a6943a/cc/layers/picture_layer_impl_perftest.cc
[modify] https://crrev.com/54d0381911a5e03dbaaf549e0afe2dba19a6943a/cc/test/layer_tree_test.cc
[modify] https://crrev.com/54d0381911a5e03dbaaf549e0afe2dba19a6943a/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/54d0381911a5e03dbaaf549e0afe2dba19a6943a/cc/trees/layer_tree_host_common.cc
[modify] https://crrev.com/54d0381911a5e03dbaaf549e0afe2dba19a6943a/cc/trees/layer_tree_host_common.h
[modify] https://crrev.com/54d0381911a5e03dbaaf549e0afe2dba19a6943a/cc/trees/layer_tree_host_common_unittest.cc
[modify] https://crrev.com/54d0381911a5e03dbaaf549e0afe2dba19a6943a/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/54d0381911a5e03dbaaf549e0afe2dba19a6943a/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/54d0381911a5e03dbaaf549e0afe2dba19a6943a/cc/trees/layer_tree_host_unittest_scroll.cc
[modify] https://crrev.com/54d0381911a5e03dbaaf549e0afe2dba19a6943a/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/54d0381911a5e03dbaaf549e0afe2dba19a6943a/cc/trees/layer_tree_impl.h
[modify] https://crrev.com/54d0381911a5e03dbaaf549e0afe2dba19a6943a/cc/trees/property_tree.cc
[modify] https://crrev.com/54d0381911a5e03dbaaf549e0afe2dba19a6943a/cc/trees/property_tree.h
[modify] https://crrev.com/54d0381911a5e03dbaaf549e0afe2dba19a6943a/cc/trees/property_tree_builder.cc
[modify] https://crrev.com/54d0381911a5e03dbaaf549e0afe2dba19a6943a/cc/trees/tree_synchronizer_unittest.cc
[modify] https://crrev.com/54d0381911a5e03dbaaf549e0afe2dba19a6943a/third_party/WebKit/Source/platform/graphics/CompositorMutableState.cpp
[modify] https://crrev.com/54d0381911a5e03dbaaf549e0afe2dba19a6943a/third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp

Project Member

Comment 46 by bugdroid1@chromium.org, Jul 1 2017

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

commit c82cbb231b6c57f5993a7674afa9ef53b0a362bf
Author: Philip Rogers <pdr@chromium.org>
Date: Sat Jul 01 01:29:28 2017

Remove ScrollNode's owning_layer_id

ScrollNode no longer needs an owning_layer_id.

Bug:  693740 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I3702b3455737e29a2b75ba4d3c1127676588347b
Reviewed-on: https://chromium-review.googlesource.com/558017
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483889}
[modify] https://crrev.com/c82cbb231b6c57f5993a7674afa9ef53b0a362bf/cc/trees/layer_tree_host_common_unittest.cc
[modify] https://crrev.com/c82cbb231b6c57f5993a7674afa9ef53b0a362bf/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/c82cbb231b6c57f5993a7674afa9ef53b0a362bf/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/c82cbb231b6c57f5993a7674afa9ef53b0a362bf/cc/trees/property_tree_builder.cc
[modify] https://crrev.com/c82cbb231b6c57f5993a7674afa9ef53b0a362bf/cc/trees/scroll_node.cc
[modify] https://crrev.com/c82cbb231b6c57f5993a7674afa9ef53b0a362bf/cc/trees/scroll_node.h
[modify] https://crrev.com/c82cbb231b6c57f5993a7674afa9ef53b0a362bf/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositorTest.cpp
[modify] https://crrev.com/c82cbb231b6c57f5993a7674afa9ef53b0a362bf/third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp

Comment 47 by pdr@chromium.org, Jul 1 2017

Status: Fixed (was: Assigned)

Comment 48 by pdr@chromium.org, Jul 13 2017

Blocking: 471333

Sign in to add a comment