[Media Router] Add enable cloud services menu item to contextual menu.

[Media Router] Add enable cloud services menu item to contextual menu.

Part 7.

Add menu item to Media Router contextual menu to toggle the cloud services preference. It will be checked if cloud services is currently enabled.

This will only appear in Chrome branded builds.

BUG=560457
Committed: https://crrev.com/0320fa7c655973aaaa3d350be82e5d0f26b1d517
Cr-Commit-Position: refs/heads/master@{#371689}

[apacible, 2016-01-13 22:58:09]

Description was changed from

==========
nux contextual menu

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

to

==========
[Media Router] Add enable cloud services menu item to contextual menu.

TODO:
- Blocked behind adding kMediaRouterEnableCloudServices pref.
- Gate behind Chrome branded build.
- Disable if using Chrome but is disabled.

BUG=560457
==========

[apacible, 2016-01-13 23:11:54]

Description was changed from

==========
[Media Router] Add enable cloud services menu item to contextual menu.

TODO:
- Blocked behind adding kMediaRouterEnableCloudServices pref.
- Gate behind Chrome branded build.
- Disable if using Chrome but is disabled.

BUG=560457
==========

to

==========
[Media Router] Add enable cloud services menu item to contextual menu.

TODO:
- Blocked behind adding kMediaRouterEnableCloudServices pref.
- Gate behind Chrome branded build.

BUG=560457
==========

[apacible, 2016-01-13 23:11:59]

Patchset #1 (id:1) has been deleted

[apacible, 2016-01-14 01:47:39]

Description was changed from

==========
[Media Router] Add enable cloud services menu item to contextual menu.

TODO:
- Blocked behind adding kMediaRouterEnableCloudServices pref.
- Gate behind Chrome branded build.

BUG=560457
==========

to

==========
[Media Router] Add enable cloud services menu item to contextual menu.

Part [x] of several.

Add menu item to Media Router contextual menu to toggle the cloud services opt
in.

This will only appear in Chrome branded builds.

TODO:
- Blocked behind adding kMediaRouterEnableCloudServices pref.
- Gate behind Chrome branded build.

BUG=560457
==========

[apacible, 2016-01-14 01:50:40]

Description was changed from

==========
[Media Router] Add enable cloud services menu item to contextual menu.

Part [x] of several.

Add menu item to Media Router contextual menu to toggle the cloud services opt
in.

This will only appear in Chrome branded builds.

TODO:
- Blocked behind adding kMediaRouterEnableCloudServices pref.
- Gate behind Chrome branded build.

BUG=560457
==========

to

==========
[Media Router] Add enable cloud services menu item to contextual menu.

Part 6 of several.

Add menu item to Media Router contextual menu to toggle the cloud services opt
in.

This will only appear in Chrome branded builds.

TODO:
- Blocked behind adding kMediaRouterEnableCloudServices pref.
- Gate behind Chrome branded build.

BUG=560457
==========

[apacible, 2016-01-14 17:02:58]

Description was changed from

==========
[Media Router] Add enable cloud services menu item to contextual menu.

Part 6 of several.

Add menu item to Media Router contextual menu to toggle the cloud services opt
in.

This will only appear in Chrome branded builds.

TODO:
- Blocked behind adding kMediaRouterEnableCloudServices pref.
- Gate behind Chrome branded build.

BUG=560457
==========

to

==========
[Media Router] Add enable cloud services menu item to contextual menu.

Part 6 of several.

Add menu item to Media Router contextual menu to toggle the cloud services opt
in.

This will only appear in Chrome branded builds.

BUG=560457
==========

[apacible, 2016-01-20 19:58:27]

Patchset #1 (id:20001) has been deleted

[apacible, 2016-01-20 20:03:38]

Patchset #1 (id:40001) has been deleted

[apacible, 2016-01-20 20:04:02]

Description was changed from

==========
[Media Router] Add enable cloud services menu item to contextual menu.

Part 6 of several.

Add menu item to Media Router contextual menu to toggle the cloud services opt
in.

This will only appear in Chrome branded builds.

BUG=560457
==========

to

==========
[Media Router] Add enable cloud services menu item to contextual menu.

Part 7.

Add menu item to Media Router contextual menu to toggle the cloud services opt
in.

This will only appear in Chrome branded builds.

BUG=560457
==========

[apacible, 2016-01-21 17:51:36]

Patchset #1 (id:60001) has been deleted

[apacible, 2016-01-21 17:56:24]

Description was changed from

==========
[Media Router] Add enable cloud services menu item to contextual menu.

Part 7.

Add menu item to Media Router contextual menu to toggle the cloud services opt
in.

This will only appear in Chrome branded builds.

BUG=560457
==========

to

==========
[Media Router] Add enable cloud services menu item to contextual menu.

Part 7.

Add menu item to Media Router contextual menu to toggle the cloud services
preference. It will be checked if cloud services is currently enabled.

This will only appear in Chrome branded builds.

BUG=560457

Screenshots:
- Checked LTR: https://screenshot.googleplex.com/CZV1tueHbcE.png
- Unchecked LTR: https://screenshot.googleplex.com/0TYSdSmmb5x.png
- Checked RTL: https://screenshot.googleplex.com/1OxBz1s4TKY.png
- Unchecked RTL: https://screenshot.googleplex.com/GqcbXfHyVOW.png
==========

[apacible, 2016-01-21 18:29:01]

Patchset #1 (id:80001) has been deleted

[apacible, 2016-01-21 18:31:51]

Patchset #1 (id:100001) has been deleted

[apacible, 2016-01-26 16:48:33]

Patchset #1 (id:120001) has been deleted

[apacible, 2016-01-26 16:48:44]

Patchset #1 (id:140001) has been deleted

[apacible, 2016-01-26 16:52:50]

Description was changed from

==========
[Media Router] Add enable cloud services menu item to contextual menu.

Part 7.

Add menu item to Media Router contextual menu to toggle the cloud services
preference. It will be checked if cloud services is currently enabled.

This will only appear in Chrome branded builds.

BUG=560457

Screenshots:
- Checked LTR: https://screenshot.googleplex.com/CZV1tueHbcE.png
- Unchecked LTR: https://screenshot.googleplex.com/0TYSdSmmb5x.png
- Checked RTL: https://screenshot.googleplex.com/1OxBz1s4TKY.png
- Unchecked RTL: https://screenshot.googleplex.com/GqcbXfHyVOW.png
==========

to

==========
[Media Router] Add enable cloud services menu item to contextual menu.

Part 7.

Add menu item to Media Router contextual menu to toggle the cloud services
preference. It will be checked if cloud services is currently enabled.

This will only appear in Chrome branded builds.

BUG=560457
==========

[apacible, 2016-01-26 16:52:50]

apacible@chromium.org changed reviewers:
+ imcheng@chromium.org, sky@chromium.org

[apacible, 2016-01-26 16:53:54]

apacible@chromium.org changed reviewers:
- imcheng@chromium.org, sky@chromium.org

[apacible, 2016-01-26 17:04:49]

Patchset #1 (id:160001) has been deleted

[apacible, 2016-01-26 17:08:34]

apacible@chromium.org changed reviewers:
+ imcheng@chromium.org, sky@chromium.org

[apacible, 2016-01-26 17:09:39]

PTAL, thanks!

+imcheng for everything.
+sky for chrome_command_ids.h and c/b/ui/toolbar/*.

Screenshots:
- Checked LTR: https://screenshot.googleplex.com/CZV1tueHbcE.png
- Unchecked LTR: https://screenshot.googleplex.com/0TYSdSmmb5x.png
- Checked RTL: https://screenshot.googleplex.com/1OxBz1s4TKY.png
- Unchecked RTL: https://screenshot.googleplex.com/GqcbXfHyVOW.png

[sky, 2016-01-26 18:15:01]

LGTM

https://codereview.chromium.org/1582143002/diff/180001/chrome/browser/ui/tool...
File chrome/browser/ui/toolbar/media_router_contextual_menu.cc (right):

https://codereview.chromium.org/1582143002/diff/180001/chrome/browser/ui/tool...
chrome/browser/ui/toolbar/media_router_contextual_menu.cc:10: #if
defined(GOOGLE_CHROME_BUILD)
Move conditional includes past other includes. See browser.cc if you want an
example.

https://codereview.chromium.org/1582143002/diff/180001/chrome/browser/ui/tool...
chrome/browser/ui/toolbar/media_router_contextual_menu.cc:64: if (command_id ==
IDC_MEDIA_ROUTER_CLOUD_SERVICES_TOGGLE) {
nit: no {}

[apacible, 2016-01-26 18:24:11]

https://codereview.chromium.org/1582143002/diff/180001/chrome/browser/ui/tool...
File chrome/browser/ui/toolbar/media_router_contextual_menu.cc (right):

https://codereview.chromium.org/1582143002/diff/180001/chrome/browser/ui/tool...
chrome/browser/ui/toolbar/media_router_contextual_menu.cc:10: #if
defined(GOOGLE_CHROME_BUILD)
On 2016/01/26 18:15:01, sky wrote:
> Move conditional includes past other includes. See browser.cc if you want an
> example.

Done.

https://codereview.chromium.org/1582143002/diff/180001/chrome/browser/ui/tool...
chrome/browser/ui/toolbar/media_router_contextual_menu.cc:64: if (command_id ==
IDC_MEDIA_ROUTER_CLOUD_SERVICES_TOGGLE) {
On 2016/01/26 18:15:01, sky wrote:
> nit: no {}

Done.

[imcheng, 2016-01-26 18:35:45]

https://codereview.chromium.org/1582143002/diff/200001/chrome/browser/ui/tool...
File chrome/browser/ui/toolbar/media_router_contextual_menu.cc (right):

https://codereview.chromium.org/1582143002/diff/200001/chrome/browser/ui/tool...
chrome/browser/ui/toolbar/media_router_contextual_menu.cc:64: return
browser_->profile()->IsSyncAllowed();
should we surface this command to users if they haven't gone through the cloud
services first run flow yet?

https://codereview.chromium.org/1582143002/diff/200001/chrome/browser/ui/tool...
chrome/browser/ui/toolbar/media_router_contextual_menu.cc:66: return true;
should this return false for the cloud services toggle in Chromium?

https://codereview.chromium.org/1582143002/diff/200001/chrome/browser/ui/tool...
chrome/browser/ui/toolbar/media_router_contextual_menu.cc:77: int string_id;
nit: we should initialize string_id to 0 or some invalid value and then DCHECK
before using it.

https://codereview.chromium.org/1582143002/diff/200001/chrome/browser/ui/tool...
chrome/browser/ui/toolbar/media_router_contextual_menu.cc:119:
browser_->profile()->GetPrefs()->SetBoolean(
would it be possible to extract browser_->profile()->GetPrefs() into a local
variable?

[apacible, 2016-01-26 19:27:34]

https://codereview.chromium.org/1582143002/diff/200001/chrome/browser/ui/tool...
File chrome/browser/ui/toolbar/media_router_contextual_menu.cc (right):

https://codereview.chromium.org/1582143002/diff/200001/chrome/browser/ui/tool...
chrome/browser/ui/toolbar/media_router_contextual_menu.cc:64: return
browser_->profile()->IsSyncAllowed();
On 2016/01/26 18:35:45, imcheng1 wrote:
> should we surface this command to users if they haven't gone through the cloud
> services first run flow yet?

Yes, we (+pm, design) decided to surface the command even if they haven't
acknowledged the first run flow yet.

https://codereview.chromium.org/1582143002/diff/200001/chrome/browser/ui/tool...
chrome/browser/ui/toolbar/media_router_contextual_menu.cc:66: return true;
On 2016/01/26 18:35:45, imcheng1 wrote:
> should this return false for the cloud services toggle in Chromium?

The cloud services toggle will not appear in Chromium.

https://codereview.chromium.org/1582143002/diff/200001/chrome/browser/ui/tool...
chrome/browser/ui/toolbar/media_router_contextual_menu.cc:77: int string_id;
On 2016/01/26 18:35:45, imcheng1 wrote:
> nit: we should initialize string_id to 0 or some invalid value and then DCHECK
> before using it.

Done.

https://codereview.chromium.org/1582143002/diff/200001/chrome/browser/ui/tool...
chrome/browser/ui/toolbar/media_router_contextual_menu.cc:119:
browser_->profile()->GetPrefs()->SetBoolean(
On 2016/01/26 18:35:45, imcheng1 wrote:
> would it be possible to extract browser_->profile()->GetPrefs() into a local
> variable?

Done.

[imcheng, 2016-01-26 19:41:38]

lgtm

[apacible, 2016-01-26 23:12:23]

The CQ bit was checked by apacible@chromium.org

[apacible, 2016-01-26 23:12:24]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org,
imcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/1582143002/#ps240001
(title: "Fix compiler errors.")

[commit-bot: I haz the power, 2016-01-26 23:15:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1582143002/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1582143002/240001

[commit-bot: I haz the power, 2016-01-26 23:24:30]

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

[commit-bot: I haz the power, 2016-01-26 23:24:31]

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

[apacible, 2016-01-27 00:10:00]

The CQ bit was checked by apacible@chromium.org

[apacible, 2016-01-27 00:10:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
imcheng@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1582143002/#ps260001
(title: "Rebase.")

[commit-bot: I haz the power, 2016-01-27 00:12:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1582143002/260001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1582143002/260001

[commit-bot: I haz the power, 2016-01-27 02:07:44]

Description was changed from

==========
[Media Router] Add enable cloud services menu item to contextual menu.

Part 7.

Add menu item to Media Router contextual menu to toggle the cloud services
preference. It will be checked if cloud services is currently enabled.

This will only appear in Chrome branded builds.

BUG=560457
==========

to

==========
[Media Router] Add enable cloud services menu item to contextual menu.

Part 7.

Add menu item to Media Router contextual menu to toggle the cloud services
preference. It will be checked if cloud services is currently enabled.

This will only appear in Chrome branded builds.

BUG=560457
==========

[commit-bot: I haz the power, 2016-01-27 02:07:44]

Committed patchset #5 (id:260001)

[commit-bot: I haz the power, 2016-01-27 02:08:38]

Description was changed from

==========
[Media Router] Add enable cloud services menu item to contextual menu.

Part 7.

Add menu item to Media Router contextual menu to toggle the cloud services
preference. It will be checked if cloud services is currently enabled.

This will only appear in Chrome branded builds.

BUG=560457
==========

to

==========
[Media Router] Add enable cloud services menu item to contextual menu.

Part 7.

Add menu item to Media Router contextual menu to toggle the cloud services
preference. It will be checked if cloud services is currently enabled.

This will only appear in Chrome branded builds.

BUG=560457

Committed: https://crrev.com/0320fa7c655973aaaa3d350be82e5d0f26b1d517
Cr-Commit-Position: refs/heads/master@{#371689}
==========

[commit-bot: I haz the power, 2016-01-27 02:08:38]

Patchset 5 (id:??) landed as
https://crrev.com/0320fa7c655973aaaa3d350be82e5d0f26b1d517
Cr-Commit-Position: refs/heads/master@{#371689}