Deduplicating compositing scrollingCoordinator helper

Deduplicating compositing scrollingCoordinator helper

In the experiments with unity builds I ran into the
scrollingCoordinatorFromLayer function that was defined identically at
two different places. This patch moves the code to PaintLayer where
it is also available to PaintLayer itself.

R=pdr@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2803013005
Cr-Commit-Position: refs/heads/master@{#464054}
Committed: https://chromium.googlesource.com/chromium/src/+/a25783c221c34f889678d96b8c119c976b0bb45a

[Daniel Bratell, 2017-04-07 19:11:21]

Description was changed from

==========
Deduplicating compositing scrollingCoordinator helper

In the experiments with unity builds I ran into the
scrollingCoordinatorFromLayer function that was defined identically at
two different places. This patch moves the code to
CompositedLayerMapping where it can be used from both places and
deleted the extra copy.

R=pdr@chromium.org
==========

to

==========
Deduplicating compositing scrollingCoordinator helper

In the experiments with unity builds I ran into the
scrollingCoordinatorFromLayer function that was defined identically at
two different places. This patch moves the code to
CompositedLayerMapping where it can be used from both places and
deleted the extra copy.

R=pdr@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Daniel Bratell, 2017-04-07 19:14:52]

bratell@opera.com changed reviewers:
+ wangxianzhu@chromium.org

[Daniel Bratell, 2017-04-07 19:14:53]

Please take a look.

[Daniel Bratell, 2017-04-07 19:17:58]

The CQ bit was checked by bratell@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-07 19:18:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Xianzhu, 2017-04-07 19:26:19]

https://codereview.chromium.org/2803013005/diff/1/third_party/WebKit/Source/c...
File
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
(left):

https://codereview.chromium.org/2803013005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp:165:
static ScrollingCoordinator* scrollingCoordinatorFromLayer(PaintLayer& layer) {
It seems better to be PaintLayer::getScrollingCoordinator().

Then we can also use it at
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/paint/Pai....

[commit-bot: I haz the power, 2017-04-07 20:47:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-07 20:47:10]

Dry run: This issue passed the CQ dry run.

[Daniel Bratell, 2017-04-08 08:08:24]

https://codereview.chromium.org/2803013005/diff/1/third_party/WebKit/Source/c...
File
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
(left):

https://codereview.chromium.org/2803013005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp:165:
static ScrollingCoordinator* scrollingCoordinatorFromLayer(PaintLayer& layer) {
On 2017/04/07 19:26:19, Xianzhu OOO 4.8 - 4.16 wrote:
> It seems better to be PaintLayer::getScrollingCoordinator().
> 
> Then we can also use it at
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/paint/Pai....

Done.

Note that the null checks were not identical at the different places so it did
not replace all the code in the PaintLayer destructor. But then, the destructor
is a special place.

[Daniel Bratell, 2017-04-08 08:08:31]

The CQ bit was checked by bratell@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-08 08:08:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Daniel Bratell, 2017-04-08 08:09:35]

Description was changed from

==========
Deduplicating compositing scrollingCoordinator helper

In the experiments with unity builds I ran into the
scrollingCoordinatorFromLayer function that was defined identically at
two different places. This patch moves the code to
CompositedLayerMapping where it can be used from both places and
deleted the extra copy.

R=pdr@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Deduplicating compositing scrollingCoordinator helper

In the experiments with unity builds I ran into the
scrollingCoordinatorFromLayer function that was defined identically at
two different places. This patch moves the code to PaintLayer where
it is also available to PaintLayer itself.

R=pdr@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[commit-bot: I haz the power, 2017-04-08 09:49:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-08 09:49:27]

Dry run: This issue passed the CQ dry run.

[pdr., 2017-04-10 01:06:25]

On 2017/04/08 at 08:08:24, bratell wrote:
>
https://codereview.chromium.org/2803013005/diff/1/third_party/WebKit/Source/c...
> File
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
(left):
> 
>
https://codereview.chromium.org/2803013005/diff/1/third_party/WebKit/Source/c...
>
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp:165:
static ScrollingCoordinator* scrollingCoordinatorFromLayer(PaintLayer& layer) {
> On 2017/04/07 19:26:19, Xianzhu OOO 4.8 - 4.16 wrote:
> > It seems better to be PaintLayer::getScrollingCoordinator().
> > 
> > Then we can also use it at
> >
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/paint/Pai....
> 
> Done.
> 
> Note that the null checks were not identical at the different places so it did
not replace all the code in the PaintLayer destructor. But then, the destructor
is a special place.

LGTM

[Daniel Bratell, 2017-04-12 14:54:25]

The CQ bit was checked by bratell@opera.com

[Daniel Bratell, 2017-04-12 14:54:25]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org
Link to the patchset: https://codereview.chromium.org/2803013005/#ps40001
(title: "Rebased post blink rename")

[commit-bot: I haz the power, 2017-04-12 14:54:46]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-12 16:41:31]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1492008865165550,
"parent_rev": "65c4eb23fd50e9e5199e8ab0fab8f652b69e277e", "commit_rev":
"a25783c221c34f889678d96b8c119c976b0bb45a"}

[commit-bot: I haz the power, 2017-04-12 16:46:39]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1492008865165550,
"parent_rev": "65c4eb23fd50e9e5199e8ab0fab8f652b69e277e", "commit_rev":
"a25783c221c34f889678d96b8c119c976b0bb45a"}

[commit-bot: I haz the power, 2017-04-12 16:46:48]

Description was changed from

==========
Deduplicating compositing scrollingCoordinator helper

In the experiments with unity builds I ran into the
scrollingCoordinatorFromLayer function that was defined identically at
two different places. This patch moves the code to PaintLayer where
it is also available to PaintLayer itself.

R=pdr@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Deduplicating compositing scrollingCoordinator helper

In the experiments with unity builds I ran into the
scrollingCoordinatorFromLayer function that was defined identically at
two different places. This patch moves the code to PaintLayer where
it is also available to PaintLayer itself.

R=pdr@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2803013005
Cr-Commit-Position: refs/heads/master@{#464054}
Committed:
https://chromium.googlesource.com/chromium/src/+/a25783c221c34f889678d96b8c11...
==========

[commit-bot: I haz the power, 2017-04-12 16:46:49]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/a25783c221c34f889678d96b8c11...