Purge the App Launcher code from Windows.

Purge the App Launcher code from Windows.

It hasn't been compiled for 1 milestone (since r394856).

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

[calamity, 2016-07-19 06:40:45]

Patchset #1 (id:1) has been deleted

[calamity, 2016-07-19 06:40:56]

Patchset #1 (id:20001) has been deleted

[calamity, 2016-07-19 06:46:46]

Patchset #1 (id:40001) has been deleted

[calamity, 2016-07-19 06:46:55]

Patchset #1 (id:60001) has been deleted

[calamity, 2016-07-19 06:47:06]

Patchset #1 (id:80001) has been deleted

[calamity, 2016-07-19 06:47:14]

Patchset #1 (id:100001) has been deleted

[calamity, 2016-07-19 06:47:25]

Patchset #1 (id:120001) has been deleted

[calamity, 2016-07-19 06:47:49]

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

[tapted, 2016-07-20 01:15:34]

make sure you do a spot-check with an official build - I don't think the trybots
will cover that, but you might tickle something in the waterfall and get
reverted.

https://codereview.chromium.org/2143893002/diff/140001/chrome/browser/ui/app_...
File chrome/browser/ui/app_list/app_list_service.cc (right):

https://codereview.chromium.org/2143893002/diff/140001/chrome/browser/ui/app_...
chrome/browser/ui/app_list/app_list_service.cc:163: bool
AppListService::HandleLaunchCommandLine(
hm - I guess desktop linux will still call this. We could delete this now, or
just to delete switches::kShowAppList entirely which should remind us

https://codereview.chromium.org/2143893002/diff/140001/chrome/browser/ui/view...
File chrome/browser/ui/views/app_list/win/app_list_service_win.cc (left):

https://codereview.chromium.org/2143893002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/app_list/win/app_list_service_win.cc:85:
GoogleUpdateSettings::UpdateDidRunStateForApp(
This is the only caller external to google_update_settings.cc --
UpdateDidRunStateForBinaries should just call WriteGoogleUpdateStrKeyInternal
directly

https://codereview.chromium.org/2143893002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/app_list/win/app_list_service_win.cc:124:
shower().WarmupForProfile(initial_profile);
I think this is the only caller of WarmupForProfile(), so we should delete that
now from AppListShower, then scythe out all the related stuff -->
AppListView::Prerender (and all the stuff it calls). We can probably remove
app_list/views/cached_label completely too, since PaintToBackingImage() is only
called in AppListItemView::Prerender()

https://codereview.chromium.org/2143893002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/app_list/win/app_list_service_win.cc:125:
UMA_HISTOGRAM_TIMES("Apps.AppListWarmupDuration",
Mark obsolete in histograms.xml

https://codereview.chromium.org/2143893002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/app_list/win/app_list_service_win.cc:131:
shower().app_list()->SetNextPaintCallback(base::Bind(callback));
This is the only caller of SetNextPaintCallback() outside of
app_list_view_unittest.cc - we should rewrite the test and delete
SetNextPaintCallback (or at least move it to the test API).

https://codereview.chromium.org/2143893002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/app_list/win/app_list_service_win.cc:194:
local_state->GetInt64(prefs::kAppListLastLaunchTime);
This is the only getter, so this should be deleted from prefs::

https://codereview.chromium.org/2143893002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/app_list/win/app_list_service_win.cc:207: return
!shower().HasView() && !profile_loader().IsAnyProfileLoading();
hm - I don't think linux calls IsAnyProfileLoading(), but it's used a lot in
tests. I guess this can wait until we delete Linux + ProfileLoader

https://codereview.chromium.org/2143893002/diff/140001/ui/app_list/views/app_...
File ui/app_list/views/app_list_view.cc (right):

https://codereview.chromium.org/2143893002/diff/140001/ui/app_list/views/app_...
ui/app_list/views/app_list_view.cc:585: params->force_show_in_taskbar = true;
we should delete Widget::InitParams::force_show_in_taskbar when we purge this on
linux #reminder

[calamity, 2016-07-22 03:15:17]

Patchset #4 (id:200001) has been deleted

[calamity, 2016-07-22 03:20:07]

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

[commit-bot: I haz the power, 2016-07-22 03:20:51]

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-07-22 04:07:39]

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

[commit-bot: I haz the power, 2016-07-22 04:07:40]

Dry run: This issue passed the CQ dry run.

[calamity, 2016-07-22 06:49:29]

Patchset #3 (id:180001) has been deleted

[calamity, 2016-07-25 02:34:22]

https://codereview.chromium.org/2143893002/diff/140001/chrome/browser/ui/app_...
File chrome/browser/ui/app_list/app_list_service.cc (right):

https://codereview.chromium.org/2143893002/diff/140001/chrome/browser/ui/app_...
chrome/browser/ui/app_list/app_list_service.cc:163: bool
AppListService::HandleLaunchCommandLine(
On 2016/07/20 01:15:34, tapted wrote:
> hm - I guess desktop linux will still call this. We could delete this now, or
> just to delete switches::kShowAppList entirely which should remind us

Acknowledged.

https://codereview.chromium.org/2143893002/diff/140001/chrome/browser/ui/view...
File chrome/browser/ui/views/app_list/win/app_list_service_win.cc (left):

https://codereview.chromium.org/2143893002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/app_list/win/app_list_service_win.cc:85:
GoogleUpdateSettings::UpdateDidRunStateForApp(
On 2016/07/20 01:15:34, tapted wrote:
> This is the only caller external to google_update_settings.cc --
> UpdateDidRunStateForBinaries should just call WriteGoogleUpdateStrKeyInternal
> directly

Done.

https://codereview.chromium.org/2143893002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/app_list/win/app_list_service_win.cc:124:
shower().WarmupForProfile(initial_profile);
On 2016/07/20 01:15:34, tapted wrote:
> I think this is the only caller of WarmupForProfile(), so we should delete
that
> now from AppListShower, then scythe out all the related stuff -->
> AppListView::Prerender (and all the stuff it calls). We can probably remove
> app_list/views/cached_label completely too, since PaintToBackingImage() is
only
> called in AppListItemView::Prerender()

Done.

https://codereview.chromium.org/2143893002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/app_list/win/app_list_service_win.cc:125:
UMA_HISTOGRAM_TIMES("Apps.AppListWarmupDuration",
On 2016/07/20 01:15:34, tapted wrote:
> Mark obsolete in histograms.xml

Done.

https://codereview.chromium.org/2143893002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/app_list/win/app_list_service_win.cc:131:
shower().app_list()->SetNextPaintCallback(base::Bind(callback));
On 2016/07/20 01:15:34, tapted wrote:
> This is the only caller of SetNextPaintCallback() outside of
> app_list_view_unittest.cc - we should rewrite the test and delete
> SetNextPaintCallback (or at least move it to the test API).

Done.

https://codereview.chromium.org/2143893002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/app_list/win/app_list_service_win.cc:194:
local_state->GetInt64(prefs::kAppListLastLaunchTime);
On 2016/07/20 01:15:34, tapted wrote:
> This is the only getter, so this should be deleted from prefs::

Done.

https://codereview.chromium.org/2143893002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/app_list/win/app_list_service_win.cc:207: return
!shower().HasView() && !profile_loader().IsAnyProfileLoading();
On 2016/07/20 01:15:34, tapted wrote:
> hm - I don't think linux calls IsAnyProfileLoading(), but it's used a lot in
> tests. I guess this can wait until we delete Linux + ProfileLoader 

Acknowledged.

https://codereview.chromium.org/2143893002/diff/140001/ui/app_list/views/app_...
File ui/app_list/views/app_list_view.cc (right):

https://codereview.chromium.org/2143893002/diff/140001/ui/app_list/views/app_...
ui/app_list/views/app_list_view.cc:585: params->force_show_in_taskbar = true;
On 2016/07/20 01:15:34, tapted wrote:
> we should delete Widget::InitParams::force_show_in_taskbar when we purge this
on
> linux #reminder

Acknowledged.

[calamity, 2016-07-25 02:35:24]

Patchset #2 (id:160001) has been deleted

[tapted, 2016-07-26 01:34:42]

lgtm % nits

https://codereview.chromium.org/2143893002/diff/220001/ui/app_list/views/app_...
File ui/app_list/views/app_list_view.cc (right):

https://codereview.chromium.org/2143893002/diff/220001/ui/app_list/views/app_...
ui/app_list/views/app_list_view.cc:390: next_paint_callback_ = callback;
this can move to the test api; just view_->next_paint_callback_ = callback;

https://codereview.chromium.org/2143893002/diff/220001/ui/app_list/views/app_...
File ui/app_list/views/app_list_view.h (right):

https://codereview.chromium.org/2143893002/diff/220001/ui/app_list/views/app_...
ui/app_list/views/app_list_view.h:136: void SetNextPaintCallback(const
base::Closure& callback);
(remove - see later comment)

https://codereview.chromium.org/2143893002/diff/220001/ui/app_list/views/test...
File ui/app_list/views/test/app_list_view_test_api.cc (right):

https://codereview.chromium.org/2143893002/diff/220001/ui/app_list/views/test...
ui/app_list/views/test/app_list_view_test_api.cc:4: #include
"ui/app_list/views/test/app_list_view_test_api.h"
nit: blank line before

https://codereview.chromium.org/2143893002/diff/220001/ui/app_list/views/test...
File ui/app_list/views/test/app_list_view_test_api.h (right):

https://codereview.chromium.org/2143893002/diff/220001/ui/app_list/views/test...
ui/app_list/views/test/app_list_view_test_api.h:13: }  // namespace app_list
this should enclose AppListViewTestApi so we can remove some app_list:: prefixes
below. You might need to update the friend declaration (and perhaps also a
forward-declare in the same file)

https://codereview.chromium.org/2143893002/diff/220001/ui/app_list/views/test...
ui/app_list/views/test/app_list_view_test_api.h:21: virtual
~AppListViewTestApi();
this doesn't need to be virtual. But also it can probably be omitted completely
since the destructor is "trivial" - clang shouldn't complain

[calamity, 2016-07-26 08:38:39]

https://codereview.chromium.org/2143893002/diff/220001/ui/app_list/views/app_...
File ui/app_list/views/app_list_view.cc (right):

https://codereview.chromium.org/2143893002/diff/220001/ui/app_list/views/app_...
ui/app_list/views/app_list_view.cc:390: next_paint_callback_ = callback;
On 2016/07/26 01:34:42, tapted wrote:
> this can move to the test api; just view_->next_paint_callback_ = callback;

Done.

https://codereview.chromium.org/2143893002/diff/220001/ui/app_list/views/app_...
File ui/app_list/views/app_list_view.h (right):

https://codereview.chromium.org/2143893002/diff/220001/ui/app_list/views/app_...
ui/app_list/views/app_list_view.h:136: void SetNextPaintCallback(const
base::Closure& callback);
On 2016/07/26 01:34:42, tapted wrote:
> (remove - see later comment)

Done.

https://codereview.chromium.org/2143893002/diff/220001/ui/app_list/views/test...
File ui/app_list/views/test/app_list_view_test_api.cc (right):

https://codereview.chromium.org/2143893002/diff/220001/ui/app_list/views/test...
ui/app_list/views/test/app_list_view_test_api.cc:4: #include
"ui/app_list/views/test/app_list_view_test_api.h"
On 2016/07/26 01:34:42, tapted wrote:
> nit: blank line before

Done.

https://codereview.chromium.org/2143893002/diff/220001/ui/app_list/views/test...
File ui/app_list/views/test/app_list_view_test_api.h (right):

https://codereview.chromium.org/2143893002/diff/220001/ui/app_list/views/test...
ui/app_list/views/test/app_list_view_test_api.h:13: }  // namespace app_list
On 2016/07/26 01:34:42, tapted wrote:
> this should enclose AppListViewTestApi so we can remove some app_list::
prefixes
> below. You might need to update the friend declaration (and perhaps also a
> forward-declare in the same file)

Done.

https://codereview.chromium.org/2143893002/diff/220001/ui/app_list/views/test...
ui/app_list/views/test/app_list_view_test_api.h:21: virtual
~AppListViewTestApi();
On 2016/07/26 01:34:42, tapted wrote:
> this doesn't need to be virtual. But also it can probably be omitted
completely
> since the destructor is "trivial" - clang shouldn't complain

Done.

[calamity, 2016-07-28 03:20:22]

calamity@chromium.org changed reviewers:
+ asvitkine@chromium.org, grt@chromium.org, oshima@chromium.org

[calamity, 2016-07-28 03:20:23]

+oshima for ash/
+grt for chrome/installer/
+asvitkine for histograms.xml

PTAL, thanks!

[grt (UTC plus 2), 2016-07-28 10:24:20]

https://codereview.chromium.org/2143893002/diff/240001/chrome/browser/ui/view...
File chrome/browser/ui/views/app_list/win/app_list_service_win.cc (left):

https://codereview.chromium.org/2143893002/diff/240001/chrome/browser/ui/view...
chrome/browser/ui/views/app_list/win/app_list_service_win.cc:85:
GoogleUpdateSettings::UpdateDidRunStateForApp(
this means we can remove the code in the installer that created the version key
for this, correct (the call to AddAppLauncherVersionKeyWorkItems can be replaced
with RemoveAppLauncherVersionKey, and the former can be removed)? could you do
so in this CL?

[Alexei Svitkine (slow), 2016-07-28 13:46:30]

histograms.xml lgtm; didn't look at anything else

[oshima, 2016-07-28 22:54:48]

lgtm

[calamity, 2016-08-01 03:28:55]

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

[commit-bot: I haz the power, 2016-08-01 03:29:03]

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-08-01 04:36:57]

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

[commit-bot: I haz the power, 2016-08-01 04:36:58]

Dry run: This issue passed the CQ dry run.

[calamity, 2016-08-01 16:18:27]

https://codereview.chromium.org/2143893002/diff/240001/chrome/browser/ui/view...
File chrome/browser/ui/views/app_list/win/app_list_service_win.cc (left):

https://codereview.chromium.org/2143893002/diff/240001/chrome/browser/ui/view...
chrome/browser/ui/views/app_list/win/app_list_service_win.cc:85:
GoogleUpdateSettings::UpdateDidRunStateForApp(
On 2016/07/28 10:24:20, grt (UTC plus 2) wrote:
> this means we can remove the code in the installer that created the version
key
> for this, correct (the call to AddAppLauncherVersionKeyWorkItems can be
replaced
> with RemoveAppLauncherVersionKey, and the former can be removed)? could you do
> so in this CL?

Done.

[grt (UTC plus 2), 2016-08-01 21:01:01]

LGTM with one more piece of dead code removal. Thanks.

https://codereview.chromium.org/2143893002/diff/280001/chrome/installer/setup...
File chrome/installer/setup/install_worker.cc (right):

https://codereview.chromium.org/2143893002/diff/280001/chrome/installer/setup...
chrome/installer/setup/install_worker.cc:248:
RemoveAppLauncherVersionKey(installer_state.root_key());
Please remove the call to this in uninstall.cc, too.

[calamity, 2016-08-02 01:54:59]

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

[commit-bot: I haz the power, 2016-08-02 01:55:11]

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-08-02 03:00:59]

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

[commit-bot: I haz the power, 2016-08-02 03:01:00]

Dry run: This issue passed the CQ dry run.

[calamity, 2016-08-02 03:14:23]

https://codereview.chromium.org/2143893002/diff/280001/chrome/installer/setup...
File chrome/installer/setup/install_worker.cc (right):

https://codereview.chromium.org/2143893002/diff/280001/chrome/installer/setup...
chrome/installer/setup/install_worker.cc:248:
RemoveAppLauncherVersionKey(installer_state.root_key());
On 2016/08/01 21:01:00, grt (UTC plus 2) wrote:
> Please remove the call to this in uninstall.cc, too.

Done.

[calamity, 2016-08-02 03:14:34]

The CQ bit was checked by calamity@chromium.org

[calamity, 2016-08-02 03:14:35]

The patchset sent to the CQ was uploaded after l-g-t-m from tapted@chromium.org,
oshima@chromium.org, asvitkine@chromium.org, grt@chromium.org
Link to the patchset: https://codereview.chromium.org/2143893002/#ps300001
(title: "address comment")

[commit-bot: I haz the power, 2016-08-02 03:14:50]

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-08-02 03:21:57]

Committed patchset #6 (id:300001)

[commit-bot: I haz the power, 2016-08-02 03:25:58]

Description was changed from

==========
Purge the App Launcher code from Windows.

It hasn't been compiled for 1 milestone (since r394856).

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

to

==========
Purge the App Launcher code from Windows.

It hasn't been compiled for 1 milestone (since r394856).

BUG=600915

Committed: https://crrev.com/33d35e6839b107d29c8b2979aa628d964ce99589
Cr-Commit-Position: refs/heads/master@{#409132}
==========

[commit-bot: I haz the power, 2016-08-02 03:26:00]

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

[grt (UTC plus 2), 2016-08-02 07:28:22]

https://codereview.chromium.org/2143893002/diff/300001/chrome/installer/setup...
File chrome/installer/setup/uninstall.cc (right):

https://codereview.chromium.org/2143893002/diff/300001/chrome/installer/setup...
chrome/installer/setup/uninstall.cc:33: #include
"chrome/installer/setup/app_launcher_installer.h"
is this now unused? please remove if so.