Do not ignore SolidColorScrollbarLayer's scroll_layer_id ctor argument

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}
Committed: https://chromium.googlesource.com/chromium/src/+/84d15f489b981ad9f33301abf3ee78f989aa20bf

[pdr., 2017-03-03 22:12:27]

Description was changed from

==========
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
==========

to

==========
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
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[pdr., 2017-03-03 22:12:46]

Description was changed from

==========
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
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
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
==========

[pdr., 2017-03-03 22:12:47]

pdr@chromium.org changed reviewers:
+ ajuma@chromium.org, bokan@chromium.org

[pdr., 2017-03-03 22:13:27]

The CQ bit was checked by pdr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-03 22:14:00]

Dry run: 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-03-03 23:06:10]

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

[commit-bot: I haz the power, 2017-03-03 23:06:12]

Dry run: This issue passed the CQ dry run.

[ajuma, 2017-03-04 00:04:50]

https://codereview.chromium.org/2734583002/diff/1/cc/layers/solid_color_scrol...
File cc/layers/solid_color_scrollbar_layer.cc (right):

https://codereview.chromium.org/2734583002/diff/1/cc/layers/solid_color_scrol...
cc/layers/solid_color_scrollbar_layer.cc:41: int scroll_layer_id)
It looks like this gets called in non-test code with scroll_layer_id hardcoded
to 0: 
https://cs.chromium.org/chromium/src/cc/blink/web_scrollbar_layer_impl.cc?rcl...

The same happens for the other scrollbar layer subclasses (that do use the
scroll_layer_id argument) though. But it seems confusing, since 0 is not the
same as INVALID_ID (which is -1).

Would it make sense as part of this cleanup to change the callsites that are
passing in 0 when they mean INVALID_ID to actually use INVALID_ID?

[pdr., 2017-03-04 02:44:23]

https://codereview.chromium.org/2734583002/diff/1/cc/layers/solid_color_scrol...
File cc/layers/solid_color_scrollbar_layer.cc (right):

https://codereview.chromium.org/2734583002/diff/1/cc/layers/solid_color_scrol...
cc/layers/solid_color_scrollbar_layer.cc:41: int scroll_layer_id)
On 2017/03/04 at 00:04:50, ajuma wrote:
> It looks like this gets called in non-test code with scroll_layer_id hardcoded
to 0: 
https://cs.chromium.org/chromium/src/cc/blink/web_scrollbar_layer_impl.cc?rcl...
> 
> The same happens for the other scrollbar layer subclasses (that do use the
scroll_layer_id argument) though. But it seems confusing, since 0 is not the
same as INVALID_ID (which is -1).
> 
> Would it make sense as part of this cleanup to change the callsites that are
passing in 0 when they mean INVALID_ID to actually use INVALID_ID?

We can actually do even better: these callsites are not needed at all. I have a
followup patch to remove them which cleans up a bunch of code:
https://codereview.chromium.org/2728253002

Mind if we land this patch as-is?

[bokan, 2017-03-06 14:17:59]

lgtm

[ajuma, 2017-03-06 14:22:55]

lgtm too

[pdr., 2017-03-06 17:08:21]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2017-03-06 17:08:37]

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-03-06 18:28:20]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1488820101748670, "parent_rev":
"3f08a50143c87ad0d41c03a0370884eccb788b39", "commit_rev":
"84d15f489b981ad9f33301abf3ee78f989aa20bf"}

[commit-bot: I haz the power, 2017-03-06 18:28:57]

Description was changed from

==========
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
==========

to

==========
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}
Committed:
https://chromium.googlesource.com/chromium/src/+/84d15f489b981ad9f33301abf3ee...
==========

[commit-bot: I haz the power, 2017-03-06 18:28:59]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/84d15f489b981ad9f33301abf3ee...