Added flag in chrome://flags to enable the New Launcher UI.

Added flag in chrome://flags to enable the New Launcher UI.

BUG=738573
Review-Url: https://codereview.chromium.org/2965813002
Cr-Commit-Position: refs/heads/master@{#484314}
Committed: https://chromium.googlesource.com/chromium/src/+/403ac1400f04a1a924811cf8515046ad58e73013

[newcomer, 2017-06-30 21:03:17]

Patchset #1 (id:1) has been deleted

[newcomer, 2017-06-30 21:04:09]

Description was changed from

==========
Added flag in chrome://flags to enable the New Launcher UI.

BUG=
==========

to

==========
Added flag in chrome://flags to enable the New Launcher UI.

BUG=738573
==========

[newcomer, 2017-06-30 21:04:09]

newcomer@chromium.org changed reviewers:
+ stevenjb@chromium.org

[newcomer, 2017-06-30 21:04:22]

PTAL!

-Alex

[newcomer, 2017-06-30 21:10:19]

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

[commit-bot: I haz the power, 2017-06-30 21:10:34]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[stevenjb, 2017-06-30 21:17:26]

https://codereview.chromium.org/2965813002/diff/20001/chrome/browser/about_fl...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/2965813002/diff/20001/chrome/browser/about_fl...
chrome/browser/about_flags.cc:1445:
SINGLE_VALUE_TYPE(app_list::switches::kEnableNewAppListUI),
Is there a better descriptor here than "new"?

https://codereview.chromium.org/2965813002/diff/20001/chrome/browser/flag_des...
File chrome/browser/flag_descriptions.cc (right):

https://codereview.chromium.org/2965813002/diff/20001/chrome/browser/flag_des...
chrome/browser/flag_descriptions.cc:2930: const char kEnableNewAppListUIName[] =
"Enable The Peeking Launcher";
Maybe enable-peeking-app-list?

https://codereview.chromium.org/2965813002/diff/20001/ui/app_list/app_list_sw...
File ui/app_list/app_list_switches.cc (right):

https://codereview.chromium.org/2965813002/diff/20001/ui/app_list/app_list_sw...
ui/app_list/app_list_switches.cc:25: const char kEnableNewAppListUI[] =
"enable-new-launcher-ui";
Is this just intended to enable the fullscreen app list feature with a flag? If
so it should be named enable-fullscreen-app-list and the variable names should
ve changed accordingly. If not, please document exactly what this switch is for.

[newcomer, 2017-06-30 21:35:53]

Patchset #2 (id:40001) has been deleted

[newcomer, 2017-06-30 21:36:58]

Patchset #2 (id:60001) has been deleted

[newcomer, 2017-06-30 21:38:42]

Patchset #2 (id:80001) has been deleted

[newcomer, 2017-06-30 21:40:24]

Thanks for the feedback!
Changed variable names.
-Alex

https://codereview.chromium.org/2965813002/diff/20001/chrome/browser/about_fl...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/2965813002/diff/20001/chrome/browser/about_fl...
chrome/browser/about_flags.cc:1445:
SINGLE_VALUE_TYPE(app_list::switches::kEnableNewAppListUI),
On 2017/06/30 21:17:25, stevenjb wrote:
> Is there a better descriptor here than "new"?

Fixed.

https://codereview.chromium.org/2965813002/diff/20001/chrome/browser/flag_des...
File chrome/browser/flag_descriptions.cc (right):

https://codereview.chromium.org/2965813002/diff/20001/chrome/browser/flag_des...
chrome/browser/flag_descriptions.cc:2930: const char kEnableNewAppListUIName[] =
"Enable The Peeking Launcher";
On 2017/06/30 21:17:25, stevenjb wrote:
> Maybe enable-peeking-app-list?

My PM wants the flag to say Launcher instead of app list, and msw@ has asked me
to keep "Launcher" out of the code base to the max extent. This is why it is
called Launcher here and app list elsewhere.

https://codereview.chromium.org/2965813002/diff/20001/ui/app_list/app_list_sw...
File ui/app_list/app_list_switches.cc (right):

https://codereview.chromium.org/2965813002/diff/20001/ui/app_list/app_list_sw...
ui/app_list/app_list_switches.cc:25: const char kEnableNewAppListUI[] =
"enable-new-launcher-ui";
On 2017/06/30 21:17:26, stevenjb wrote:
> Is this just intended to enable the fullscreen app list feature with a flag?
If
> so it should be named enable-fullscreen-app-list and the variable names should
> ve changed accordingly. If not, please document exactly what this switch is
for.

Done.

[stevenjb, 2017-06-30 22:18:59]

lgtm w/ one nit/suggestion

https://codereview.chromium.org/2965813002/diff/100001/chrome/browser/flag_de...
File chrome/browser/flag_descriptions.cc (right):

https://codereview.chromium.org/2965813002/diff/100001/chrome/browser/flag_de...
chrome/browser/flag_descriptions.cc:2934: "features.";
'launcher' is fine in these strings, although I have no idea what 'Peeking'
means; presumably folks using this switch will know?

s/The new launcher/The peeking launcher/ ?

[newcomer, 2017-06-30 22:24:04]

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

[commit-bot: I haz the power, 2017-06-30 22:24:20]

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, 2017-06-30 23:24:36]

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

[commit-bot: I haz the power, 2017-06-30 23:24:37]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[newcomer, 2017-07-05 16:36:01]

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

[commit-bot: I haz the power, 2017-07-05 16:36:20]

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, 2017-07-05 17:45:15]

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

[commit-bot: I haz the power, 2017-07-05 17:45:16]

Dry run: This issue passed the CQ dry run.

[newcomer, 2017-07-05 18:19:23]

The CQ bit was checked by newcomer@chromium.org

[newcomer, 2017-07-05 18:19:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org
Link to the patchset: https://codereview.chromium.org/2965813002/#ps140001
(title: "Added flag to histograms/enums.xml")

[commit-bot: I haz the power, 2017-07-05 18:19:52]

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, 2017-07-05 18:26:32]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1499278763971020,
"parent_rev": "e2ae4df240f4949fe9a4e433cb534a8d810efc81", "commit_rev":
"403ac1400f04a1a924811cf8515046ad58e73013"}

[commit-bot: I haz the power, 2017-07-05 18:26:48]

Description was changed from

==========
Added flag in chrome://flags to enable the New Launcher UI.

BUG=738573
==========

to

==========
Added flag in chrome://flags to enable the New Launcher UI.

BUG=738573

Review-Url: https://codereview.chromium.org/2965813002
Cr-Commit-Position: refs/heads/master@{#484314}
Committed:
https://chromium.googlesource.com/chromium/src/+/403ac1400f04a1a924811cf85150...
==========

[commit-bot: I haz the power, 2017-07-05 18:26:49]

Committed patchset #4 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/403ac1400f04a1a924811cf85150...

[newcomer, 2017-07-11 19:54:05]

https://codereview.chromium.org/2965813002/diff/100001/chrome/browser/flag_de...
File chrome/browser/flag_descriptions.cc (right):

https://codereview.chromium.org/2965813002/diff/100001/chrome/browser/flag_de...
chrome/browser/flag_descriptions.cc:2934: "features.";
On 2017/06/30 22:18:58, stevenjb wrote:
> 'launcher' is fine in these strings, although I have no idea what 'Peeking'
> means; presumably folks using this switch will know?
> 
> s/The new launcher/The peeking launcher/ ?

Good catch!