Expose the MacViewsWebUIDialogs feature flag to chrome://flags

Expose the MacViewsWebUIDialogs feature flag to chrome://flags

Currently it's enabled via --enable=features=MacViewsWebUIDialogs, but
we want to allow changing it in chrome://flags as well.

Replace the old-style --enable-mac-views-dialogs switch for other dialog
types with a feature flag as well to be consistent. Now it is
--enable-features=MacViewsNativeDialogs.

BUG=603373
Committed: https://crrev.com/d1cb0bfdddf26dc5f7d4eb0a8a4c8bcf2f0cee5d
Cr-Commit-Position: refs/heads/master@{#396100}

[tapted, 2016-05-24 05:58:18]

Patchset #1 (id:1) has been deleted

[tapted, 2016-05-24 05:58:26]

Patchset #1 (id:20001) has been deleted

[tapted, 2016-05-24 08:07:52]

Description was changed from

==========
Expose the MacViewsWebUIDialogs feature flag to chrome://flags

Currently it's enabled via --enable=features=MacViewsWebUIDialogs, but
we want to allow changing it in chrome://flags as well.

BUG=603373
==========

to

==========
Expose the MacViewsWebUIDialogs feature flag to chrome://flags

Currently it's enabled via --enable=features=MacViewsWebUIDialogs, but
we want to allow changing it in chrome://flags as well.

BUG=603373
==========

[tapted, 2016-05-24 08:07:52]

tapted@chromium.org changed reviewers:
+ msw@chromium.org

[tapted, 2016-05-24 08:09:55]

Hi Mike, please take a look

[msw, 2016-05-24 17:01:30]

lgtm with nits

https://codereview.chromium.org/2004043003/diff/80001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2004043003/diff/80001/chrome/app/generated_re...
chrome/app/generated_resources.grd:6587: +        <message
name="IDS_FLAGS_MAC_VIEWS_DIALOGS_PHASE2_NAME" desc="Name of the flag to enable
the Phase2 set of toolkit-views browser dialogs on Mac." translateable="false">
nit: Phase numbers don't seem all that meaningful; can we use
'mac-views-[native-]dialogs' and 'mac-views-webui-dialogs' or similar?

https://codereview.chromium.org/2004043003/diff/80001/chrome/browser/ui/brows...
File chrome/browser/ui/browser_dialogs_mac.cc (right):

https://codereview.chromium.org/2004043003/diff/80001/chrome/browser/ui/brows...
chrome/browser/ui/browser_dialogs_mac.cc:18: return
base::CommandLine::ForCurrentProcess()->HasSwitch(
optional nit: convert this to a feature for parity?

[tapted, 2016-05-25 12:01:13]

Description was changed from

==========
Expose the MacViewsWebUIDialogs feature flag to chrome://flags

Currently it's enabled via --enable=features=MacViewsWebUIDialogs, but
we want to allow changing it in chrome://flags as well.

BUG=603373
==========

to

==========
Expose the MacViewsWebUIDialogs feature flag to chrome://flags

Currently it's enabled via --enable=features=MacViewsWebUIDialogs, but
we want to allow changing it in chrome://flags as well.

Replace the old-style --enable-mac-views-dialogs switch for other dialog
types with a feature flag as well to be consistent. Now it is
--enable-features=MacViewsNativeDialogs.

BUG=603373
==========

[tapted, 2016-05-25 12:01:50]

msw, PTAL

(also +ellyjones cc FYI)

https://codereview.chromium.org/2004043003/diff/80001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2004043003/diff/80001/chrome/app/generated_re...
chrome/app/generated_resources.grd:6587: +        <message
name="IDS_FLAGS_MAC_VIEWS_DIALOGS_PHASE2_NAME" desc="Name of the flag to enable
the Phase2 set of toolkit-views browser dialogs on Mac." translateable="false">
On 2016/05/24 17:01:29, msw wrote:
> nit: Phase numbers don't seem all that meaningful; can we use
> 'mac-views-[native-]dialogs' and 'mac-views-webui-dialogs' or similar?

Done.

https://codereview.chromium.org/2004043003/diff/80001/chrome/browser/ui/brows...
File chrome/browser/ui/browser_dialogs_mac.cc (right):

https://codereview.chromium.org/2004043003/diff/80001/chrome/browser/ui/brows...
chrome/browser/ui/browser_dialogs_mac.cc:18: return
base::CommandLine::ForCurrentProcess()->HasSwitch(
On 2016/05/24 17:01:29, msw wrote:
> optional nit: convert this to a feature for parity?

Done. I do like consistency :). But my conscience wouldn't let me just tick the
commit box.. a couple of new files are in the diff now.

I guess it raises the issue of whether ToolkitViewsDialogsEnabled() should be
renamed too, but that's a bit more churn - a follow-up perhaps. But also if
things go smoothly, ToolkitViewsWebUIDialogsEnabled() goes away "soon" and
ToolkitViewsDialogsEnabled() makes sense again by itself, so maybe it's fine as
is.

[msw, 2016-05-25 16:48:16]

lgtm with nits; thanks!

https://codereview.chromium.org/2004043003/diff/120001/chrome/browser/ui/brow...
File chrome/browser/ui/browser_dialogs_mac.cc (right):

https://codereview.chromium.org/2004043003/diff/120001/chrome/browser/ui/brow...
chrome/browser/ui/browser_dialogs_mac.cc:7: #include "base/command_line.h"
nit: remove

https://codereview.chromium.org/2004043003/diff/120001/chrome/browser/ui/brow...
chrome/browser/ui/browser_dialogs_mac.cc:9: #include
"chrome/common/chrome_switches.h"
nit: remove

[tapted, 2016-05-26 00:48:54]

Patchset #6 (id:140001) has been deleted

[tapted, 2016-05-26 00:49:19]

https://codereview.chromium.org/2004043003/diff/120001/chrome/browser/ui/brow...
File chrome/browser/ui/browser_dialogs_mac.cc (right):

https://codereview.chromium.org/2004043003/diff/120001/chrome/browser/ui/brow...
chrome/browser/ui/browser_dialogs_mac.cc:7: #include "base/command_line.h"
On 2016/05/25 16:48:16, msw wrote:
> nit: remove

Done.

https://codereview.chromium.org/2004043003/diff/120001/chrome/browser/ui/brow...
chrome/browser/ui/browser_dialogs_mac.cc:9: #include
"chrome/common/chrome_switches.h"
On 2016/05/25 16:48:15, msw wrote:
> nit: remove

Done.

[tapted, 2016-05-26 00:49:26]

The CQ bit was checked by tapted@chromium.org

[tapted, 2016-05-26 00:49:26]

The patchset sent to the CQ was uploaded after l-g-t-m from msw@chromium.org
Link to the patchset: https://codereview.chromium.org/2004043003/#ps160001
(title: "remove unused #includes")

[commit-bot: I haz the power, 2016-05-26 00:50:37]

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

[commit-bot: I haz the power, 2016-05-26 03:26:36]

Description was changed from

==========
Expose the MacViewsWebUIDialogs feature flag to chrome://flags

Currently it's enabled via --enable=features=MacViewsWebUIDialogs, but
we want to allow changing it in chrome://flags as well.

Replace the old-style --enable-mac-views-dialogs switch for other dialog
types with a feature flag as well to be consistent. Now it is
--enable-features=MacViewsNativeDialogs.

BUG=603373
==========

to

==========
Expose the MacViewsWebUIDialogs feature flag to chrome://flags

Currently it's enabled via --enable=features=MacViewsWebUIDialogs, but
we want to allow changing it in chrome://flags as well.

Replace the old-style --enable-mac-views-dialogs switch for other dialog
types with a feature flag as well to be consistent. Now it is
--enable-features=MacViewsNativeDialogs.

BUG=603373
==========

[commit-bot: I haz the power, 2016-05-26 03:26:37]

Committed patchset #6 (id:160001)

[commit-bot: I haz the power, 2016-05-26 03:29:09]

Description was changed from

==========
Expose the MacViewsWebUIDialogs feature flag to chrome://flags

Currently it's enabled via --enable=features=MacViewsWebUIDialogs, but
we want to allow changing it in chrome://flags as well.

Replace the old-style --enable-mac-views-dialogs switch for other dialog
types with a feature flag as well to be consistent. Now it is
--enable-features=MacViewsNativeDialogs.

BUG=603373
==========

to

==========
Expose the MacViewsWebUIDialogs feature flag to chrome://flags

Currently it's enabled via --enable=features=MacViewsWebUIDialogs, but
we want to allow changing it in chrome://flags as well.

Replace the old-style --enable-mac-views-dialogs switch for other dialog
types with a feature flag as well to be consistent. Now it is
--enable-features=MacViewsNativeDialogs.

BUG=603373

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

[commit-bot: I haz the power, 2016-05-26 03:29:10]

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