Toolbar: Do not include vector icons if on Android with no VR.

Toolbar: Do not include vector icons if on Android with no VR.

A previous change rolled components/toolbar into Android for use with
VR. But, to respect svelte devices, we should probably continue to not
include these icons if not needed for VR.

BUG=

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

[cjgrant, 2017-06-29 17:18:02]

cjgrant@chromium.org changed reviewers:
+ pkasting@chromium.org

[cjgrant, 2017-06-29 17:18:04]

Peter, my previous VR toolbar model change added vector icons to the Android
build.  I made a trivial build-fix post code review, but then realized I'm
actually growing the binary for Svelte/non-VR devices.

This CL is ugly, but maintains the conditional exclusion of icons.  Please let
me know if you want this or not.

[cjgrant, 2017-06-29 17:20:13]

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

[commit-bot: I haz the power, 2017-06-29 17:20:30]

Dry run: CQ is trying da patch.

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

[Peter Kasting, 2017-06-29 17:33:58]

LGTM, this isn't much pain for a binary size win.

https://codereview.chromium.org/2964513002/diff/1/components/toolbar/BUILD.gn
File components/toolbar/BUILD.gn (right):

https://codereview.chromium.org/2964513002/diff/1/components/toolbar/BUILD.gn...
components/toolbar/BUILD.gn:71: if (!(is_android && !enable_vr) && !is_ios) {
Nit: This nested !s expression is kinda hard to understand.  What about:

  if ((!is_android || enable_vr) && !is_ios)

https://codereview.chromium.org/2964513002/diff/1/components/toolbar/toolbar_...
File components/toolbar/toolbar_model_impl.cc (right):

https://codereview.chromium.org/2964513002/diff/1/components/toolbar/toolbar_...
components/toolbar/toolbar_model_impl.cc:25: #if !(defined(OS_ANDROID) &&
!BUILDFLAG(ENABLE_VR)) && !defined(OS_IOS)
Same comments on these.

[commit-bot: I haz the power, 2017-06-29 18:50:34]

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

[commit-bot: I haz the power, 2017-06-29 18:50:35]

Dry run: This issue passed the CQ dry run.

[cjgrant, 2017-06-29 21:13:22]

https://codereview.chromium.org/2964513002/diff/1/components/toolbar/BUILD.gn
File components/toolbar/BUILD.gn (right):

https://codereview.chromium.org/2964513002/diff/1/components/toolbar/BUILD.gn...
components/toolbar/BUILD.gn:71: if (!(is_android && !enable_vr) && !is_ios) {
On 2017/06/29 17:33:58, Peter Kasting wrote:
> Nit: This nested !s expression is kinda hard to understand.  What about:
> 
>   if ((!is_android || enable_vr) && !is_ios)

Done.

https://codereview.chromium.org/2964513002/diff/1/components/toolbar/toolbar_...
File components/toolbar/toolbar_model_impl.cc (right):

https://codereview.chromium.org/2964513002/diff/1/components/toolbar/toolbar_...
components/toolbar/toolbar_model_impl.cc:25: #if !(defined(OS_ANDROID) &&
!BUILDFLAG(ENABLE_VR)) && !defined(OS_IOS)
On 2017/06/29 17:33:58, Peter Kasting wrote:
> Same comments on these.

Done.

[cjgrant, 2017-06-29 21:13:28]

The CQ bit was checked by cjgrant@chromium.org

[cjgrant, 2017-06-29 21:13:28]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/2964513002/#ps20001
(title: "Demonstrate distributivity of negation for readability (ie. address
Peter's comment)")

[commit-bot: I haz the power, 2017-06-29 21:13:53]

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-06-29 22:39:32]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1498770808199000,
"parent_rev": "29af7c299ff70e3e2d203dd715af412dbcb87689", "commit_rev":
"f161d33b8b0fce61c0c29e7ff6dde6e64da68897"}

[commit-bot: I haz the power, 2017-06-29 22:39:47]

Description was changed from

==========
Toolbar: Do not include vector icons if on Android with no VR.

A previous change rolled components/toolbar into Android for use with
VR. But, to respect svelte devices, we should probably continue to not
include these icons if not needed for VR.

BUG=
==========

to

==========
Toolbar: Do not include vector icons if on Android with no VR.

A previous change rolled components/toolbar into Android for use with
VR. But, to respect svelte devices, we should probably continue to not
include these icons if not needed for VR.

BUG=

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

[commit-bot: I haz the power, 2017-06-29 22:39:48]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/f161d33b8b0fce61c0c29e7ff6dd...