[Media Router] Update Media Router flags post-launch.

[Media Router] Update Media Router flags post-launch.

- Remove --media-router flag as this functionality has shipped
  (crbug.com/651255)

- Remove lots of GOOGLE_CHROME_BUILD #ifdefs around cloud services
  (crbug.com/646171)

- Adds a --load-media-router-component-extension flag to control loading of the
  Media Router component.  This is default on in Chrome and default off in
  Chromium.  (crbug.com/646170)

BUG=646170, 646171, 651255
Committed: https://crrev.com/ac0030f5d07e66752b93695bc373bcb55ab26602
Cr-Commit-Position: refs/heads/master@{#428493}

[mark a. foltz, 2016-10-25 19:03:24]

mfoltz@chromium.org changed reviewers:
+ anthonyvd@chromium.org, apacible@chromium.org, msw@chromium.org,
rdevlin.cronin@chromium.org

[mark a. foltz, 2016-10-25 19:03:25]

PTAL, thanks

apacible@: everything
anthonyvd@:      profile.cc
grt@:            command_ids.h
msw@:            chrome/browser/ui/toolbar/*
rdevlin.cronin@: extensions/*, chrome/browser/extensions/*

[mark a. foltz, 2016-10-25 19:03:56]

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

[commit-bot: I haz the power, 2016-10-25 19:04:23]

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-10-25 19:18:16]

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

[commit-bot: I haz the power, 2016-10-25 19:18:17]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_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_...)

[Devlin, 2016-10-25 20:38:02]

https://codereview.chromium.org/2451573003/diff/1/extensions/common/feature_s...
File extensions/common/feature_switch.cc (right):

https://codereview.chromium.org/2451573003/diff/1/extensions/common/feature_s...
extensions/common/feature_switch.cc:55: kLoadMediaRouterComponentExtensionFlag,
Do we want chrome users to be able to disable this functionality?

[anthonyvd, 2016-10-25 20:44:22]

c/b/profiles/profile.cc lgtm

[mark a. foltz, 2016-10-25 21:06:10]

On 2016/10/25 at 20:38:02, rdevlin.cronin wrote:
>
https://codereview.chromium.org/2451573003/diff/1/extensions/common/feature_s...
> File extensions/common/feature_switch.cc (right):
> 
>
https://codereview.chromium.org/2451573003/diff/1/extensions/common/feature_s...
> extensions/common/feature_switch.cc:55:
kLoadMediaRouterComponentExtensionFlag,
> Do we want chrome users to be able to disable this functionality?

It's useful for development, and would allow us to remove permissions from the
existing component to manage multiple installs.

[apacible, 2016-10-25 22:01:28]

wrt ungating cloud services - should we check if
kLoadMediaRouterComponentExtension is true if it's a chromium build? Otherwise,
if the user is signed in and opens the dialog (with no devices) but is expecting
cloud sinks, they may be confused.

I don't know actual numbers of chromium usage, but I remember in uni we had
chromium preinstalled on our linux desktops and needed special admin permissions
to install other software (i.e. Chrome). I would probably have run into the
aforementioned case if I didn't know about the
kLoadMediaRouterComponentExtension flag. :)

https://codereview.chromium.org/2451573003/diff/1/chrome/app/media_router_str...
File chrome/app/media_router_strings.grdp (right):

https://codereview.chromium.org/2451573003/diff/1/chrome/app/media_router_str...
chrome/app/media_router_strings.grdp:79: Enable casting to cloud-based services
like Google Hangouts.
nit: indentation.

https://codereview.chromium.org/2451573003/diff/1/chrome/app/media_router_str...
chrome/app/media_router_strings.grdp:79: Enable casting to cloud-based services
like Google Hangouts.
nit: In Chromium, do we still want to mention "Google" specifically?

https://codereview.chromium.org/2451573003/diff/1/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/2451573003/diff/1/chrome/browser/about_flags....
chrome/browser/about_flags.cc:2073: // NOTE: Adding new command-line switches
requires adding corresponding
nit: indentation.

https://codereview.chromium.org/2451573003/diff/1/chrome/browser/media/router...
File chrome/browser/media/router/media_router_feature.cc (right):

https://codereview.chromium.org/2451573003/diff/1/chrome/browser/media/router...
chrome/browser/media/router/media_router_feature.cc:43: return true;
L43 and L45 both return true -- can we consolidate these?

[Devlin, 2016-10-25 22:04:43]

On 2016/10/25 21:06:10, mark a. foltz wrote:
> On 2016/10/25 at 20:38:02, rdevlin.cronin wrote:
> >
>
https://codereview.chromium.org/2451573003/diff/1/extensions/common/feature_s...
> > File extensions/common/feature_switch.cc (right):
> > 
> >
>
https://codereview.chromium.org/2451573003/diff/1/extensions/common/feature_s...
> > extensions/common/feature_switch.cc:55:
> kLoadMediaRouterComponentExtensionFlag,
> > Do we want chrome users to be able to disable this functionality?
> 
> It's useful for development, and would allow us to remove permissions from the
> existing component to manage multiple installs.

Will anything else in chrome break if the user disables it?  Most components
expect MR to be present, so if it isn't, are we going to crash anywhere?  (I
don't know - but seems like something that could happen.)

[mark a. foltz, 2016-10-25 22:44:33]

On 2016/10/25 at 22:04:43, rdevlin.cronin wrote:
> On 2016/10/25 21:06:10, mark a. foltz wrote:
> > On 2016/10/25 at 20:38:02, rdevlin.cronin wrote:
> > >
> >
https://codereview.chromium.org/2451573003/diff/1/extensions/common/feature_s...
> > > File extensions/common/feature_switch.cc (right):
> > > 
> > >
> >
https://codereview.chromium.org/2451573003/diff/1/extensions/common/feature_s...
> > > extensions/common/feature_switch.cc:55:
> > kLoadMediaRouterComponentExtensionFlag,
> > > Do we want chrome users to be able to disable this functionality?
> > 
> > It's useful for development, and would allow us to remove permissions from
the
> > existing component to manage multiple installs.
> 
> Will anything else in chrome break if the user disables it?  Most components
expect MR to be present, so if it isn't, are we going to crash anywhere?  (I
don't know - but seems like something that could happen.)

It will revert to the default behavior that's been the case for a while in
Chromium - no component is loaded.  The Media Router service on desktop won't be
able to bind the Mojo stub to talk to the component.  The MR has code to handle
this and will just discard incoming calls.  The user won't see any devices in
their dialog, but nothing should crash.

[Devlin, 2016-10-25 22:54:07]

extensions lgtm

[mark a. foltz, 2016-10-25 22:56:22]

On 2016/10/25 at 22:01:28, apacible wrote:
> wrt ungating cloud services - should we check if
kLoadMediaRouterComponentExtension is true if it's a chromium build? Otherwise,
if the user is signed in and opens the dialog (with no devices) but is expecting
cloud sinks, they may be confused.
> 
> I don't know actual numbers of chromium usage, but I remember in uni we had
chromium preinstalled on our linux desktops and needed special admin permissions
to install other software (i.e. Chrome). I would probably have run into the
aforementioned case if I didn't know about the
kLoadMediaRouterComponentExtension flag. :)


Good point, there are pros and cons to this.  I'll raise the issue in the
associated bug.


> 
>
https://codereview.chromium.org/2451573003/diff/1/chrome/app/media_router_str...
> File chrome/app/media_router_strings.grdp (right):
> 
>
https://codereview.chromium.org/2451573003/diff/1/chrome/app/media_router_str...
> chrome/app/media_router_strings.grdp:79: Enable casting to cloud-based
services like Google Hangouts.
> nit: indentation.
> 
>
https://codereview.chromium.org/2451573003/diff/1/chrome/app/media_router_str...
> chrome/app/media_router_strings.grdp:79: Enable casting to cloud-based
services like Google Hangouts.
> nit: In Chromium, do we still want to mention "Google" specifically?
> 
>
https://codereview.chromium.org/2451573003/diff/1/chrome/browser/about_flags.cc
> File chrome/browser/about_flags.cc (right):
> 
>
https://codereview.chromium.org/2451573003/diff/1/chrome/browser/about_flags....
> chrome/browser/about_flags.cc:2073: // NOTE: Adding new command-line switches
requires adding corresponding
> nit: indentation.
> 
>
https://codereview.chromium.org/2451573003/diff/1/chrome/browser/media/router...
> File chrome/browser/media/router/media_router_feature.cc (right):
> 
>
https://codereview.chromium.org/2451573003/diff/1/chrome/browser/media/router...
> chrome/browser/media/router/media_router_feature.cc:43: return true;
> L43 and L45 both return true -- can we consolidate these?

[mark a. foltz, 2016-10-25 22:56:36]

https://codereview.chromium.org/2451573003/diff/1/chrome/app/media_router_str...
File chrome/app/media_router_strings.grdp (right):

https://codereview.chromium.org/2451573003/diff/1/chrome/app/media_router_str...
chrome/app/media_router_strings.grdp:79: Enable casting to cloud-based services
like Google Hangouts.
On 2016/10/25 at 22:01:28, apacible wrote:
> nit: indentation.

Done.

Re: Google Hangouts

Hangouts is a Google product so I think we should keep it as-is...

https://codereview.chromium.org/2451573003/diff/1/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/2451573003/diff/1/chrome/browser/about_flags....
chrome/browser/about_flags.cc:2073: // NOTE: Adding new command-line switches
requires adding corresponding
On 2016/10/25 at 22:01:28, apacible wrote:
> nit: indentation.

Done.

https://codereview.chromium.org/2451573003/diff/1/chrome/browser/media/router...
File chrome/browser/media/router/media_router_feature.cc (right):

https://codereview.chromium.org/2451573003/diff/1/chrome/browser/media/router...
chrome/browser/media/router/media_router_feature.cc:43: return true;
On 2016/10/25 at 22:01:28, apacible wrote:
> L43 and L45 both return true -- can we consolidate these?

Done.

[mark a. foltz, 2016-10-25 23:48:25]

mfoltz@chromium.org changed reviewers:
+ grt@chromium.org, jdufault@chromium.org, sky@chromium.org

[mark a. foltz, 2016-10-25 23:48:26]

+grt for chrome_command_ids.h (for real)
+sky for chrome/browser/ui/ash

[sky, 2016-10-25 23:54:48]

Nuking chrome/browser/ui/ash/system_tray_tray_cast_browsertest_chromeos.cc LGTM

[mark a. foltz, 2016-10-26 04:22:08]

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

[commit-bot: I haz the power, 2016-10-26 04:22:19]

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-10-26 04:26:33]

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

[commit-bot: I haz the power, 2016-10-26 04:26:34]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[grt (UTC plus 2), 2016-10-26 08:04:28]

chrome/app/chrome_command_ids.h lgtm

[apacible, 2016-10-26 15:39:14]

wrt 646171 - are you planning on adding the "no component extension" issue
messaging in this patch or in a follow up patch?

https://codereview.chromium.org/2451573003/diff/1/chrome/app/media_router_str...
File chrome/app/media_router_strings.grdp (right):

https://codereview.chromium.org/2451573003/diff/1/chrome/app/media_router_str...
chrome/app/media_router_strings.grdp:79: Enable casting to cloud-based services
like Google Hangouts.
On 2016/10/25 22:56:36, mark a. foltz wrote:
> On 2016/10/25 at 22:01:28, apacible wrote:
> > nit: indentation.
> 
> Done.
> 
> Re: Google Hangouts
> 
> Hangouts is a Google product so I think we should keep it as-is...

We typically want to avoid mentioning anything "Google" branded for Chromium.
Can you check with skonig for guidance? I'm not sure how strict we should be in
this case.

[Stephen, 2016-10-27 17:02:46]

On 2016/10/26 15:39:14, apacible wrote:
> wrt 646171 - are you planning on adding the "no component extension" issue
> messaging in this patch or in a follow up patch?
> 
>
https://codereview.chromium.org/2451573003/diff/1/chrome/app/media_router_str...
> File chrome/app/media_router_strings.grdp (right):
> 
>
https://codereview.chromium.org/2451573003/diff/1/chrome/app/media_router_str...
> chrome/app/media_router_strings.grdp:79: Enable casting to cloud-based
services
> like Google Hangouts.
> On 2016/10/25 22:56:36, mark a. foltz wrote:
> > On 2016/10/25 at 22:01:28, apacible wrote:
> > > nit: indentation.
> > 
> > Done.
> > 
> > Re: Google Hangouts
> > 
> > Hangouts is a Google product so I think we should keep it as-is...
> 
> We typically want to avoid mentioning anything "Google" branded for Chromium.
> Can you check with skonig for guidance? I'm not sure how strict we should be
in
> this case.

In this case I think it's okay, since the Chromium user has to explicitly toggle
this flag in order to enable this, so they presumably know they are getting a
Google product/service with it.

[msw, 2016-10-27 18:14:32]

c/b/ui/lgtm with nits

https://codereview.chromium.org/2451573003/diff/60001/chrome/browser/ui/ash/s...
File chrome/browser/ui/ash/system_tray_tray_cast_browsertest_chromeos.cc (left):

https://codereview.chromium.org/2451573003/diff/60001/chrome/browser/ui/ash/s...
chrome/browser/ui/ash/system_tray_tray_cast_browsertest_chromeos.cc:5: #include
"ash/common/system/cast/tray_cast.h"
Should we wait to remove these tests for when we remove tray_cast, etc?

https://codereview.chromium.org/2451573003/diff/60001/chrome/browser/ui/toolb...
File chrome/browser/ui/toolbar/media_router_contextual_menu_unittest.cc (right):

https://codereview.chromium.org/2451573003/diff/60001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/media_router_contextual_menu_unittest.cc:71: // An
additional menu item to toggle cloud services settings.
nit: just init |expected_number_items| to 8 instead of 7... though a comment
listing all the items might be nice.

https://codereview.chromium.org/2451573003/diff/60001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/media_router_contextual_menu_unittest.cc:79: bool
expected_visibility = true;
nit: combine |expected_visibility| decl and init. (or inline in EXPECT_EQ)

https://codereview.chromium.org/2451573003/diff/60001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/media_router/media_router_test.cc (right):

https://codereview.chromium.org/2451573003/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/media_router/media_router_test.cc:11:
MediaRouterTest::~MediaRouterTest() {
optional nit: {} formatting

https://codereview.chromium.org/2451573003/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/media_router/media_router_test.cc:15: return new
DialogTestBrowserWindow;
aside: I wonder if we still need MediaRouterTest to be a subclass of
BrowserWithTestWindowTest for this, or if some simplification can be made.

[jdufault, 2016-10-27 18:38:08]

https://codereview.chromium.org/2451573003/diff/60001/chrome/browser/ui/ash/s...
File chrome/browser/ui/ash/system_tray_tray_cast_browsertest_chromeos.cc (left):

https://codereview.chromium.org/2451573003/diff/60001/chrome/browser/ui/ash/s...
chrome/browser/ui/ash/system_tray_tray_cast_browsertest_chromeos.cc:5: #include
"ash/common/system/cast/tray_cast.h"
On 2016/10/27 18:14:31, msw wrote:
> Should we wait to remove these tests for when we remove tray_cast, etc?

tray_cast isn't going away, but the code this tests is.

https://codereview.chromium.org/2424183002/ removes it.

[mark a. foltz, 2016-10-27 20:16:50]

https://codereview.chromium.org/2451573003/diff/60001/chrome/browser/ui/ash/s...
File chrome/browser/ui/ash/system_tray_tray_cast_browsertest_chromeos.cc (left):

https://codereview.chromium.org/2451573003/diff/60001/chrome/browser/ui/ash/s...
chrome/browser/ui/ash/system_tray_tray_cast_browsertest_chromeos.cc:5: #include
"ash/common/system/cast/tray_cast.h"
On 2016/10/27 at 18:38:08, jdufault wrote:
> On 2016/10/27 18:14:31, msw wrote:
> > Should we wait to remove these tests for when we remove tray_cast, etc?
> 
> tray_cast isn't going away, but the code this tests is.
> 
> https://codereview.chromium.org/2424183002/ removes it.

Can you add imcheng@ as a reviewer to the change if you are adding any new
behavior that uses the Media Router?

https://codereview.chromium.org/2451573003/diff/60001/chrome/browser/ui/toolb...
File chrome/browser/ui/toolbar/media_router_contextual_menu_unittest.cc (right):

https://codereview.chromium.org/2451573003/diff/60001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/media_router_contextual_menu_unittest.cc:71: // An
additional menu item to toggle cloud services settings.
On 2016/10/27 at 18:14:31, msw wrote:
> nit: just init |expected_number_items| to 8 instead of 7... though a comment
listing all the items might be nice.

Done.

https://codereview.chromium.org/2451573003/diff/60001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/media_router_contextual_menu_unittest.cc:79: bool
expected_visibility = true;
On 2016/10/27 at 18:14:31, msw wrote:
> nit: combine |expected_visibility| decl and init. (or inline in EXPECT_EQ)

Done.

https://codereview.chromium.org/2451573003/diff/60001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/media_router/media_router_test.cc (right):

https://codereview.chromium.org/2451573003/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/media_router/media_router_test.cc:11:
MediaRouterTest::~MediaRouterTest() {
On 2016/10/27 at 18:14:31, msw wrote:
> optional nit: {} formatting

Done.

https://codereview.chromium.org/2451573003/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/media_router/media_router_test.cc:15: return new
DialogTestBrowserWindow;
On 2016/10/27 at 18:14:31, msw wrote:
> aside: I wonder if we still need MediaRouterTest to be a subclass of
BrowserWithTestWindowTest for this, or if some simplification can be made.

It looks like BrowserWithTestWindowTest calls this method and we're
intentionally overriding it from the default.

[mark a. foltz, 2016-10-27 20:17:33]

mfoltz@chromium.org changed reviewers:
+ isherman@chromium.org

[mark a. foltz, 2016-10-27 20:17:34]

+isherman for histograms.xml

[mark a. foltz, 2016-10-27 20:26:25]

On 2016/10/26 at 15:39:14, apacible wrote:
> wrt 646171 - are you planning on adding the "no component extension" issue
messaging in this patch or in a follow up patch?

I'll do that in a separate patch, I want to make sure there is a specific error
propagated to the UI when there's no component.

Filed http://crbug.com/660139 to track this work.

[mark a. foltz, 2016-10-27 21:06:20]

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

[commit-bot: I haz the power, 2016-10-27 21:07:37]

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

[Ilya Sherman, 2016-10-27 21:53:51]

histograms.xml lgtm

[japacible, 2016-10-27 22:08:02]

On 2016/10/27 17:02:46, Stephen wrote:
> On 2016/10/26 15:39:14, apacible wrote:
> > wrt 646171 - are you planning on adding the "no component extension" issue
> > messaging in this patch or in a follow up patch?
Tracking at crbug/660139.

> > 
> >
>
https://codereview.chromium.org/2451573003/diff/1/chrome/app/media_router_str...
> > File chrome/app/media_router_strings.grdp (right):
> > 
> >
>
https://codereview.chromium.org/2451573003/diff/1/chrome/app/media_router_str...
> > chrome/app/media_router_strings.grdp:79: Enable casting to cloud-based
> services
> > like Google Hangouts.
> > On 2016/10/25 22:56:36, mark a. foltz wrote:
> > > On 2016/10/25 at 22:01:28, apacible wrote:
> > > > nit: indentation.
> > > 
> > > Done.
> > > 
> > > Re: Google Hangouts
> > > 
> > > Hangouts is a Google product so I think we should keep it as-is...
> > 
> > We typically want to avoid mentioning anything "Google" branded for
Chromium.
> > Can you check with skonig for guidance? I'm not sure how strict we should be
> in
> > this case.
> 
> In this case I think it's okay, since the Chromium user has to explicitly
toggle
> this flag in order to enable this, so they presumably know they are getting a
> Google product/service with it.

SGTM.

[commit-bot: I haz the power, 2016-10-27 22:49:23]

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

[commit-bot: I haz the power, 2016-10-27 22:49:25]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[apacible, 2016-10-28 02:42:37]

LGTM

https://codereview.chromium.org/2451573003/diff/80001/chrome/browser/media/ro...
File chrome/browser/media/router/media_router_feature.cc (right):

https://codereview.chromium.org/2451573003/diff/80001/chrome/browser/media/ro...
chrome/browser/media/router/media_router_feature.cc:43: #else  //
!(defined(OS_ANDROID) || defined(ENABLE_EXTENSIONS))
Could we do:

#if defined(ENABLE_MEDIA_ROUTER) &&
    (defined(OS_ANDROID) || defined(ENABLE_EXTENSIONS))
  if (... prefs checks here..)
    return allowed;
  return true;
#endif
  return false;

https://codereview.chromium.org/2451573003/diff/80001/chrome/browser/ui/toolb...
File chrome/browser/ui/toolbar/media_router_contextual_menu_unittest.cc (right):

https://codereview.chromium.org/2451573003/diff/80001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/media_router_contextual_menu_unittest.cc:69: // Enable
cloud services
nit: also specify this is a check item.

[mark a. foltz, 2016-10-28 19:54:44]

https://codereview.chromium.org/2451573003/diff/80001/chrome/browser/media/ro...
File chrome/browser/media/router/media_router_feature.cc (right):

https://codereview.chromium.org/2451573003/diff/80001/chrome/browser/media/ro...
chrome/browser/media/router/media_router_feature.cc:43: #else  //
!(defined(OS_ANDROID) || defined(ENABLE_EXTENSIONS))
On 2016/10/28 at 02:42:37, apacible wrote:
> Could we do:
> 
> #if defined(ENABLE_MEDIA_ROUTER) &&
>     (defined(OS_ANDROID) || defined(ENABLE_EXTENSIONS))
>   if (... prefs checks here..)
>     return allowed;
>   return true;
> #endif
>   return false;

I tried it, but it results in line wrapping the #ifdefs and was less readable
IMO.

https://codereview.chromium.org/2451573003/diff/80001/chrome/browser/ui/toolb...
File chrome/browser/ui/toolbar/media_router_contextual_menu_unittest.cc (right):

https://codereview.chromium.org/2451573003/diff/80001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/media_router_contextual_menu_unittest.cc:69: // Enable
cloud services
On 2016/10/28 at 02:42:37, apacible wrote:
> nit: also specify this is a check item.

Done.

[mark a. foltz, 2016-10-28 20:08:16]

The CQ bit was checked by mfoltz@chromium.org

[mark a. foltz, 2016-10-28 20:08:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
anthonyvd@chromium.org, rdevlin.cronin@chromium.org, sky@chromium.org,
msw@chromium.org, grt@chromium.org, apacible@chromium.org, isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/2451573003/#ps100001
(title: "Rebase & respond to apacible@ comments.")

[commit-bot: I haz the power, 2016-10-28 20:08: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, 2016-10-28 21:17:22]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-10-28 21:20:38]

Description was changed from

==========
[Media Router] Update Media Router flags post-launch.

- Remove --media-router flag as this functionality has shipped
  (crbug.com/651255)

- Remove lots of GOOGLE_CHROME_BUILD #ifdefs around cloud services
  (crbug.com/646171)

- Adds a --load-media-router-component-extension flag to control loading of the
  Media Router component.  This is default on in Chrome and default off in
  Chromium.  (crbug.com/646170)

BUG=646170,646171,651255
==========

to

==========
[Media Router] Update Media Router flags post-launch.

- Remove --media-router flag as this functionality has shipped
  (crbug.com/651255)

- Remove lots of GOOGLE_CHROME_BUILD #ifdefs around cloud services
  (crbug.com/646171)

- Adds a --load-media-router-component-extension flag to control loading of the
  Media Router component.  This is default on in Chrome and default off in
  Chromium.  (crbug.com/646170)

BUG=646170,646171,651255

Committed: https://crrev.com/ac0030f5d07e66752b93695bc373bcb55ab26602
Cr-Commit-Position: refs/heads/master@{#428493}
==========

[commit-bot: I haz the power, 2016-10-28 21:20:40]

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