mash: Have chrome set itself as the app list presenter.

Do not let ash connect to chrome to get the presenter.

Expose mojom::AppList interface in ash to set the presenter.
Chrome's AppListPresenterService connects to register.
Move instance from Chrome's FactoryImpl to AppListServiceAsh.

Implement app_list::AppList; owned by ash::WmShell.

Add export info to app_list's 'presenter' component.
(lets it add the mojom as a public DEPS; thanks Yuzhu!)

Remove connection and interface pointer from AppListPresenterMus.

TODO: What's not TODO here? It's a mess!

BUG=670775
TEST=No regressions in chrome --mash app list behavior
R=jamescook@chromium.org,tsepez@chromium.org,xiyuan@chromium.org,rockot@chromium.org

Review-Url: https://codereview.chromium.org/2567523002
Cr-Commit-Position: refs/heads/master@{#438044}

ash/common/mojo_interface_factory.cc

@@ -11,6 +11,7 @@
 #include "ash/common/new_window_controller.h"
 #include "ash/common/session/session_controller.h"
 #include "ash/common/shelf/shelf_controller.h"
+#include "ash/common/shell_delegate.h"
 #include "ash/common/shutdown_controller.h"
 #include "ash/common/system/locale/locale_notification_controller.h"
 #include "ash/common/system/tray/system_tray_controller.h"
@@ -19,6 +20,7 @@
 #include "ash/common/wm_shell.h"
 #include "base/bind.h"
 #include "services/service_manager/public/cpp/interface_registry.h"
+#include "ui/app_list/presenter/app_list.h"
 
 #if defined(OS_CHROMEOS)
 #include "ash/common/system/chromeos/network/vpn_list.h"
@@ -33,6 +35,10 @@ void BindAcceleratorControllerRequestOnMainThread(
   WmShell::Get()->accelerator_controller()->BindRequest(std::move(request));
 }
 
+void BindAppListRequestOnMainThread(app_list::mojom::AppListRequest request) {
+  WmShell::Get()->app_list()->BindRequest(std::move(request));
+}
+
 void BindCastConfigOnMainThread(mojom::CastConfigRequest request) {
   WmShell::Get()->cast_config()->BindRequest(std::move(request));
 }
@@ -91,6 +97,8 @@ void RegisterInterfaces(
   registry->AddInterface(
       base::Bind(&BindAcceleratorControllerRequestOnMainThread),
       main_thread_task_runner);
+  registry->AddInterface(base::Bind(&BindAppListRequestOnMainThread),
+                         main_thread_task_runner);
   registry->AddInterface(base::Bind(&BindCastConfigOnMainThread),
                          main_thread_task_runner);
   registry->AddInterface(

ash/common/wm_shell.cc

@@ -47,6 +47,7 @@
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
+#include "ui/app_list/presenter/app_list.h"
 #include "ui/app_list/presenter/app_list_presenter.h"
 #include "ui/display/display.h"
 #include "ui/views/focus/focus_manager_factory.h"
@@ -249,6 +250,7 @@ void WmShell::SetPaletteDelegateForTesting(
 
 WmShell::WmShell(std::unique_ptr<ShellDelegate> shell_delegate)
     : delegate_(std::move(shell_delegate)),
+      app_list_(base::MakeUnique<app_list::AppList>()),
       cast_config_(base::MakeUnique<CastConfigController>()),
       focus_cycler_(base::MakeUnique<FocusCycler>()),
       immersive_context_(base::MakeUnique<ImmersiveContextAsh>()),

ash/common/wm_shell.h

@@ -21,6 +21,10 @@
 #include "ui/compositor/layer_type.h"
 #include "ui/wm/public/window_types.h"
 
+namespace app_list {
+class AppList;
+}
+
 namespace base {
 class SequencedWorkerPool;
 }
@@ -117,6 +121,8 @@ class ASH_EXPORT WmShell {
     return accessibility_delegate_.get();
   }
 
+  app_list::AppList* app_list() { return app_list_.get(); }
+
   BrightnessControlDelegate* brightness_control_delegate() {
     return brightness_control_delegate_.get();
   }
@@ -481,6 +487,7 @@ class ASH_EXPORT WmShell {
 
   std::unique_ptr<AcceleratorController> accelerator_controller_;
   std::unique_ptr<AccessibilityDelegate> accessibility_delegate_;
+  std::unique_ptr<app_list::AppList> app_list_;
   std::unique_ptr<BrightnessControlDelegate> brightness_control_delegate_;
   std::unique_ptr<CastConfigController> cast_config_;
   std::unique_ptr<FocusCycler> focus_cycler_;

ash/mus/app_list_presenter_mus.cc

@@ -4,37 +4,34 @@
 
 #include "ash/mus/app_list_presenter_mus.h"
 
-#include "content/public/common/service_names.mojom.h"
-#include "services/service_manager/public/cpp/connector.h"
+#include "ash/common/wm_shell.h"
+#include "ui/app_list/presenter/app_list.h"
 
 namespace ash {
 
-namespace {
-
-bool HasConnection(app_list::mojom::AppListPresenterPtr* interface_ptr) {
-  return interface_ptr->is_bound() && !interface_ptr->encountered_error();
-}
-
-}  // namespace
-
-AppListPresenterMus::AppListPresenterMus(service_manager::Connector* connector)
-    : connector_(connector) {}
+AppListPresenterMus::AppListPresenterMus() {}
 
 AppListPresenterMus::~AppListPresenterMus() {}
 
 void AppListPresenterMus::Show(int64_t display_id) {
-  ConnectIfNeeded();
-  presenter_->Show(display_id);
+  app_list::mojom::AppListPresenter* app_list_presenter =
+      WmShell::Get()->app_list()->GetAppListPresenter();
+  if (app_list_presenter)
+    app_list_presenter->Show(display_id);
 }
 
 void AppListPresenterMus::Dismiss() {
-  ConnectIfNeeded();
-  presenter_->Dismiss();
+  app_list::mojom::AppListPresenter* app_list_presenter =
+      WmShell::Get()->app_list()->GetAppListPresenter();
+  if (app_list_presenter)
+    app_list_presenter->Dismiss();
 }
 
 void AppListPresenterMus::ToggleAppList(int64_t display_id) {
-  ConnectIfNeeded();
-  presenter_->ToggleAppList(display_id);
+  app_list::mojom::AppListPresenter* app_list_presenter =
+      WmShell::Get()->app_list()->GetAppListPresenter();
+  if (app_list_presenter)
+    app_list_presenter->ToggleAppList(display_id);
 }
 
 bool AppListPresenterMus::IsVisible() const {
@@ -49,13 +46,4 @@ bool AppListPresenterMus::GetTargetVisibility() const {
   return false;
 }
 
-void AppListPresenterMus::ConnectIfNeeded() {
-  if (!connector_ || HasConnection(&presenter_))
-    return;
-  connector_->ConnectToInterface(content::mojom::kBrowserServiceName,
-                                 &presenter_);
-  CHECK(HasConnection(&presenter_))
-      << "Could not connect to app_list::mojom::AppListPresenter.";
-}
-
 }  // namespace ash

ash/mus/app_list_presenter_mus.h

@@ -7,19 +7,14 @@
 
 #include "base/macros.h"
 #include "ui/app_list/presenter/app_list_presenter.h"
-#include "ui/app_list/presenter/app_list_presenter.mojom.h"
-
-namespace service_manager {
-class Connector;
-}
 
 namespace ash {
 
 // Mus+ash implementation of the AppListPresenter interface for mash, which
-// talks to the app list service in chrome.
+// talks to the app list presenter service in chrome.
 class AppListPresenterMus : public app_list::AppListPresenter {
  public:
-  explicit AppListPresenterMus(service_manager::Connector* connector);
+  AppListPresenterMus();
   ~AppListPresenterMus() override;
 
   // app_list::AppListPresenter:
@@ -30,13 +25,6 @@ class AppListPresenterMus : public app_list::AppListPresenter {
   bool GetTargetVisibility() const override;
 
  private:
-  // Connect to the app list service in chrome if the connection hasn't
-  // been established or has an error.
-  void ConnectIfNeeded();
-
-  service_manager::Connector* connector_;
-  app_list::mojom::AppListPresenterPtr presenter_;
-
   DISALLOW_COPY_AND_ASSIGN(AppListPresenterMus);
 };
 

ash/mus/manifest.json

@@ -7,6 +7,7 @@
         // Modifications here should correspond with changes to
         // chrome_content_browser_manifest_overlay.json.
         "ash": [
+          "app_list::mojom::AppList",
           "ash::mojom::AcceleratorController",
           "ash::mojom::CastConfig",
           "ash::mojom::LocaleNotificationController",

ash/mus/shell_delegate_mus.cc

@@ -19,7 +19,6 @@
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "components/user_manager/user_info_impl.h"
-#include "ui/app_list/presenter/app_list_presenter.h"
 #include "ui/gfx/image/image.h"
 
 #if defined(OS_CHROMEOS)
@@ -110,7 +109,7 @@ class MediaDelegateStub : public MediaDelegate {
 }  // namespace
 
 ShellDelegateMus::ShellDelegateMus(service_manager::Connector* connector)
-    : connector_(connector), app_list_presenter_(connector) {
+    : connector_(connector) {
   // |connector_| may be null in tests.
 }
 

chrome/browser/chrome_content_browser_manifest_overlay.json

@@ -20,6 +20,7 @@
         "ash": [
           // Under classic ash the browser provides some of the ash interfaces
           // to itself.
+          "app_list::mojom::AppList",
           "ash::mojom::AcceleratorController",
           "ash::mojom::CastConfig",
           "ash::mojom::LocaleNotificationController",

chrome/browser/chromeos/chrome_interface_factory.cc

@@ -16,7 +16,6 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
-#include "chrome/browser/ui/ash/app_list/app_list_presenter_service.h"
 #include "chrome/browser/ui/ash/ash_util.h"
 #include "chrome/browser/ui/ash/keyboard_ui_service.h"
 #include "chrome/browser/ui/browser_commands.h"
@@ -26,7 +25,6 @@
 #include "mojo/public/cpp/bindings/binding_set.h"
 #include "services/service_manager/public/cpp/connection.h"
 #include "services/service_manager/public/cpp/interface_registry.h"
-#include "ui/app_list/presenter/app_list_presenter.mojom.h"
 #include "ui/keyboard/keyboard.mojom.h"
 
 namespace chromeos {
@@ -113,21 +111,11 @@ class FactoryImpl {
     launchable_->ProcessRequest(std::move(request));
   }
 
-  void BindRequest(app_list::mojom::AppListPresenterRequest request) {
-    if (!app_list_presenter_service_)
-      app_list_presenter_service_ = base::MakeUnique<AppListPresenterService>();
-    app_list_presenter_bindings_.AddBinding(app_list_presenter_service_.get(),
-                                            std::move(request));
-  }
-
   static base::LazyInstance<std::unique_ptr<FactoryImpl>>::Leaky factory_;
 
   std::unique_ptr<KeyboardUIService> keyboard_ui_service_;
   mojo::BindingSet<keyboard::mojom::Keyboard> keyboard_bindings_;
   std::unique_ptr<ChromeLaunchable> launchable_;
-  std::unique_ptr<AppListPresenterService> app_list_presenter_service_;
-  mojo::BindingSet<app_list::mojom::AppListPresenter>
-      app_list_presenter_bindings_;
 
   DISALLOW_COPY_AND_ASSIGN(FactoryImpl);
 };
@@ -153,8 +141,6 @@ bool ChromeInterfaceFactory::OnConnect(
                                                      main_thread_task_runner_);
   FactoryImpl::AddFactory<mash::mojom::Launchable>(registry,
                                                    main_thread_task_runner_);
-  FactoryImpl::AddFactory<app_list::mojom::AppListPresenter>(
-      registry, main_thread_task_runner_);
 
   // In classic ash, the browser process provides ash services to itself.
   if (!chrome::IsRunningInMash()) {

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

@@ -5,9 +5,24 @@
 #include "chrome/browser/ui/ash/app_list/app_list_presenter_service.h"
 
 #include "chrome/browser/ui/ash/app_list/app_list_service_ash.h"
+#include "chrome/browser/ui/ash/ash_util.h"
+#include "content/public/common/service_manager_connection.h"
+#include "services/service_manager/public/cpp/connector.h"
 #include "ui/app_list/presenter/app_list_presenter.h"
 
-AppListPresenterService::AppListPresenterService() {}
+AppListPresenterService::AppListPresenterService() : binding_(this) {
+  content::ServiceManagerConnection* connection =
+      content::ServiceManagerConnection::GetForProcess();
+  if (connection && connection->GetConnector()) {
+    // Connect to the app list interface in the ash service.
+    app_list::mojom::AppListPtr app_list_ptr;
+    connection->GetConnector()->ConnectToInterface(
+        ash_util::GetAshServiceName(), &app_list_ptr);
+    // Register this object as the app list presenter.
+    app_list_ptr->SetAppListPresenter(binding_.CreateInterfacePtrAndBind());
+  }
+}
+
 AppListPresenterService::~AppListPresenterService() {}
 
 void AppListPresenterService::Show(int64_t display_id) {

chrome/browser/ui/ash/app_list/app_list_presenter_service.h

@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_UI_ASH_APP_LIST_APP_LIST_PRESENTER_SERVICE_H_
 
 #include "base/macros.h"
+#include "mojo/public/cpp/bindings/binding.h"
 #include "ui/app_list/presenter/app_list_presenter.mojom.h"
 
 namespace app_list {
@@ -26,6 +27,8 @@ class AppListPresenterService : public app_list::mojom::AppListPresenter {
  private:
   app_list::AppListPresenter* GetPresenter();
 
+  mojo::Binding<app_list::mojom::AppListPresenter> binding_;
+
   DISALLOW_COPY_AND_ASSIGN(AppListPresenterService);
 };
 

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

@@ -9,6 +9,7 @@
 
 #include "base/macros.h"
 #include "chrome/browser/ui/app_list/app_list_service_impl.h"
+#include "chrome/browser/ui/ash/app_list/app_list_presenter_service.h"
 #include "ui/app_list/app_list_model.h"
 
 namespace app_list {
@@ -76,6 +77,8 @@ class AppListServiceAsh : public AppListServiceImpl {
   std::unique_ptr<app_list::AppListPresenterImpl> app_list_presenter_;
   std::unique_ptr<AppListControllerDelegateAsh> controller_delegate_;
 
+  AppListPresenterService app_list_presenter_service_;
+
   DISALLOW_COPY_AND_ASSIGN(AppListServiceAsh);
 };
 

ui/app_list/DEPS

 include_rules = [
   "+components/keyed_service/core",
   "+components/sync",
+  "+mojo/public/cpp",
   "+skia",
   "+third_party/google_toolbox_for_mac/src",
   "+third_party/skia",

ui/app_list/presenter/BUILD.gn

@@ -8,8 +8,23 @@ import("//testing/test.gni")
 
 assert(use_aura)
 
+mojom("mojom") {
+  sources = [
+    "app_list_presenter.mojom",
+  ]
+  deps = [
+    "//services/ui/public/interfaces",
+  ]
+
+  export_class_attribute = "APP_LIST_PRESENTER_EXPORT"
+  export_define = "APP_LIST_PRESENTER_IMPLEMENTATION=1"
+  export_header = "ui/app_list/presenter/app_list_presenter_export.h"
+}
+
 component("presenter") {
   sources = [
+    "app_list.cc",
+    "app_list.h",
     "app_list_presenter.h",
     "app_list_presenter_delegate.cc",
     "app_list_presenter_delegate.h",
@@ -24,7 +39,9 @@ component("presenter") {
   defines = [ "APP_LIST_PRESENTER_IMPLEMENTATION" ]
 
   public_deps = [
+    ":mojom",
     "//base",
+    "//mojo/public/cpp/bindings",
     "//ui/app_list",
     "//ui/aura",
     "//ui/compositor",
@@ -37,15 +54,6 @@ component("presenter") {
   ]
 }
 
-mojom("mojom") {
-  sources = [
-    "app_list_presenter.mojom",
-  ]
-  deps = [
-    "//services/ui/public/interfaces",
-  ]
-}
-
 static_library("test_support") {
   sources = [
     "test/app_list_presenter_impl_test_api.cc",

ui/app_list/presenter/app_list.cc

+// Copyright 2016 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 "ui/app_list/presenter/app_list.h"
+
+namespace app_list {
+
+AppList::AppList() {}
+
+AppList::~AppList() {}
+
+void AppList::BindRequest(mojom::AppListRequest request) {
+  bindings_.AddBinding(this, std::move(request));
+}
+
+mojom::AppListPresenter* AppList::GetAppListPresenter() {
+  return presenter_.get();
+}
+
+void AppList::SetAppListPresenter(mojom::AppListPresenterPtr presenter) {
+  presenter_ = std::move(presenter);
+}
+
+}  // namespace app_list

ui/app_list/presenter/app_list.h

+// Copyright 2016 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 UI_APP_LIST_PRESENTER_APP_LIST_H_
+#define UI_APP_LIST_PRESENTER_APP_LIST_H_
+
+#include "mojo/public/cpp/bindings/binding_set.h"
+#include "ui/app_list/presenter/app_list_presenter.mojom.h"
+#include "ui/app_list/presenter/app_list_presenter_export.h"
+
+namespace app_list {
+
+// Stores the app list presenter interface pointer.
+class APP_LIST_PRESENTER_EXPORT AppList : public mojom::AppList {
+ public:
+  AppList();
+  ~AppList() override;
+
+  // Binds the mojom::AppList interface request to this object.
+  void BindRequest(mojom::AppListRequest request);
+
+  // Get a raw pointer to the mojom::AppListPresenter interface; may be null.
+  mojom::AppListPresenter* GetAppListPresenter();
+
+  // mojom::AppList:
+  void SetAppListPresenter(mojom::AppListPresenterPtr presenter) override;
+
+ private:
+  // Bindings for the mojom::AppList interface.
+  mojo::BindingSet<mojom::AppList> bindings_;
+
+  // App list presenter interface in chrome; used to show/hide the app list.
+  mojom::AppListPresenterPtr presenter_;
+
+  DISALLOW_COPY_AND_ASSIGN(AppList);
+};
+
+}  // namespace app_list
+
+#endif  // UI_APP_LIST_PRESENTER_APP_LIST_H_

ui/app_list/presenter/app_list_presenter.mojom

@@ -6,6 +6,17 @@ module app_list.mojom;
 
 import "services/ui/public/interfaces/window_manager_constants.mojom";
 
+// TODO(msw): Rename this file to app_list.mojom; move to ash?
+// TODO(msw): Ash should implement the app list and presenter; chrome should
+//            just push data about its apps into the app list interface.
+
+// Implemented by ash. Used by chrome to set the presenter interface.
+interface AppList {
+  // Set the app list presenter interface, to let ash trigger Chrome's app list.
+  SetAppListPresenter(AppListPresenter presenter);
+};
+
+// Implemented by chrome. Used by ash to actually show and dismiss the app list.
 interface AppListPresenter {
   // Show the app list on the specified display.
   Show(int64 display_id);