Revert "Enable MidiManagerWinrt by default on Windows 10"

Revert "Enable MidiManagerWinrt by default on Windows 10"

Make MidiManagerWinrt back behind a Finch flag disabled by default.

Reverting CL https://codereview.chromium.org/2392263002/ while preserving
strings and histograms for the "disabled" flag. Revert this CL to enable it by
default again.

BUG=645403, 659458
Committed: https://crrev.com/2d340033715f50c38b75b4cbaa06b7445411869c
Cr-Commit-Position: refs/heads/master@{#428608}

[Shao-Chuan Lee, 2016-10-26 03:54:23]

shaochuan@chromium.org changed reviewers:
+ toyoshim@chromium.org

[Shao-Chuan Lee, 2016-10-26 03:54:23]

ptal

[Takashi Toyoshima, 2016-10-26 21:29:54]

https://codereview.chromium.org/2450033003/diff/1/media/midi/midi_manager_win.cc
File media/midi/midi_manager_win.cc (right):

https://codereview.chromium.org/2450033003/diff/1/media/midi/midi_manager_win...
media/midi/midi_manager_win.cc:1197: if
(base::FeatureList::IsEnabled(features::kMidiManagerWinrt) &&
Does this compile?
Since we are outside the media namespace here, features::kMidiManagerWinrt
should not be resolved as media::midi::features::kMidiManagerWinrt.

Anyway, this should be fine once you fix the switch side namespace.

https://codereview.chromium.org/2450033003/diff/1/media/midi/midi_switches.cc
File media/midi/midi_switches.cc (right):

https://codereview.chromium.org/2450033003/diff/1/media/midi/midi_switches.cc...
media/midi/midi_switches.cc:17: namespace media {
ditto

https://codereview.chromium.org/2450033003/diff/1/media/midi/midi_switches.h
File media/midi/midi_switches.h (right):

https://codereview.chromium.org/2450033003/diff/1/media/midi/midi_switches.h#...
media/midi/midi_switches.h:22: namespace media {
Ah, now we are not inside the media namespace any more in ToT because
mojofication does not recommend to use nested namespace for a service.
Can you remove this though you will need it for beta branch.

[Takashi Toyoshima, 2016-10-26 21:32:11]

Comments order was reversed. So, 'ditto' means 'see comments on
midi_switches.h'.

[Shao-Chuan Lee, 2016-10-27 08:26:17]

Oh, I didn't notice the namespace change. Now it compiles.
Could you help with merging to M54 once it's approved? Or it will take another
day due to timezone... thanks.

https://codereview.chromium.org/2450033003/diff/1/media/midi/midi_manager_win.cc
File media/midi/midi_manager_win.cc (right):

https://codereview.chromium.org/2450033003/diff/1/media/midi/midi_manager_win...
media/midi/midi_manager_win.cc:1197: if
(base::FeatureList::IsEnabled(features::kMidiManagerWinrt) &&
On 2016/10/26 21:29:54, toyoshim (slow til Nov.2) wrote:
> Does this compile?
> Since we are outside the media namespace here, features::kMidiManagerWinrt
> should not be resolved as media::midi::features::kMidiManagerWinrt.
> 
> Anyway, this should be fine once you fix the switch side namespace.

Done.

https://codereview.chromium.org/2450033003/diff/1/media/midi/midi_switches.cc
File media/midi/midi_switches.cc (right):

https://codereview.chromium.org/2450033003/diff/1/media/midi/midi_switches.cc...
media/midi/midi_switches.cc:17: namespace media {
On 2016/10/26 21:29:54, toyoshim (slow til Nov.2) wrote:
> ditto

Done.

https://codereview.chromium.org/2450033003/diff/1/media/midi/midi_switches.h
File media/midi/midi_switches.h (right):

https://codereview.chromium.org/2450033003/diff/1/media/midi/midi_switches.h#...
media/midi/midi_switches.h:22: namespace media {
On 2016/10/26 21:29:54, toyoshim (slow til Nov.2) wrote:
> Ah, now we are not inside the media namespace any more in ToT because
> mojofication does not recommend to use nested namespace for a service.
> Can you remove this though you will need it for beta branch.

Done.

[Takashi Toyoshima, 2016-10-27 17:42:53]

lgtm

[Takashi Toyoshima, 2016-10-27 17:43:20]

The CQ bit was checked by toyoshim@chromium.org

[commit-bot: I haz the power, 2016-10-27 17:43:43]

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-27 18:52:29]

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

[commit-bot: I haz the power, 2016-10-27 18:52:30]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Takashi Toyoshima, 2016-10-28 17:37:06]

The CQ bit was checked by toyoshim@chromium.org

[commit-bot: I haz the power, 2016-10-28 17:37:35]

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:23:30]

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

[commit-bot: I haz the power, 2016-10-28 21:23:34]

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

[Shao-Chuan Lee, 2016-10-29 06:28:03]

The CQ bit was checked by shaochuan@chromium.org

[commit-bot: I haz the power, 2016-10-29 06:28:08]

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-29 07:26:22]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-29 07:27:53]

Description was changed from

==========
Revert "Enable MidiManagerWinrt by default on Windows 10"

Make MidiManagerWinrt back behind a Finch flag disabled by default.

Reverting CL https://codereview.chromium.org/2392263002/ while preserving
strings and histograms for the "disabled" flag. Revert this CL to enable it by
default again.

BUG=645403,659458
==========

to

==========
Revert "Enable MidiManagerWinrt by default on Windows 10"

Make MidiManagerWinrt back behind a Finch flag disabled by default.

Reverting CL https://codereview.chromium.org/2392263002/ while preserving
strings and histograms for the "disabled" flag. Revert this CL to enable it by
default again.

BUG=645403,659458

Committed: https://crrev.com/2d340033715f50c38b75b4cbaa06b7445411869c
Cr-Commit-Position: refs/heads/master@{#428608}
==========

[commit-bot: I haz the power, 2016-10-29 07:27:54]

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