Refactor app_list_util.h into AppListService abstract base.

chrome/browser/ui/app_list_service.h

+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_UI_APP_LIST_SERVICE_H_
+#define CHROME_BROWSER_UI_APP_LIST_SERVICE_H_
+
+#include <string>
+
+#include "base/basictypes.h"
+
+class Profile;
+class PrefRegistrySimple;
+class Profile;
+
+namespace base {
+class FilePath;
+}
+
+namespace gfx {
+class ImageSkia;
+}
+
+class AppListService {
+ public:
+  static AppListService* Get();

[benwells, 2013/02/11 04:12:32]

So this would become GetForLastActiveDesktop at some point? And Get take a
desktop as well?

[tapted, 2013/02/11 05:35:11]

yep!

+  static AppListService* GetDisabled();

[benwells, 2013/02/11 04:12:32]

What is GetDisabled for? Unless it's obvious and I'm just being obtuse, it
probably should be commented.

[tapted, 2013/02/11 05:35:11]

It's the Singleton for AppListService(). i.e. the "dummy" AppListService, which
does nothing.

[benwells, 2013/02/11 05:52:36]

I think we should organize the code so it isn't on the public interface.

[tapted, 2013/02/18 07:05:39]

Done.

+
+  static base::FilePath GetAppListProfilePath(
+      const base::FilePath& user_data_dir);
+
+  static void RegisterAppListPrefs(PrefRegistrySimple* registry);
+
+  // Do any once off initialization needed for the app list.
+  virtual void Init(Profile* initial_profile);
+
+  // Show the app list.
+  virtual void ShowAppList(Profile* profile);

[tapted, 2013/02/11 04:04:20]

probably just 'Show()' is fine, same for all the other functions --
's/AppList//g'

[benwells, 2013/02/11 05:52:36]

sgtm

[tapted, 2013/02/19 03:15:32]

I've changed my mind about this... since the usage sounds like we're showing
"the service" which doesn't make sense. Some of the others might make sense to
change though, like the static functions, WDYT?

+
+  // Change the profile that the app list is showing.
+  virtual void SetAppListProfile(const base::FilePath& profile_file_path);
+
+  // Dismiss the app list.
+  virtual void DismissAppList();

[benwells, 2013/02/11 04:12:32]

Can we move the test only stuff into private, and let the tests get to it via
friending?

[tapted, 2013/02/11 05:35:11]

"maybe"? Maintaing friends could get fiddly/fragile if we reach a situation
where we need lots of test fixtures. Maybe a public interface exposed here, and
a app_list_service_internal that exposes the interface that actually gets
implemented (which includes tests?)?

[benwells, 2013/02/11 05:52:36]

Maybe if we just keep all the testing only functions together, with a comment
that they are just for testing. I'd just like to see it made clear somehow.

[tapted, 2013/02/19 03:15:32]

IsAppListVisible now finds itself in non-test code for ash. I think the only
test-only function is now GetCurrentAppListProfile, but there is a TODO that
might make it relevant for non-test code..

+
+  // Get the profile the app list is currently showing.
+  virtual Profile* GetCurrentAppListProfile();
+
+  // Returns true if the app list is visible.
+  virtual bool IsAppListVisible();
+
+  // Notify the app list that an extension has started downloading.
+  virtual void NotifyAppListOfBeginExtensionInstall(
+      Profile* profile,
+      const std::string& extension_id,
+      const std::string& extension_name,
+      const gfx::ImageSkia& installing_icon);
+
+  virtual void NotifyAppListOfDownloadProgress(
+      Profile* profile,
+      const std::string& extension_id,
+      int percent_downloaded);
+
+ protected:
+  AppListService() {}
+  virtual ~AppListService() {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(AppListService);
+};
+
+#endif  // CHROME_BROWSER_UI_APP_LIST_SERVICE_H_