VR: UiElement for Screen Capture indicator

VR: UiElement for Screen Capture indicator

- Screen share icon was moved from ash resources to a more common place (ui).

BUG=722836
TBR=blundell@chromium.org

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

[acondor_, 2017-05-16 19:23:43]

acondor@google.com changed reviewers:
+ cjgrant@chromium.org, mthiesse@chromium.org

[acondor_, 2017-05-16 19:23:43]

mthiesse, cjgrant PTAL before reaching OWNERS

[mthiesse, 2017-05-16 19:36:14]

lgtm

[acondor_, 2017-05-16 19:41:43]

acondor@google.com changed reviewers:
+ sky@chromium.org

[acondor_, 2017-05-16 19:41:43]

Hello +sky could you PTAL?

[sky, 2017-05-17 16:32:44]

sky@chromium.org changed reviewers:
+ estade@chromium.org, jam@chromium.org

[sky, 2017-05-17 16:32:45]

ui is intended to contain frameworks used to build ui. While we have used ui as
a dumping ground I'm trying to clean it up. Could you please put these icons in
components/vector_icons and we'll move the rest there at a later date? Add
estade as an OWNER of this new directory.

+estade for FYI
+jam as he is a top level owner and I made he sure agrees with this.

[acondor_, 2017-05-17 21:21:50]

On 2017/05/17 16:32:45, sky wrote:
> ui is intended to contain frameworks used to build ui. While we have used ui
as
> a dumping ground I'm trying to clean it up. Could you please put these icons
in
> components/vector_icons and we'll move the rest there at a later date? Add
> estade as an OWNER of this new directory.
> 
> +estade for FYI
> +jam as he is a top level owner and I made he sure agrees with this.

Thanks sky. It is now ready for another look.

[Evan Stade, 2017-05-17 21:51:41]

https://codereview.chromium.org/2891503002/diff/40001/components/vector_icons...
File components/vector_icons/BUILD.gn (right):

https://codereview.chromium.org/2891503002/diff/40001/components/vector_icons...
components/vector_icons/BUILD.gn:16: source_set("vector_icons") {
I think this should be a static_library.

[acondor_, 2017-05-18 14:14:39]

https://codereview.chromium.org/2891503002/diff/40001/components/vector_icons...
File components/vector_icons/BUILD.gn (right):

https://codereview.chromium.org/2891503002/diff/40001/components/vector_icons...
components/vector_icons/BUILD.gn:16: source_set("vector_icons") {
On 2017/05/17 21:51:40, Evan Stade wrote:
> I think this should be a static_library.

Done.

[jam, 2017-05-18 14:27:24]

On 2017/05/17 16:32:45, sky wrote:
> ui is intended to contain frameworks used to build ui. While we have used ui
as
> a dumping ground I'm trying to clean it up. Could you please put these icons
in
> components/vector_icons and we'll move the rest there at a later date? Add
> estade as an OWNER of this new directory.
> 
> +estade for FYI
> +jam as he is a top level owner and I made he sure agrees with this.

lgtm
(you're a top level owner too)

[Evan Stade, 2017-05-18 14:30:27]

lgtm

https://codereview.chromium.org/2891503002/diff/60001/chrome/browser/android/...
File chrome/browser/android/vr_shell/ui_elements/screen_capture_indicator.h
(right):

https://codereview.chromium.org/2891503002/diff/60001/chrome/browser/android/...
chrome/browser/android/vr_shell/ui_elements/screen_capture_indicator.h:24:
std::unique_ptr<UiTexture> texture_;
nit: does this need to be a (smart) pointer?

https://codereview.chromium.org/2891503002/diff/60001/components/vector_icons...
File components/vector_icons/vector_icons.cc.template (right):

https://codereview.chromium.org/2891503002/diff/60001/components/vector_icons...
components/vector_icons/vector_icons.cc.template:1: // Copyright 2016 The
Chromium Authors. All rights reserved.
2017

https://codereview.chromium.org/2891503002/diff/60001/components/vector_icons...
File components/vector_icons/vector_icons.h.template (right):

https://codereview.chromium.org/2891503002/diff/60001/components/vector_icons...
components/vector_icons/vector_icons.h.template:1: // Copyright 2016 The
Chromium Authors. All rights reserved.
ditto

[acondor_, 2017-05-18 15:54:00]

The CQ bit was checked by acondor@google.com

[acondor_, 2017-05-18 15:54:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
mthiesse@chromium.org, jam@chromium.org, estade@chromium.org
Link to the patchset: https://codereview.chromium.org/2891503002/#ps80001
(title: "nit")

[acondor_, 2017-05-18 15:54:18]

https://codereview.chromium.org/2891503002/diff/60001/chrome/browser/android/...
File chrome/browser/android/vr_shell/ui_elements/screen_capture_indicator.h
(right):

https://codereview.chromium.org/2891503002/diff/60001/chrome/browser/android/...
chrome/browser/android/vr_shell/ui_elements/screen_capture_indicator.h:24:
std::unique_ptr<UiTexture> texture_;
On 2017/05/18 14:30:27, Evan Stade wrote:
> nit: does this need to be a (smart) pointer?

Not quite, but changing this requires removing the const in |GetTexture|.
Leaving a TODO.

https://codereview.chromium.org/2891503002/diff/60001/components/vector_icons...
File components/vector_icons/vector_icons.cc.template (right):

https://codereview.chromium.org/2891503002/diff/60001/components/vector_icons...
components/vector_icons/vector_icons.cc.template:1: // Copyright 2016 The
Chromium Authors. All rights reserved.
On 2017/05/18 14:30:27, Evan Stade wrote:
> 2017

Done.

https://codereview.chromium.org/2891503002/diff/60001/components/vector_icons...
File components/vector_icons/vector_icons.h.template (right):

https://codereview.chromium.org/2891503002/diff/60001/components/vector_icons...
components/vector_icons/vector_icons.h.template:1: // Copyright 2016 The
Chromium Authors. All rights reserved.
On 2017/05/18 14:30:27, Evan Stade wrote:
> ditto

Done.

[commit-bot: I haz the power, 2017-05-18 15:54:58]

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-05-18 16:00:16]

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

[commit-bot: I haz the power, 2017-05-18 16:00:17]

Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[acondor_, 2017-05-18 17:58:48]

The CQ bit was checked by acondor@google.com

[acondor_, 2017-05-18 17:58:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
mthiesse@chromium.org, jam@chromium.org, estade@chromium.org
Link to the patchset: https://codereview.chromium.org/2891503002/#ps100001
(title: "rebase")

[commit-bot: I haz the power, 2017-05-18 18:00:05]

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-05-18 19:57:46]

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

[commit-bot: I haz the power, 2017-05-18 19:57:47]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[acondor_, 2017-05-18 20:14:10]

The CQ bit was checked by acondor@google.com

[commit-bot: I haz the power, 2017-05-18 20:15:47]

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-05-19 02:50:50]

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

[commit-bot: I haz the power, 2017-05-19 02:50:51]

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_...)

[acondor_, 2017-05-19 04:20:50]

The CQ bit was checked by acondor@google.com

[commit-bot: I haz the power, 2017-05-19 04:21:25]

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-05-19 04:30:31]

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

[commit-bot: I haz the power, 2017-05-19 04:30:33]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[acondor_, 2017-05-19 14:42:46]

The CQ bit was checked by acondor@google.com

[acondor_, 2017-05-19 14:42:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
mthiesse@chromium.org, jam@chromium.org, estade@chromium.org
Link to the patchset: https://codereview.chromium.org/2891503002/#ps120001
(title: "rebase 2")

[commit-bot: I haz the power, 2017-05-19 14:43:21]

CQ is trying da patch.

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

[acondor_, 2017-05-19 15:00:51]

Description was changed from

==========
VR: UiElement for Screen Capture indicator

- Screen share icon was moved from ash resources to a more common place (ui).

BUG=722836
==========

to

==========
VR: UiElement for Screen Capture indicator

- Screen share icon was moved from ash resources to a more common place (ui).

BUG=722836
TBR=blundell@chromium.org
==========

[acondor_, 2017-05-19 15:01:12]

acondor@google.com changed reviewers:
+ blundell@chromium.org

[acondor_, 2017-05-19 15:01:13]

blundell TBR

[commit-bot: I haz the power, 2017-05-19 15:52:51]

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

[commit-bot: I haz the power, 2017-05-19 15:52:52]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[acondor_, 2017-05-19 16:08:13]

The CQ bit was checked by acondor@google.com

[commit-bot: I haz the power, 2017-05-19 16:09:07]

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-05-19 18:35:18]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1495210093340870,
"parent_rev": "edd966f42e17c86c7658b9e3464838162640e18d", "commit_rev":
"f9d264174d41df066ab32166e4495f19da8c6d9c"}

[commit-bot: I haz the power, 2017-05-19 18:35:29]

Description was changed from

==========
VR: UiElement for Screen Capture indicator

- Screen share icon was moved from ash resources to a more common place (ui).

BUG=722836
TBR=blundell@chromium.org
==========

to

==========
VR: UiElement for Screen Capture indicator

- Screen share icon was moved from ash resources to a more common place (ui).

BUG=722836
TBR=blundell@chromium.org

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

[commit-bot: I haz the power, 2017-05-19 18:35:31]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/f9d264174d41df066ab32166e449...

[blundell, 2017-05-22 08:34:22]

lgtm

As a general rule, please get a review of a //components OWNER before adding a
new component...that doesn't really fall within the scope of TBR.