Refactor app_list_util.h into AppListService abstract base.

chrome/browser/ui/ash/app_list/app_list_service_ash.cc

Index: chrome/browser/ui/ash/app_list/app_list_service_ash.cc
diff --git a/chrome/browser/ui/ash/app_list/app_list_service_ash.cc b/chrome/browser/ui/ash/app_list/app_list_service_ash.cc
new file mode 100644
index 0000000000000000000000000000000000000000..2918b1dc1deb996727dce12c5a3e0b8cfe99d448
--- /dev/null
+++ b/chrome/browser/ui/ash/app_list/app_list_service_ash.cc
@@ -0,0 +1,57 @@
+// 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.
+
+#include "chrome/browser/ui/app_list/app_list_service_ash.h"
+
+#include "ash/shell.h"
+#include "base/memory/singleton.h"
+#include "chrome/browser/ui/app_list/app_list_service.h"

[tfarina, 2013/02/21 23:04:44]

This gets really confusing, you are inheriting from AppListService, but the main
file is chrome/browser/ui/app_list/app_list_service_ash.h.

There are some examples of how to this in a more cross-platform way, but I don't
recall from head right now :/

[tapted, 2013/02/25 07:09:21]

This approach is mainly to keep DEPS happy. I think a follow-up will move a lot
of this to /apps, so the organisation will be a bit neater.

+
+namespace {
+
+class AppListServiceAsh : public AppListService {
+ public:
+  static AppListServiceAsh* GetInstance() {
+    return Singleton<AppListServiceAsh>.get();

[tfarina, 2013/02/21 23:04:44]

is this what we want? Why we can't make someone own AppListService instead?

[tapted, 2013/02/25 07:09:21]

I don't think it makes sense for AppListService to have an owner ("yet").

It is a service like PrefService, MetricsService, etc. Those are owned by
g_browser_process, but we don't want to couple ourselves to the browser. We also
can't attach it to a profile, since it needs ways to switch profiles. (unlike,
e.g., ExtensionService+System, which is a per-profile).

+  }
+
+  // AppListService overrides:

[tfarina, 2013/02/21 23:04:44]

can be private?

[tapted, 2013/02/25 07:09:21]

Done.

+  virtual void ShowAppList(Profile* default_profile) OVERRIDE;
+  virtual bool IsAppListVisible() const OVERRIDE;
+  virtual void DismissAppList() OVERRIDE;
+
+ private:
+  AppListServiceAsh() {}
+
+  friend struct DefaultSingletonTraits<AppListServiceAsh>;

[tfarina, 2013/02/21 23:04:44]

this comes first, it should be the first thing in the private section.

[tapted, 2013/02/25 07:09:21]

Hmm - the guideline is pretty ambiguous about these, and the pattern above is a
better match for the boilerplate in base/memory/singleton.h. But I'm not fussed
- I think 'friends first' wins out in a git-grep-fight. Done.

+
+  DISALLOW_COPY_AND_ASSIGN(AppListServiceAsh);
+};
+
+void AppListServiceAsh::ShowAppList(Profile* default_profile) {
+  // This may not work correctly if the profile passed in is different from the
+  // one the ash Shell is currently using.
+  // TODO(ananta): Handle profile changes correctly when !defined(OS_CHROMEOS).
+  if (!IsAppListVisible())
+    ash::Shell::GetInstance()->ToggleAppList(NULL);
+}
+
+bool AppListServiceAsh::IsAppListVisible() const {
+  return ash::Shell::GetInstance()->GetAppListTargetVisibility();
+}
+
+void AppListServiceAsh::DismissAppList() {
+  if (IsAppListVisible())
+    ash::Shell::GetInstance()->ToggleAppList(NULL);
+}
+
+}  // namespace
+
+namespace chrome {
+
+AppListService* GetAppListServiceAsh() {
+  return AppListServiceAsh::GetInstance();
+}
+
+}

[tfarina, 2013/02/21 23:04:44]

// namespace chrome

[tapted, 2013/02/25 07:09:21]

Done.