[Media Router] Add cloud services preferences.

[Media Router] Add cloud services preferences.

Part 6.

This change adds two syncable preferences related to using cloud services with Media Router.

kMediaRouterCloudServicesPrefSet tracks whether or not the user has explicitly acknowledged the first run flow with the cloud services preference present. This is important to keep track of since there are users who have acknowledged the first run flow before the work related to the cloud services lands (currently in progress). This will be used to surface a different prompt to ask if users want to enable cloud services.

kMediaRouterEnableCloudServices tracks whether the user has enabled cloud services.

BUG=560457
Committed: https://crrev.com/40dd5a18974d28dfe7ac1a385a4f43bbb95cf3c6
Cr-Commit-Position: refs/heads/master@{#371456}

[apacible, 2016-01-14 01:58:11]

Description was changed from

==========
[Media Router] Add enable cloud services preference.

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

to

==========
[Media Router] Add enable cloud services preference.

Part 5 of several.

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

[apacible, 2016-01-14 02:53:14]

Patchset #1 (id:1) has been deleted

[apacible, 2016-01-14 02:54:56]

Patchset #1 (id:20001) has been deleted

[apacible, 2016-01-20 17:48:19]

Patchset #1 (id:40001) has been deleted

[apacible, 2016-01-20 18:07:56]

Patchset #1 (id:60001) has been deleted

[apacible, 2016-01-20 18:08:55]

Patchset #1 (id:80001) has been deleted

[apacible, 2016-01-20 18:12:05]

Patchset #1 (id:100001) has been deleted

[apacible, 2016-01-20 18:24:32]

Description was changed from

==========
[Media Router] Add enable cloud services preference.

Part 5 of several.

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

to

==========
[Media Router] Add enable cloud services preference.

Part 5 of several.

kMediaRouterCloudServicesPrefSet and kMediaRouterEnableCloudServices are both
new syncable preferences related to using cloud services with Media Router.

kMediaRouterCloudServicesPrefSet tracks whether or not the user has explicitly
acknowledged the first run flow with the cloud services preference present.
kMediaRouterEnableCloudServices tracks whether the user has enabled cloud
services.

kMediaRouterCloudServicesPrefSet is important to keep track of since there are
users who have acknowledged the first run flow before the work related to the
cloud services lands (currently in progress). This will be used to surface a
different prompt to ask if users want to enable cloud services.

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

[apacible, 2016-01-20 18:24:46]

Patchset #1 (id:120001) has been deleted

[apacible, 2016-01-20 18:27:47]

Description was changed from

==========
[Media Router] Add enable cloud services preference.

Part 5 of several.

kMediaRouterCloudServicesPrefSet and kMediaRouterEnableCloudServices are both
new syncable preferences related to using cloud services with Media Router.

kMediaRouterCloudServicesPrefSet tracks whether or not the user has explicitly
acknowledged the first run flow with the cloud services preference present.
kMediaRouterEnableCloudServices tracks whether the user has enabled cloud
services.

kMediaRouterCloudServicesPrefSet is important to keep track of since there are
users who have acknowledged the first run flow before the work related to the
cloud services lands (currently in progress). This will be used to surface a
different prompt to ask if users want to enable cloud services.

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

to

==========
[Media Router] Add cloud services preferences.

Part 5 of several.

This change adds two syncable preferences related to using cloud services with
Media Router.

kMediaRouterCloudServicesPrefSet tracks whether or not the user has explicitly
acknowledged the first run flow with the cloud services preference present. This
is important to keep track of since there are users who have acknowledged the
first run flow before the work related to the cloud services lands (currently in
progress). This will be used to surface a different prompt to ask if users want
to enable cloud services.

kMediaRouterEnableCloudServices tracks whether the user has enabled cloud
services.

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

[apacible, 2016-01-20 18:27:47]

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

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

Patchset #1 (id:140001) has been deleted

[apacible, 2016-01-20 18:28:46]

PTAL, thanks!

+imcheng for everything.
+anthonyvd for chrome/browser/profiles/profile.cc.

[anthonyvd, 2016-01-21 15:52:33]

c/b/profiles/profile.cc lgtm % a little question.

https://codereview.chromium.org/1582943003/diff/180001/chrome/browser/profile...
File chrome/browser/profiles/profile.cc (right):

https://codereview.chromium.org/1582943003/diff/180001/chrome/browser/profile...
chrome/browser/profiles/profile.cc:163: registry->RegisterBooleanPref(
Those seem to only be used when GOOGLE_CHROME_BUILD is defined. Shouldn't they
also be only registered in that case?

[apacible, 2016-01-21 16:36:46]

https://codereview.chromium.org/1582943003/diff/180001/chrome/browser/profile...
File chrome/browser/profiles/profile.cc (right):

https://codereview.chromium.org/1582943003/diff/180001/chrome/browser/profile...
chrome/browser/profiles/profile.cc:163: registry->RegisterBooleanPref(
On 2016/01/21 15:52:33, anthonyvd wrote:
> Those seem to only be used when GOOGLE_CHROME_BUILD is defined. Shouldn't they
> also be only registered in that case?

Fixed.

[imcheng, 2016-01-25 19:21:06]

https://codereview.chromium.org/1582943003/diff/200001/chrome/browser/resourc...
File chrome/browser/resources/media_router/media_router.js (right):

https://codereview.chromium.org/1582943003/diff/200001/chrome/browser/resourc...
chrome/browser/resources/media_router/media_router.js:91: /** @type
{{optedIntoCloudServices: boolean}} */
move this line above "var detail = ..."

https://codereview.chromium.org/1582943003/diff/200001/chrome/browser/resourc...
File chrome/browser/resources/media_router/media_router_ui_interface.js (right):

https://codereview.chromium.org/1582943003/diff/200001/chrome/browser/resourc...
chrome/browser/resources/media_router/media_router_ui_interface.js:58: *        
 showFirstRunFlowCloudPref: boolean}} data
Should this be {boolean|undefined}? Another way is to set it to false in
Chromium builds in MediaRouterWebUIMessageHandler. Also see comment below.

https://codereview.chromium.org/1582943003/diff/200001/chrome/browser/resourc...
chrome/browser/resources/media_router/media_router_ui_interface.js:76:
data['showFirstRunFlowCloudPref'];
should this be "data['showFirstRunFlowCloudPref'] || false" so it's not
undefined in Chromium builds?

https://codereview.chromium.org/1582943003/diff/200001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc
(right):

https://codereview.chromium.org/1582943003/diff/200001/chrome/browser/ui/webu...
chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc:360:
bool show_cloud_pref = profile->IsSyncAllowed() &&
Why is IsSyncAllowed() checked here? Could you add some comments?

https://codereview.chromium.org/1582943003/diff/200001/chrome/browser/ui/webu...
chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc:360:
bool show_cloud_pref = profile->IsSyncAllowed() &&
can this be replaced with whether the kMediaRouterEnableCloudServices preference
is already set?

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

https://codereview.chromium.org/1582943003/diff/200001/chrome/browser/resourc...
File chrome/browser/resources/media_router/media_router.js (right):

https://codereview.chromium.org/1582943003/diff/200001/chrome/browser/resourc...
chrome/browser/resources/media_router/media_router.js:91: /** @type
{{optedIntoCloudServices: boolean}} */
On 2016/01/25 19:21:06, imcheng1 wrote:
> move this line above "var detail = ..."

Done.

https://codereview.chromium.org/1582943003/diff/200001/chrome/browser/resourc...
File chrome/browser/resources/media_router/media_router_ui_interface.js (right):

https://codereview.chromium.org/1582943003/diff/200001/chrome/browser/resourc...
chrome/browser/resources/media_router/media_router_ui_interface.js:58: *        
 showFirstRunFlowCloudPref: boolean}} data
On 2016/01/25 19:21:06, imcheng1 wrote:
> Should this be {boolean|undefined}? Another way is to set it to false in
> Chromium builds in MediaRouterWebUIMessageHandler. Also see comment below.

Done. Set to false in Chromium builds.

https://codereview.chromium.org/1582943003/diff/200001/chrome/browser/resourc...
chrome/browser/resources/media_router/media_router_ui_interface.js:76:
data['showFirstRunFlowCloudPref'];
On 2016/01/25 19:21:06, imcheng1 wrote:
> should this be "data['showFirstRunFlowCloudPref'] || false" so it's not
> undefined in Chromium builds?

See above comment.

https://codereview.chromium.org/1582943003/diff/200001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc
(right):

https://codereview.chromium.org/1582943003/diff/200001/chrome/browser/ui/webu...
chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc:360:
bool show_cloud_pref = profile->IsSyncAllowed() &&
On 2016/01/25 19:21:06, imcheng1 wrote:
> Why is IsSyncAllowed() checked here? Could you add some comments?

Added comments.

Per discussion with product, we only want to show the acknowledgement if sync is
enabled. When the user enables sync on Chrome, that action is treated as opting
into Google/proprietary services, including cloud services (via discussion
w/privacy). In that case, we'll enable cloud services for that user and mark the
pref as set.

https://codereview.chromium.org/1582943003/diff/200001/chrome/browser/ui/webu...
chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc:360:
bool show_cloud_pref = profile->IsSyncAllowed() &&
On 2016/01/25 19:21:06, imcheng1 wrote:
> can this be replaced with whether the kMediaRouterEnableCloudServices
preference
> is already set?

I'm not sure I understand this comment. kMediaRouterEnableCloudServices
preference defaults to false. We have a second preference to keep track of
whether or not it's already been set, used in line 362.

[imcheng, 2016-01-26 01:27:48]

lgtm

[apacible, 2016-01-26 01:28:43]

Description was changed from

==========
[Media Router] Add cloud services preferences.

Part 5 of several.

This change adds two syncable preferences related to using cloud services with
Media Router.

kMediaRouterCloudServicesPrefSet tracks whether or not the user has explicitly
acknowledged the first run flow with the cloud services preference present. This
is important to keep track of since there are users who have acknowledged the
first run flow before the work related to the cloud services lands (currently in
progress). This will be used to surface a different prompt to ask if users want
to enable cloud services.

kMediaRouterEnableCloudServices tracks whether the user has enabled cloud
services.

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

to

==========
[Media Router] Add cloud services preferences.

Part 6.

This change adds two syncable preferences related to using cloud services with
Media Router.

kMediaRouterCloudServicesPrefSet tracks whether or not the user has explicitly
acknowledged the first run flow with the cloud services preference present. This
is important to keep track of since there are users who have acknowledged the
first run flow before the work related to the cloud services lands (currently in
progress). This will be used to surface a different prompt to ask if users want
to enable cloud services.

kMediaRouterEnableCloudServices tracks whether the user has enabled cloud
services.

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

[apacible, 2016-01-26 02:54:35]

The CQ bit was checked by apacible@chromium.org

[apacible, 2016-01-26 02:54:36]

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

[commit-bot: I haz the power, 2016-01-26 02:55:13]

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

[commit-bot: I haz the power, 2016-01-26 04:41:01]

Description was changed from

==========
[Media Router] Add cloud services preferences.

Part 6.

This change adds two syncable preferences related to using cloud services with
Media Router.

kMediaRouterCloudServicesPrefSet tracks whether or not the user has explicitly
acknowledged the first run flow with the cloud services preference present. This
is important to keep track of since there are users who have acknowledged the
first run flow before the work related to the cloud services lands (currently in
progress). This will be used to surface a different prompt to ask if users want
to enable cloud services.

kMediaRouterEnableCloudServices tracks whether the user has enabled cloud
services.

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

to

==========
[Media Router] Add cloud services preferences.

Part 6.

This change adds two syncable preferences related to using cloud services with
Media Router.

kMediaRouterCloudServicesPrefSet tracks whether or not the user has explicitly
acknowledged the first run flow with the cloud services preference present. This
is important to keep track of since there are users who have acknowledged the
first run flow before the work related to the cloud services lands (currently in
progress). This will be used to surface a different prompt to ask if users want
to enable cloud services.

kMediaRouterEnableCloudServices tracks whether the user has enabled cloud
services.

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

[commit-bot: I haz the power, 2016-01-26 04:41:01]

Committed patchset #6 (id:260001)

[commit-bot: I haz the power, 2016-01-26 04:42:29]

Description was changed from

==========
[Media Router] Add cloud services preferences.

Part 6.

This change adds two syncable preferences related to using cloud services with
Media Router.

kMediaRouterCloudServicesPrefSet tracks whether or not the user has explicitly
acknowledged the first run flow with the cloud services preference present. This
is important to keep track of since there are users who have acknowledged the
first run flow before the work related to the cloud services lands (currently in
progress). This will be used to surface a different prompt to ask if users want
to enable cloud services.

kMediaRouterEnableCloudServices tracks whether the user has enabled cloud
services.

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

to

==========
[Media Router] Add cloud services preferences.

Part 6.

This change adds two syncable preferences related to using cloud services with
Media Router.

kMediaRouterCloudServicesPrefSet tracks whether or not the user has explicitly
acknowledged the first run flow with the cloud services preference present. This
is important to keep track of since there are users who have acknowledged the
first run flow before the work related to the cloud services lands (currently in
progress). This will be used to surface a different prompt to ask if users want
to enable cloud services.

kMediaRouterEnableCloudServices tracks whether the user has enabled cloud
services.

BUG=560457

Committed: https://crrev.com/40dd5a18974d28dfe7ac1a385a4f43bbb95cf3c6
Cr-Commit-Position: refs/heads/master@{#371456}
==========

[commit-bot: I haz the power, 2016-01-26 04:42:29]

Patchset 6 (id:??) landed as
https://crrev.com/40dd5a18974d28dfe7ac1a385a4f43bbb95cf3c6
Cr-Commit-Position: refs/heads/master@{#371456}