Split NativeThemeAura into Overlay and NonOverlay versions.

Split NativeThemeAura into Overlay and NonOverlay versions.

This patch moves the overlay scrollbar functionality from NativeThemeAura out
into NativeThemeAuraOverlay. This allows Views code to continue using
non-overlay scrollbars while web content uses overlays.

I've also replaced the check for overlays from ui::IsOverlayScrollbarEnabled to
a bool passed in by the caller. IsOverlayScrollbarEnabled is statically
determined and is meant to indicate behavior in stable for end-users. Based on
its value, Chrome sets the Blink overlay scrollbar RuntimeEnabledFeature.
However, tests mostly run with this REF disabled so we need some way to
determine which NativeTheme to use at runtime. Otherwise, these tests will run
with Blink thinking it's using non-overlay scrollbars but when Blink asks
NativeTheme to do some painting, NativeTheme thinks it's using the overlay
theme. Now Blink passes the value of the REF so that we always choose the
correct NativeTheme.

BUG=307091

[bokan, 2016-11-05 18:24:32]

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

[commit-bot: I haz the power, 2016-11-05 18:24:39]

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, 2016-11-05 18:28:29]

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

[commit-bot: I haz the power, 2016-11-05 18:28:30]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[bokan, 2016-11-05 18:32:16]

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

[commit-bot: I haz the power, 2016-11-05 18:32:26]

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

[bokan, 2016-11-05 21:13:24]

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

[commit-bot: I haz the power, 2016-11-05 21:13:32]

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, 2016-11-05 23:14:09]

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

[commit-bot: I haz the power, 2016-11-05 23:14:10]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[bokan, 2016-11-06 20:05:34]

Description was changed from

==========
Split NativeThemeAura into Overlay and NonOverlay versions.

This patch moves the overlay scrollbar functionality from NativeThemeAura out
into NativeThemeAuraOverlay. This allows Views code to continue using
non-overlay scrollbars while web content uses overlays.

I've also replaced the check for overlays from ui::IsOverlayScrollbarEnabled to
a bool passed in by the caller. IsOverlayScrollbarEnabled is statically
determined and is meant to indicate behavior in stable for end-users. Based on
its value, Chrome sets the Blink overlay scrollbar RuntimeEnabledFeature.
However, tests mostly run with this REF disabled so we need some way to
determine which NativeTheme to use at runtime. Otherwise, these tests will run
with Blink thinking it's using non-overlay scrollbars but when Blink asks
NativeTheme to do some painting, NativeTheme thinks it's using the overlay
theme. Now Blink passes the value of the REF so that we always choose the
correct NativeTheme.

BUG=307091
==========

to

==========
Split NativeThemeAura into Overlay and NonOverlay versions.

This patch moves the overlay scrollbar functionality from NativeThemeAura out
into NativeThemeAuraOverlay. This allows Views code to continue using
non-overlay scrollbars while web content uses overlays.

I've also replaced the check for overlays from ui::IsOverlayScrollbarEnabled to
a bool passed in by the caller. IsOverlayScrollbarEnabled is statically
determined and is meant to indicate behavior in stable for end-users. Based on
its value, Chrome sets the Blink overlay scrollbar RuntimeEnabledFeature.
However, tests mostly run with this REF disabled so we need some way to
determine which NativeTheme to use at runtime. Otherwise, these tests will run
with Blink thinking it's using non-overlay scrollbars but when Blink asks
NativeTheme to do some painting, NativeTheme thinks it's using the overlay
theme. Now Blink passes the value of the REF so that we always choose the
correct NativeTheme.

BUG=307091
==========

[bokan, 2016-11-06 20:05:34]

bokan@chromium.org changed reviewers:
+ estade@chromium.org

[Evan Stade, 2016-11-06 23:05:30]

Sorry that we clashed on this. My version is here[1]. I don't think there's a
lot that separates them but the other one has the advantage of requiring fewer
changes and fixing an additional TODO (plus getting us closer to making native
theme instantiation a good deal simpler/easier to understand[2]. What do you
think?

[1] https://codereview.chromium.org/2480993002/
[2] https://codereview.chromium.org/2483683002/

[bokan, 2016-11-06 23:38:56]

On 2016/11/06 23:05:30, Evan Stade wrote:
> Sorry that we clashed on this. My version is here[1]. I don't think there's a
> lot that separates them but the other one has the advantage of requiring fewer
> changes and fixing an additional TODO (plus getting us closer to making native
> theme instantiation a good deal simpler/easier to understand[2]. What do you
> think?
> 
> [1] https://codereview.chromium.org/2480993002/
> [2] https://codereview.chromium.org/2483683002/

Ok, sgtm to land your version. Could you add the change from this patch where we
choose whether to use overlays based on the RuntimeEnabledFeature? Otherwise
Blink and NativeTheme are in disagreement in tests that have overlays off.

[Evan Stade, 2016-11-09 00:44:53]

On 2016/11/06 23:38:56, bokan wrote:
> On 2016/11/06 23:05:30, Evan Stade wrote:
> > Sorry that we clashed on this. My version is here[1]. I don't think there's
a
> > lot that separates them but the other one has the advantage of requiring
fewer
> > changes and fixing an additional TODO (plus getting us closer to making
native
> > theme instantiation a good deal simpler/easier to understand[2]. What do you
> > think?
> > 
> > [1] https://codereview.chromium.org/2480993002/
> > [2] https://codereview.chromium.org/2483683002/
> 
> Ok, sgtm to land your version. Could you add the change from this patch where
we
> choose whether to use overlays based on the RuntimeEnabledFeature? Otherwise
> Blink and NativeTheme are in disagreement in tests that have overlays off.

this is not waiting for review and can be closed, right?