[Media Router] Enable MR by default now that it is at 100% in stable.

[Media Router] Enable MR by default now that it is at 100% in stable.

BUG=633269
Committed: https://crrev.com/72217db0c2b09929ba393414493e0d0dd0d6e823
Cr-Commit-Position: refs/heads/master@{#417626}

[imcheng, 2016-09-02 08:06:27]

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

[commit-bot: I haz the power, 2016-09-02 08:06:47]

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

[imcheng, 2016-09-02 08:07:13]

imcheng@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[imcheng, 2016-09-02 08:07:14]

Devlin: PTAL

[commit-bot: I haz the power, 2016-09-02 08:46:39]

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

[commit-bot: I haz the power, 2016-09-02 08:46:40]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Devlin, 2016-09-02 14:23:20]

Looks like there are some failing tests, some of which seem to be failing with
broken assumptions about the extension_action_redesign().  I'll see if I can't
put up a CL to pull more of those out.

[imcheng, 2016-09-06 16:31:07]

Would it be simpler to just remove those test cases? It looks like all of them
are testing with the EAR switch disabled, but we've already turned EAR on by
default for quite some time now (I assume the FeatureSwitch would be removed
soon).

[Devlin, 2016-09-06 19:24:51]

On 2016/09/06 16:31:07, imcheng wrote:
> Would it be simpler to just remove those test cases? It looks like all of them
> are testing with the EAR switch disabled, but we've already turned EAR on by
> default for quite some time now (I assume the FeatureSwitch would be removed
> soon).

Yes and no.
Yes, EAR has been enabled by default for awhile, and yes, we should work towards
removing the FeatureSwitch.  I'm hoping to do that soon. :)
Unfortunately, some of these CLs test common behavior (that's unaffected by
EAR), so we can't just remove the tests.  Instead, we should remove the logic
that disables EAR for them.  I have a CL up to do just that.

[imcheng, 2016-09-08 01:51:03]

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

[commit-bot: I haz the power, 2016-09-08 01:51:26]

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-09-08 02:32:49]

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

[commit-bot: I haz the power, 2016-09-08 02:32:49]

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

[imcheng, 2016-09-08 15:34:20]

Thanks Devlin! I see that your patch has landed. However there are still 3 test
failures (see ps2). Could you please advise on what needs to be done for them?

Also just FYI, I will be merging this and your other patch back to M54.

[Devlin, 2016-09-08 16:54:45]

On 2016/09/08 15:34:20, imcheng wrote:
> Thanks Devlin! I see that your patch has landed. However there are still 3
test
> failures (see ps2). Could you please advise on what needs to be done for them?
> 
> Also just FYI, I will be merging this and your other patch back to M54.

The remaining tests all explicitly test legacy behavior.  Long-term, they should
probably be removed (waiting to iron out a few last things), but for the time
being, I think it's safe to use a FeatureSwitch::ScopedOverride to disable the
media router switch there as well, since the toolbar redesign is disabled.

[imcheng, 2016-09-08 18:35:40]

Thanks, I updated the tests to disable MR. PTAL.

[imcheng, 2016-09-08 18:35:48]

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

[commit-bot: I haz the power, 2016-09-08 18:36:15]

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-09-08 19:36:46]

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

[commit-bot: I haz the power, 2016-09-08 19:36:47]

Dry run: This issue passed the CQ dry run.

[Devlin, 2016-09-08 22:03:13]

lgtm!

https://codereview.chromium.org/2307803002/diff/40001/chrome/browser/extensio...
File chrome/browser/extensions/extension_context_menu_model_unittest.cc (right):

https://codereview.chromium.org/2307803002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/extension_context_menu_model_unittest.cc:345: //
Disable Media Router first, since Extension Action Redesign would be force
It might be out of scope for this change, but can we remove the logic to link
these switches now that they are both default-enabled?

[imcheng, 2016-09-08 23:24:02]

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

[commit-bot: I haz the power, 2016-09-08 23:24:39]

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

[imcheng, 2016-09-09 16:51:19]

The CQ bit was checked by imcheng@chromium.org

[imcheng, 2016-09-09 16:51:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
rdevlin.cronin@chromium.org
Link to the patchset: https://codereview.chromium.org/2307803002/#ps80001
(title: "rm comments")

[imcheng, 2016-09-09 16:51:24]

https://codereview.chromium.org/2307803002/diff/40001/chrome/browser/extensio...
File chrome/browser/extensions/extension_context_menu_model_unittest.cc (right):

https://codereview.chromium.org/2307803002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/extension_context_menu_model_unittest.cc:345: //
Disable Media Router first, since Extension Action Redesign would be force
On 2016/09/08 22:03:13, Devlin wrote:
> It might be out of scope for this change, but can we remove the logic to link
> these switches now that they are both default-enabled?

Sure, it should be a pretty small and harmless change (now that both are enabled
by default)

[commit-bot: I haz the power, 2016-09-09 16:51:41]

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-09-09 17:53:08]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-09 17:57:07]

Description was changed from

==========
[Media Router] Enable MR by default now that it is at 100% in stable.

BUG=633269
==========

to

==========
[Media Router] Enable MR by default now that it is at 100% in stable.

BUG=633269

Committed: https://crrev.com/72217db0c2b09929ba393414493e0d0dd0d6e823
Cr-Commit-Position: refs/heads/master@{#417626}
==========

[commit-bot: I haz the power, 2016-09-09 17:57:08]

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