Add an App Service Launch method

BUG=826982

Change-Id: I91dde844b443343a63ac099eece6e15e0b8b90fd
Reviewed-on: https://chromium-review.googlesource.com/c/1352128
Commit-Queue: Nigel Tao <nigeltao@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612040}

chrome/browser/apps/app_service/app_service_proxy.cc

@@ -68,6 +68,12 @@ void AppServiceProxy::LoadIcon(
   }
 }
 
+void AppServiceProxy::Launch(const std::string& app_id, int32_t event_flags) {
+  cache_.ForOneApp(app_id, [this, event_flags](const apps::AppUpdate& update) {
+    app_service_->Launch(update.AppType(), update.AppId(), event_flags);
+  });
+}
+
 void AppServiceProxy::OnApps(std::vector<apps::mojom::AppPtr> deltas) {
   cache_.OnApps(std::move(deltas));
 }

chrome/browser/apps/app_service/app_service_proxy.h

@@ -41,6 +41,8 @@ class AppServiceProxy : public KeyedService, public apps::mojom::Subscriber {
                 int32_t size_hint_in_dip,
                 apps::mojom::Publisher::LoadIconCallback callback);
 
+  void Launch(const std::string& app_id, int32_t event_flags);
+
  private:
   // apps::mojom::Subscriber overrides.
   void OnApps(std::vector<apps::mojom::AppPtr> deltas) override;

chrome/browser/apps/app_service/built_in_chromeos_apps.cc

@@ -96,4 +96,9 @@ void BuiltInChromeOsApps::LoadIcon(
   std::move(callback).Run(apps::mojom::IconValue::New());
 }
 
+void BuiltInChromeOsApps::Launch(const std::string& app_id,
+                                 int32_t event_flags) {
+  app_list::OpenInternalApp(app_id, profile_, event_flags);
+}
+
 }  // namespace apps

chrome/browser/apps/app_service/built_in_chromeos_apps.h

@@ -33,6 +33,7 @@ class BuiltInChromeOsApps : public apps::mojom::Publisher {
                 apps::mojom::IconCompression icon_compression,
                 int32_t size_hint_in_dip,
                 LoadIconCallback callback) override;
+  void Launch(const std::string& app_id, int32_t event_flags) override;
 
   mojo::Binding<apps::mojom::Publisher> binding_;
   Profile* profile_;

chrome/browser/ui/app_list/app_service_app_item.cc

@@ -7,6 +7,7 @@
 #include "ash/public/cpp/app_list/app_list_config.h"
 #include "base/bind.h"
 #include "chrome/browser/apps/app_service/app_service_proxy.h"
+#include "chrome/browser/ui/app_list/internal_app/internal_app_item.h"
 
 // static
 const char AppServiceAppItem::kItemType[] = "AppServiceAppItem";
@@ -16,7 +17,8 @@ AppServiceAppItem::AppServiceAppItem(
     AppListModelUpdater* model_updater,
     const app_list::AppListSyncableService::SyncItem* sync_item,
     const apps::AppUpdate& app_update)
-    : ChromeAppListItem(profile, app_update.AppId()) {
+    : ChromeAppListItem(profile, app_update.AppId()),
+      app_type_(app_update.AppType()) {
   SetName(app_update.Name());
   apps::AppServiceProxy* proxy = apps::AppServiceProxy::Get(profile);
   if (proxy) {
@@ -43,6 +45,30 @@ AppServiceAppItem::AppServiceAppItem(
 
 AppServiceAppItem::~AppServiceAppItem() = default;
 
+void AppServiceAppItem::Activate(int event_flags) {
+  // This code snippet makes us update the same UMA histogram both before and
+  // after --enable-features=AppService. Before, it was updated in
+  // internal_app_item.h, which meant that there were histogram entries:
+  //   - only for internal apps, not for other app types.
+  //   - only when launched from the app list, and not from e.g. the shelf.
+  //     Keeping this behavior means that, even after the App Service is
+  //     enabled, the histogram is updated in this UI code rather than by the
+  //     App Service's app publisher, since the publisher can also be invoked
+  //     from other UIs like the shelf.
+  //
+  // TODO(crbug.com/826982): be more structured about UMA histograms, after we
+  // migrate more of the ui/app_list code over to use the App Service. Should
+  // they apply to all app types? Should they apply to all the different ways
+  // we can launch apps? Should we just delete the
+  // UMA_HISTOGRAM_ENUMERATION("Apps.AppListInternalApp.Activate", etc) code?
+  if (app_type_ == apps::mojom::AppType::kBuiltIn)
+    InternalAppItem::RecordActiveHistogram(id());
+
+  apps::AppServiceProxy* proxy = apps::AppServiceProxy::Get(profile());
+  if (proxy)
+    proxy->Launch(id(), event_flags);
+}
+
 const char* AppServiceAppItem::GetItemType() const {
   return AppServiceAppItem::kItemType;
 }

chrome/browser/ui/app_list/app_service_app_item.h

@@ -9,6 +9,7 @@
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/ui/app_list/chrome_app_list_item.h"
 #include "chrome/services/app_service/public/cpp/app_update.h"
+#include "chrome/services/app_service/public/mojom/types.mojom.h"
 
 class AppServiceAppItem : public ChromeAppListItem {
  public:
@@ -22,11 +23,14 @@ class AppServiceAppItem : public ChromeAppListItem {
 
  private:
   // ChromeAppListItem overrides:
+  void Activate(int event_flags) override;
   const char* GetItemType() const override;
-  // TODO(crbug.com/826982): Activate, GetContextMenuModel, etc.
+  // TODO(crbug.com/826982): GetContextMenuModel, etc.
 
   void OnLoadIcon(apps::mojom::IconValuePtr icon_value);
 
+  apps::mojom::AppType app_type_;
+
   base::WeakPtrFactory<AppServiceAppItem> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(AppServiceAppItem);

chrome/browser/ui/app_list/internal_app/internal_app_item.cc

@@ -13,7 +13,7 @@
 
 namespace {
 
-void RecordActiveHistogram(app_list::InternalAppName name) {
+void RecordActiveHistogramInternal(app_list::InternalAppName name) {
   UMA_HISTOGRAM_ENUMERATION("Apps.AppListInternalApp.Activate", name);
 }
 
@@ -22,6 +22,11 @@ void RecordActiveHistogram(app_list::InternalAppName name) {
 // static
 const char InternalAppItem::kItemType[] = "InternalAppItem";
 
+// static
+void InternalAppItem::RecordActiveHistogram(const std::string& app_id) {
+  RecordActiveHistogramInternal(app_list::GetInternalAppNameByAppId(app_id));
+}
+
 InternalAppItem::InternalAppItem(
     Profile* profile,
     AppListModelUpdater* model_updater,
@@ -45,7 +50,7 @@ const char* InternalAppItem::GetItemType() const {
 }
 
 void InternalAppItem::Activate(int event_flags) {
-  RecordActiveHistogram(app_list::GetInternalAppNameByAppId(id()));
+  RecordActiveHistogram(id());
   app_list::OpenInternalApp(id(), profile(), event_flags);
 }
 

chrome/browser/ui/app_list/internal_app/internal_app_item.h

@@ -18,6 +18,8 @@ class InternalAppItem : public ChromeAppListItem {
  public:
   static const char kItemType[];
 
+  static void RecordActiveHistogram(const std::string& app_id);
+
   InternalAppItem(Profile* profile,
                   AppListModelUpdater* model_updater,
                   const app_list::AppListSyncableService::SyncItem* sync_item,

chrome/services/app_service/app_service_impl.cc

@@ -77,6 +77,16 @@ void AppServiceImpl::LoadIcon(apps::mojom::AppType app_type,
                          size_hint_in_dip, std::move(callback));
 }
 
+void AppServiceImpl::Launch(apps::mojom::AppType app_type,
+                            const std::string& app_id,
+                            int32_t event_flags) {
+  auto iter = publishers_.find(app_type);
+  if (iter == publishers_.end()) {
+    return;
+  }
+  iter->second->Launch(app_id, event_flags);
+}
+
 void AppServiceImpl::OnPublisherDisconnected(apps::mojom::AppType app_type) {
   publishers_.erase(app_type);
 }

chrome/services/app_service/app_service_impl.h

@@ -37,6 +37,9 @@ class AppServiceImpl : public apps::mojom::AppService {
                 apps::mojom::IconCompression icon_compression,
                 int32_t size_hint_in_dip,
                 LoadIconCallback callback) override;
+  void Launch(apps::mojom::AppType app_type,
+              const std::string& app_id,
+              int32_t event_flags) override;
 
  private:
   void OnPublisherDisconnected(apps::mojom::AppType app_type);

chrome/services/app_service/app_service_impl_unittest.cc

@@ -54,6 +54,8 @@ class FakePublisher : public apps::mojom::Publisher {
     std::move(callback).Run(apps::mojom::IconValue::New());
   }
 
+  void Launch(const std::string& app_id, int32_t event_flags) override {}
+
   void CallOnApps(apps::mojom::Subscriber* subscriber,
                   std::vector<std::string>& app_ids) {
     std::vector<apps::mojom::AppPtr> apps;

chrome/services/app_service/public/mojom/app_service.mojom

@@ -25,6 +25,9 @@ interface AppService {
       IconKey icon_key,
       IconCompression icon_compression,
       int32 size_hint_in_dip) => (IconValue icon_value);
+
+  // App Runner methods.
+  Launch(AppType app_type, string app_id, int32 event_flags);
 };
 
 interface Publisher {
@@ -37,6 +40,9 @@ interface Publisher {
       IconKey icon_key,
       IconCompression icon_compression,
       int32 size_hint_in_dip) => (IconValue icon_value);
+
+  // App Runner methods.
+  Launch(string app_id, int32 event_flags);
 };
 
 interface Subscriber {