Remove BrowserDistribution::SHORTCUT_APP_LAUNCHER

Remove BrowserDistribution::SHORTCUT_APP_LAUNCHER

Also remove the BrowserDistribution::ShortcutType enum (since there
would only be a single value left).

BrowserDistribution depends on strings that won't be available once
enable_app_list is flipped to 0 on Windows. Since these constants are
generated by create_string_rc.py (which is hard to #ifdef), and used on
the single `DO_INSTALLER_STRING_MAPPING` line of
chrome_browser_main_win.cc, it's hard to prepare for the flip without
this first step.

Remove accesses to those strings by nerfing the app list install flow on
Windows and removing the icon and relaunch customizations.

For now, this just makes the app launcher window show in the taskbar
with a Chrome icon, and no window title. A follow-up in m52 will cause
attempts to show the app launcher window instead just open a browser
window at chrome://apps.

BUG=600915
Committed: https://crrev.com/d7ef82ae05ed94c97a93d2ce975260ed2bc0c7f0
Cr-Commit-Position: refs/heads/master@{#393415}

[tapted, 2016-04-29 03:30:15]

Description was changed from

==========
Remove BrowserDistribution::SHORTCUT_APP_LAUNCHER

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

to

==========
Remove BrowserDistribution::SHORTCUT_APP_LAUNCHER

BrowserDistribution depends on strings that won't be available once
enable_app_list is flipped to 0 on Windows.

Remove accesses to those strings by nerfing the app list install flow on
Windows and removing the icon and relaunch customizations. This makes
the app launcher window merge with Chrome windows if it is shown.

A follow-up will remove the app launcher window and instead just open a
browser window at chrome://apps.

BUG=600915
==========

[tapted, 2016-04-29 03:30:15]

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

[tapted, 2016-04-29 04:17:55]

Description was changed from

==========
Remove BrowserDistribution::SHORTCUT_APP_LAUNCHER

BrowserDistribution depends on strings that won't be available once
enable_app_list is flipped to 0 on Windows.

Remove accesses to those strings by nerfing the app list install flow on
Windows and removing the icon and relaunch customizations. This makes
the app launcher window merge with Chrome windows if it is shown.

A follow-up will remove the app launcher window and instead just open a
browser window at chrome://apps.

BUG=600915
==========

to

==========
Remove BrowserDistribution::SHORTCUT_APP_LAUNCHER

BrowserDistribution depends on strings that won't be available once
enable_app_list is flipped to 0 on Windows.

Remove accesses to those strings by nerfing the app list install flow on
Windows and removing the icon and relaunch customizations. This makes
the app launcher window merge with Chrome windows if it is shown.

A follow-up in m52 will remove the app launcher window and instead just
open a browser window at chrome://apps.

BUG=600915
==========

[tapted, 2016-04-29 05:49:11]

Description was changed from

==========
Remove BrowserDistribution::SHORTCUT_APP_LAUNCHER

BrowserDistribution depends on strings that won't be available once
enable_app_list is flipped to 0 on Windows.

Remove accesses to those strings by nerfing the app list install flow on
Windows and removing the icon and relaunch customizations. This makes
the app launcher window merge with Chrome windows if it is shown.

A follow-up in m52 will remove the app launcher window and instead just
open a browser window at chrome://apps.

BUG=600915
==========

to

==========
Remove BrowserDistribution::SHORTCUT_APP_LAUNCHER

BrowserDistribution depends on strings that won't be available once
enable_app_list is flipped to 0 on Windows.

Remove accesses to those strings by nerfing the app list install flow on
Windows and removing the icon and relaunch customizations.

For now, this just makes the app launcher window show in the taskbar
with a Chrome icon, and no window title. A follow-up in m52 will cause
attempts to show the app launcher window instead just open a browser
window at chrome://apps.

BUG=600915
==========

[tapted, 2016-04-29 05:49:11]

tapted@chromium.org changed reviewers:
+ grt@chromium.org
- mgiuca@chromium.org

[tapted, 2016-04-29 05:57:49]

Hi Greg, please take a look (and, e.g., should I just delete the
BrowserDistribution::ShortcutType entirely?)

Thanks!

[grt (UTC plus 2), 2016-04-29 14:36:26]

On 2016/04/29 05:57:49, tapted wrote:
> Hi Greg, please take a look (and, e.g., should I just delete the
> BrowserDistribution::ShortcutType entirely?)

I'm not aware of any upcoming need for different types of shortcuts, so I think
removing it altogether is a nice cleanup. Thanks.

[tapted, 2016-05-02 06:28:17]

Patchset #4 (id:60001) has been deleted

[tapted, 2016-05-02 06:42:01]

Description was changed from

==========
Remove BrowserDistribution::SHORTCUT_APP_LAUNCHER

BrowserDistribution depends on strings that won't be available once
enable_app_list is flipped to 0 on Windows.

Remove accesses to those strings by nerfing the app list install flow on
Windows and removing the icon and relaunch customizations.

For now, this just makes the app launcher window show in the taskbar
with a Chrome icon, and no window title. A follow-up in m52 will cause
attempts to show the app launcher window instead just open a browser
window at chrome://apps.

BUG=600915
==========

to

==========
Remove BrowserDistribution::SHORTCUT_APP_LAUNCHER

Also remove the BrowserDistribution::ShortcutType enum (there's only one
value left).

BrowserDistribution depends on strings that won't be available once
enable_app_list is flipped to 0 on Windows. Since these constants are
generated by create_string_rc.py (which is hard to #ifdef), and used on
the single `DO_INSTALLER_STRING_MAPPING` line of
chrome_browser_main_win.cc, it's hard to prepare for the flip without
this first step.

Remove accesses to those strings by nerfing the app list install flow on
Windows and removing the icon and relaunch customizations.

For now, this just makes the app launcher window show in the taskbar
with a Chrome icon, and no window title. A follow-up in m52 will cause
attempts to show the app launcher window instead just open a browser
window at chrome://apps.

BUG=600915
==========

[tapted, 2016-05-02 06:43:56]

Description was changed from

==========
Remove BrowserDistribution::SHORTCUT_APP_LAUNCHER

Also remove the BrowserDistribution::ShortcutType enum (there's only one
value left).

BrowserDistribution depends on strings that won't be available once
enable_app_list is flipped to 0 on Windows. Since these constants are
generated by create_string_rc.py (which is hard to #ifdef), and used on
the single `DO_INSTALLER_STRING_MAPPING` line of
chrome_browser_main_win.cc, it's hard to prepare for the flip without
this first step.

Remove accesses to those strings by nerfing the app list install flow on
Windows and removing the icon and relaunch customizations.

For now, this just makes the app launcher window show in the taskbar
with a Chrome icon, and no window title. A follow-up in m52 will cause
attempts to show the app launcher window instead just open a browser
window at chrome://apps.

BUG=600915
==========

to

==========
Remove BrowserDistribution::SHORTCUT_APP_LAUNCHER

Also remove the BrowserDistribution::ShortcutType enum (since there
would only be a single value left).

BrowserDistribution depends on strings that won't be available once
enable_app_list is flipped to 0 on Windows. Since these constants are
generated by create_string_rc.py (which is hard to #ifdef), and used on
the single `DO_INSTALLER_STRING_MAPPING` line of
chrome_browser_main_win.cc, it's hard to prepare for the flip without
this first step.

Remove accesses to those strings by nerfing the app list install flow on
Windows and removing the icon and relaunch customizations.

For now, this just makes the app launcher window show in the taskbar
with a Chrome icon, and no window title. A follow-up in m52 will cause
attempts to show the app launcher window instead just open a browser
window at chrome://apps.

BUG=600915
==========

[tapted, 2016-05-02 10:57:52]

Patchset #4 (id:80001) has been deleted

[tapted, 2016-05-02 10:58:12]

Patchset #3 (id:40001) has been deleted

[tapted, 2016-05-02 11:00:09]

On 2016/04/29 14:36:26, grt wrote:
> On 2016/04/29 05:57:49, tapted wrote:
> > Hi Greg, please take a look (and, e.g., should I just delete the
> > BrowserDistribution::ShortcutType entirely?)
> 
> I'm not aware of any upcoming need for different types of shortcuts, so I
think
> removing it altogether is a nice cleanup. Thanks.

Done.

[grt (UTC plus 2), 2016-05-03 16:44:11]

grt@chromium.org changed reviewers:
+ huangs@chromium.org

[grt (UTC plus 2), 2016-05-03 16:44:11]

Sam: do you have cycles to review this change?

[huangs, 2016-05-03 17:43:44]

Sure I'll review this.

[huangs, 2016-05-03 18:12:02]

Looks good, with some minor comments.

Are there plans to remove the DidRun flag from the registry?

https://codereview.chromium.org/1926403002/diff/100001/chrome/browser/ui/view...
File chrome/browser/ui/views/app_list/win/app_list_service_win.cc (right):

https://codereview.chromium.org/1926403002/diff/100001/chrome/browser/ui/view...
chrome/browser/ui/views/app_list/win/app_list_service_win.cc:73: void
SetDidRunForNDayActiveStats() {
Add TODO for removing this function?

https://codereview.chromium.org/1926403002/diff/100001/chrome/browser/ui/view...
chrome/browser/ui/views/app_list/win/app_list_service_win.cc:149: NOTREACHED();
Are all the callers dealt with? For example, there's one in
AppListServiceImpl::EnableAppList().

https://codereview.chromium.org/1926403002/diff/100001/chrome/installer/util/...
File chrome/installer/util/browser_distribution.h (right):

https://codereview.chromium.org/1926403002/diff/100001/chrome/installer/util/...
chrome/installer/util/browser_distribution.h:79: // Returns the localized name
of the shortcut identified by |shortcut_type|
Update comment re. |shortcut_type|.

https://codereview.chromium.org/1926403002/diff/100001/chrome/installer/util/...
chrome/installer/util/browser_distribution.h:84: // |shortcut_type|, inside the
file specified by GetIconFilename().
|shortcut_type|

[tapted, 2016-05-04 09:50:57]

On 2016/05/03 18:12:02, huangs wrote:
> Are there plans to remove the DidRun flag from the registry?

Filed http://crbug.com/609009 for this

https://codereview.chromium.org/1926403002/diff/100001/chrome/browser/ui/view...
File chrome/browser/ui/views/app_list/win/app_list_service_win.cc (right):

https://codereview.chromium.org/1926403002/diff/100001/chrome/browser/ui/view...
chrome/browser/ui/views/app_list/win/app_list_service_win.cc:73: void
SetDidRunForNDayActiveStats() {
On 2016/05/03 18:12:02, huangs wrote:
> Add TODO for removing this function?

as part of http://crbug.com/600915 we'll actually be deleting the entire file :)

https://codereview.chromium.org/1926403002/diff/100001/chrome/browser/ui/view...
chrome/browser/ui/views/app_list/win/app_list_service_win.cc:149: NOTREACHED();
On 2016/05/03 18:12:02, huangs wrote:
> Are all the callers dealt with? For example, there's one in
> AppListServiceImpl::EnableAppList().

This is already disabled server-side on the webstore for m51+. The only other
way of getting here is by passing --enable-app-list on the command line, which
is no longer supported. Shortly after this and http://crrev.com/1861233003 lands
we will flip enable_app_list to 0 on Windows, which will substitute
app_list_service_win.cc entirely with app_list_service_disabled.cc

https://codereview.chromium.org/1926403002/diff/100001/chrome/installer/util/...
File chrome/installer/util/browser_distribution.h (right):

https://codereview.chromium.org/1926403002/diff/100001/chrome/installer/util/...
chrome/installer/util/browser_distribution.h:79: // Returns the localized name
of the shortcut identified by |shortcut_type|
On 2016/05/03 18:12:02, huangs wrote:
> Update comment re. |shortcut_type|.

Done.

https://codereview.chromium.org/1926403002/diff/100001/chrome/installer/util/...
chrome/installer/util/browser_distribution.h:84: // |shortcut_type|, inside the
file specified by GetIconFilename().
On 2016/05/03 18:12:02, huangs wrote:
> |shortcut_type|

Done.

[huangs, 2016-05-04 15:54:08]

LGTM with comments on possibly deleting more stuff.  Will look again if
requested.

https://codereview.chromium.org/1926403002/diff/120001/chrome/browser/shell_i...
File chrome/browser/shell_integration_win.cc (right):

https://codereview.chromium.org/1926403002/diff/120001/chrome/browser/shell_i...
chrome/browser/shell_integration_win.cc:129:
command_line.GetSwitchValueASCII(switches::kAppId)));
Delete other instances of kShowAppList?  Or perhaps that's planned for another
CL?

https://codereview.chromium.org/1926403002/diff/120001/chrome/browser/ui/view...
File chrome/browser/ui/views/app_list/win/app_list_service_win.cc (right):

https://codereview.chromium.org/1926403002/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/app_list/win/app_list_service_win.cc:36: #include
"chrome/browser/web_applications/web_app.h"
Can probably delete web_app.h -- and in general, perhaps curate #include 
depends on how you feel about "current state must be as good as possible" vs.
"this file will disappear anyway".

https://codereview.chromium.org/1926403002/diff/120001/chrome/installer/util/...
File chrome/installer/util/prebuild/create_string_rc.py (right):

https://codereview.chromium.org/1926403002/diff/120001/chrome/installer/util/...
chrome/installer/util/prebuild/create_string_rc.py:67:
'IDS_SHORTCUT_NEW_WINDOW',
Code search shows that these strings appear in more places, e.g.,
src/chrome/browser/ui/app_list/app_list_service_mac.mm

should these be deleted in this CL, or would this be done elsewhere?

[tapted, 2016-05-05 07:14:17]

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

[tapted, 2016-05-05 07:14:18]

+erg for c/b/profiles/OWNERS
grt: PTAL for OWNERS when you get a chance

Thanks!

https://codereview.chromium.org/1926403002/diff/120001/chrome/browser/shell_i...
File chrome/browser/shell_integration_win.cc (right):

https://codereview.chromium.org/1926403002/diff/120001/chrome/browser/shell_i...
chrome/browser/shell_integration_win.cc:129:
command_line.GetSwitchValueASCII(switches::kAppId)));
On 2016/05/04 15:54:07, huangs wrote:
> Delete other instances of kShowAppList?  Or perhaps that's planned for another
> CL?

There's a bit about this on the bug at http://crbug.com/600915#c4 -- we'll keep
kShowAppList around for a while, but instead of showing the app list, it will
just open a browser window at chrome://apps

https://codereview.chromium.org/1926403002/diff/120001/chrome/browser/ui/view...
File chrome/browser/ui/views/app_list/win/app_list_service_win.cc (right):

https://codereview.chromium.org/1926403002/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/app_list/win/app_list_service_win.cc:36: #include
"chrome/browser/web_applications/web_app.h"
On 2016/05/04 15:54:07, huangs wrote:
> Can probably delete web_app.h -- and in general, perhaps curate #include 
> depends on how you feel about "current state must be as good as possible" vs.
> "this file will disappear anyway".
> 

Done. Linted a bunch of headers.

https://codereview.chromium.org/1926403002/diff/120001/chrome/installer/util/...
File chrome/installer/util/prebuild/create_string_rc.py (right):

https://codereview.chromium.org/1926403002/diff/120001/chrome/installer/util/...
chrome/installer/util/prebuild/create_string_rc.py:67:
'IDS_SHORTCUT_NEW_WINDOW',
On 2016/05/04 15:54:08, huangs wrote:
> Code search shows that these strings appear in more places, e.g.,
> src/chrome/browser/ui/app_list/app_list_service_mac.mm
> 
> should these be deleted in this CL, or would this be done elsewhere?

This will come in a follow-up, but these need to go first since these
occurrences in a python script are rather tricky to #ifdef around :). (There's
more about this in the CL description).

[Elliot Glaysher, 2016-05-05 17:11:57]

lgtm

[tapted, 2016-05-11 12:41:23]

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

[tapted, 2016-05-11 12:41:24]

grt: ping for OWNERS
or +gab, if you're around.

Thanks!

[grt (UTC plus 2), 2016-05-12 17:16:24]

rubberstamp lgtm. apologies for the delay.

[tapted, 2016-05-12 23:57:06]

The CQ bit was checked by tapted@chromium.org

[tapted, 2016-05-12 23:57:07]

The patchset sent to the CQ was uploaded after l-g-t-m from huangs@chromium.org
Link to the patchset: https://codereview.chromium.org/1926403002/#ps140001
(title: "lint headers")

[commit-bot: I haz the power, 2016-05-12 23:57:24]

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

[commit-bot: I haz the power, 2016-05-13 01:10:32]

Description was changed from

==========
Remove BrowserDistribution::SHORTCUT_APP_LAUNCHER

Also remove the BrowserDistribution::ShortcutType enum (since there
would only be a single value left).

BrowserDistribution depends on strings that won't be available once
enable_app_list is flipped to 0 on Windows. Since these constants are
generated by create_string_rc.py (which is hard to #ifdef), and used on
the single `DO_INSTALLER_STRING_MAPPING` line of
chrome_browser_main_win.cc, it's hard to prepare for the flip without
this first step.

Remove accesses to those strings by nerfing the app list install flow on
Windows and removing the icon and relaunch customizations.

For now, this just makes the app launcher window show in the taskbar
with a Chrome icon, and no window title. A follow-up in m52 will cause
attempts to show the app launcher window instead just open a browser
window at chrome://apps.

BUG=600915
==========

to

==========
Remove BrowserDistribution::SHORTCUT_APP_LAUNCHER

Also remove the BrowserDistribution::ShortcutType enum (since there
would only be a single value left).

BrowserDistribution depends on strings that won't be available once
enable_app_list is flipped to 0 on Windows. Since these constants are
generated by create_string_rc.py (which is hard to #ifdef), and used on
the single `DO_INSTALLER_STRING_MAPPING` line of
chrome_browser_main_win.cc, it's hard to prepare for the flip without
this first step.

Remove accesses to those strings by nerfing the app list install flow on
Windows and removing the icon and relaunch customizations.

For now, this just makes the app launcher window show in the taskbar
with a Chrome icon, and no window title. A follow-up in m52 will cause
attempts to show the app launcher window instead just open a browser
window at chrome://apps.

BUG=600915
==========

[commit-bot: I haz the power, 2016-05-13 01:10:33]

Committed patchset #5 (id:140001)

[commit-bot: I haz the power, 2016-05-13 01:11:57]

Description was changed from

==========
Remove BrowserDistribution::SHORTCUT_APP_LAUNCHER

Also remove the BrowserDistribution::ShortcutType enum (since there
would only be a single value left).

BrowserDistribution depends on strings that won't be available once
enable_app_list is flipped to 0 on Windows. Since these constants are
generated by create_string_rc.py (which is hard to #ifdef), and used on
the single `DO_INSTALLER_STRING_MAPPING` line of
chrome_browser_main_win.cc, it's hard to prepare for the flip without
this first step.

Remove accesses to those strings by nerfing the app list install flow on
Windows and removing the icon and relaunch customizations.

For now, this just makes the app launcher window show in the taskbar
with a Chrome icon, and no window title. A follow-up in m52 will cause
attempts to show the app launcher window instead just open a browser
window at chrome://apps.

BUG=600915
==========

to

==========
Remove BrowserDistribution::SHORTCUT_APP_LAUNCHER

Also remove the BrowserDistribution::ShortcutType enum (since there
would only be a single value left).

BrowserDistribution depends on strings that won't be available once
enable_app_list is flipped to 0 on Windows. Since these constants are
generated by create_string_rc.py (which is hard to #ifdef), and used on
the single `DO_INSTALLER_STRING_MAPPING` line of
chrome_browser_main_win.cc, it's hard to prepare for the flip without
this first step.

Remove accesses to those strings by nerfing the app list install flow on
Windows and removing the icon and relaunch customizations.

For now, this just makes the app launcher window show in the taskbar
with a Chrome icon, and no window title. A follow-up in m52 will cause
attempts to show the app launcher window instead just open a browser
window at chrome://apps.

BUG=600915

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

[commit-bot: I haz the power, 2016-05-13 01:11:59]

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