Add an is_platform_app bit to the App Service

BUG=826982

Change-Id: I12fd7ed12431a8d89c0fd3f7070cbeb8b1bd3ed2
Reviewed-on: https://chromium-review.googlesource.com/c/1455837
Commit-Queue: Nigel Tao <nigeltao@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#629811}

chrome/browser/apps/app_service/arc_apps.cc

@@ -400,6 +400,8 @@ apps::mojom::AppPtr ArcApps::Convert(const std::string& app_id,
                                   ? apps::mojom::OptionalBool::kTrue
                                   : apps::mojom::OptionalBool::kFalse;
 
+  app->is_platform_app = apps::mojom::OptionalBool::kFalse;
+
   auto show = app_info.show_in_launcher ? apps::mojom::OptionalBool::kTrue
                                         : apps::mojom::OptionalBool::kFalse;
   app->show_in_launcher = show;

chrome/browser/apps/app_service/built_in_chromeos_apps.cc

@@ -42,6 +42,7 @@ apps::mojom::AppPtr Convert(const app_list::InternalApp& internal_app) {
   app->install_time = base::Time();
 
   app->installed_internally = apps::mojom::OptionalBool::kTrue;
+  app->is_platform_app = apps::mojom::OptionalBool::kFalse;
   app->show_in_launcher = internal_app.show_in_launcher
                               ? apps::mojom::OptionalBool::kTrue
                               : apps::mojom::OptionalBool::kFalse;

chrome/browser/apps/app_service/crostini_apps.cc

@@ -180,6 +180,7 @@ apps::mojom::AppPtr CrostiniApps::Convert(
   app->install_time = registration.InstallTime();
 
   app->installed_internally = apps::mojom::OptionalBool::kFalse;
+  app->is_platform_app = apps::mojom::OptionalBool::kFalse;
 
   // TODO(crbug.com/826982): if Crostini isn't enabled, don't show the Terminal
   // item until it becomes enabled.

chrome/browser/apps/app_service/extension_apps.cc

@@ -43,11 +43,6 @@
 // TODO(crbug.com/826982): do we also need to watch prefs, the same as
 // ExtensionAppModelBuilder?
 
-// TODO(crbug.com/826982): support the is_platform_app bit. We might not need
-// to plumb this all the way through the Mojo methods, as AFAICT it's only used
-// for populating the context menu, which is done on the app publisher side
-// (i.e. in this C++ file) and not at all on the app subscriber side.
-
 namespace {
 
 // Only supporting important permissions for now.
@@ -464,6 +459,10 @@ apps::mojom::AppPtr ExtensionApps::Convert(
                                   ? apps::mojom::OptionalBool::kTrue
                                   : apps::mojom::OptionalBool::kFalse;
 
+  app->is_platform_app = extension->is_platform_app()
+                             ? apps::mojom::OptionalBool::kTrue
+                             : apps::mojom::OptionalBool::kFalse;
+
   auto show = app_list::ShouldShowInLauncher(extension, profile_)
                   ? apps::mojom::OptionalBool::kTrue
                   : apps::mojom::OptionalBool::kFalse;

chrome/browser/ui/app_list/app_service_app_item.cc

@@ -52,7 +52,8 @@ AppServiceAppItem::AppServiceAppItem(
     const app_list::AppListSyncableService::SyncItem* sync_item,
     const apps::AppUpdate& app_update)
     : ChromeAppListItem(profile, app_update.AppId()),
-      app_type_(app_update.AppType()) {
+      app_type_(app_update.AppType()),
+      is_platform_app_(false) {
   OnAppUpdate(app_update, true);
   if (sync_item && sync_item->item_ordinal.IsValid()) {
     UpdateFromSync(sync_item);
@@ -86,6 +87,11 @@ void AppServiceAppItem::OnAppUpdate(const apps::AppUpdate& app_update,
                                      weak_ptr_factory_.GetWeakPtr()));
     }
   }
+
+  if (in_constructor || app_update.IsPlatformAppChanged()) {
+    is_platform_app_ =
+        app_update.IsPlatformApp() == apps::mojom::OptionalBool::kTrue;
+  }
 }
 
 void AppServiceAppItem::Activate(int event_flags) {
@@ -97,12 +103,8 @@ const char* AppServiceAppItem::GetItemType() const {
 }
 
 void AppServiceAppItem::GetContextMenuModel(GetMenuModelCallback callback) {
-  // TODO(crbug.com/826982): don't hard-code false. The App Service should
-  // probably provide this.
-  const bool is_platform_app = false;
-
   context_menu_ = MakeAppContextMenu(app_type_, this, profile(), id(),
-                                     GetController(), is_platform_app);
+                                     GetController(), is_platform_app_);
   context_menu_->GetMenuModel(std::move(callback));
 }
 

chrome/browser/ui/app_list/app_service_app_item.h

@@ -51,6 +51,7 @@ class AppServiceAppItem : public ChromeAppListItem,
   void OnLoadIcon(apps::mojom::IconValuePtr icon_value);
 
   apps::mojom::AppType app_type_;
+  bool is_platform_app_;
 
   std::unique_ptr<app_list::AppContextMenu> context_menu_;
 

chrome/browser/ui/app_list/search/app_service_app_result.cc

@@ -21,6 +21,7 @@ AppServiceAppResult::AppServiceAppResult(Profile* profile,
                                          bool is_recommendation)
     : AppResult(profile, app_id, controller, is_recommendation),
       app_type_(apps::mojom::AppType::kUnknown),
+      is_platform_app_(false),
       show_in_launcher_(false),
       weak_ptr_factory_(this) {
   apps::AppServiceProxy* proxy = apps::AppServiceProxy::Get(profile);
@@ -28,6 +29,8 @@ AppServiceAppResult::AppServiceAppResult(Profile* profile,
   if (proxy) {
     proxy->Cache().ForOneApp(app_id, [this](const apps::AppUpdate& update) {
       app_type_ = update.AppType();
+      is_platform_app_ =
+          update.IsPlatformApp() == apps::mojom::OptionalBool::kTrue;
       show_in_launcher_ =
           update.ShowInLauncher() == apps::mojom::OptionalBool::kTrue;
     });
@@ -80,10 +83,6 @@ void AppServiceAppResult::Open(int event_flags) {
 }
 
 void AppServiceAppResult::GetContextMenuModel(GetMenuModelCallback callback) {
-  // TODO(crbug.com/826982): don't hard-code false. The App Service should
-  // probably provide this.
-  const bool is_platform_app = false;
-
   // TODO(crbug.com/826982): drop the (app_type_ == etc), and check
   // show_in_launcher_ for all app types?
   if ((app_type_ == apps::mojom::AppType::kBuiltIn) && !show_in_launcher_) {
@@ -92,7 +91,7 @@ void AppServiceAppResult::GetContextMenuModel(GetMenuModelCallback callback) {
   }
 
   context_menu_ = AppServiceAppItem::MakeAppContextMenu(
-      app_type_, this, profile(), app_id(), controller(), is_platform_app);
+      app_type_, this, profile(), app_id(), controller(), is_platform_app_);
   context_menu_->GetMenuModel(std::move(callback));
 }
 

chrome/browser/ui/app_list/search/app_service_app_result.h

@@ -35,6 +35,7 @@ class AppServiceAppResult : public AppResult {
   void OnLoadIcon(bool chip, apps::mojom::IconValuePtr icon_value);
 
   apps::mojom::AppType app_type_;
+  bool is_platform_app_;
   bool show_in_launcher_;
 
   std::unique_ptr<AppContextMenu> context_menu_;

chrome/services/app_service/public/cpp/app_update.cc

@@ -63,6 +63,9 @@ void AppUpdate::Merge(apps::mojom::App* state, const apps::mojom::App* delta) {
   if (delta->installed_internally != apps::mojom::OptionalBool::kUnknown) {
     state->installed_internally = delta->installed_internally;
   }
+  if (delta->is_platform_app != apps::mojom::OptionalBool::kUnknown) {
+    state->is_platform_app = delta->is_platform_app;
+  }
   if (delta->show_in_launcher != apps::mojom::OptionalBool::kUnknown) {
     state->show_in_launcher = delta->show_in_launcher;
   }
@@ -222,6 +225,23 @@ bool AppUpdate::InstalledInternallyChanged() const {
           (delta_->installed_internally != state_->installed_internally));
 }
 
+apps::mojom::OptionalBool AppUpdate::IsPlatformApp() const {
+  if (delta_ &&
+      (delta_->is_platform_app != apps::mojom::OptionalBool::kUnknown)) {
+    return delta_->is_platform_app;
+  }
+  if (state_) {
+    return state_->is_platform_app;
+  }
+  return apps::mojom::OptionalBool::kUnknown;
+}
+
+bool AppUpdate::IsPlatformAppChanged() const {
+  return delta_ &&
+         (delta_->is_platform_app != apps::mojom::OptionalBool::kUnknown) &&
+         (!state_ || (delta_->is_platform_app != state_->is_platform_app));
+}
+
 apps::mojom::OptionalBool AppUpdate::ShowInLauncher() const {
   if (delta_ &&
       (delta_->show_in_launcher != apps::mojom::OptionalBool::kUnknown)) {

chrome/services/app_service/public/cpp/app_update.h

@@ -82,6 +82,9 @@ class AppUpdate {
   apps::mojom::OptionalBool InstalledInternally() const;
   bool InstalledInternallyChanged() const;
 
+  apps::mojom::OptionalBool IsPlatformApp() const;
+  bool IsPlatformAppChanged() const;
+
   apps::mojom::OptionalBool ShowInLauncher() const;
   bool ShowInLauncherChanged() const;
 

chrome/services/app_service/public/cpp/app_update_unittest.cc

@@ -38,6 +38,9 @@ class AppUpdateTest : public testing::Test {
   apps::mojom::OptionalBool expect_installed_internally_;
   bool expect_installed_internally_changed_;
 
+  apps::mojom::OptionalBool expect_is_platform_app_;
+  bool expect_is_platform_app_changed_;
+
   apps::mojom::OptionalBool expect_show_in_launcher_;
   bool expect_show_in_launcher_changed_;
 
@@ -65,6 +68,7 @@ class AppUpdateTest : public testing::Test {
     expect_install_time_changed_ = false;
     expect_permissions_changed_ = false;
     expect_installed_internally_changed_ = false;
+    expect_is_platform_app_changed_ = false;
     expect_show_in_launcher_changed_ = false;
     expect_show_in_search_changed_ = false;
   }
@@ -95,6 +99,9 @@ class AppUpdateTest : public testing::Test {
     EXPECT_EQ(expect_installed_internally_changed_,
               u.InstalledInternallyChanged());
 
+    EXPECT_EQ(expect_is_platform_app_, u.IsPlatformApp());
+    EXPECT_EQ(expect_is_platform_app_changed_, u.IsPlatformAppChanged());
+
     EXPECT_EQ(expect_show_in_launcher_, u.ShowInLauncher());
     EXPECT_EQ(expect_show_in_launcher_changed_, u.ShowInLauncherChanged());
 
@@ -117,6 +124,7 @@ class AppUpdateTest : public testing::Test {
     expect_install_time_ = base::Time();
     expect_permissions_.clear();
     expect_installed_internally_ = apps::mojom::OptionalBool::kUnknown;
+    expect_is_platform_app_ = apps::mojom::OptionalBool::kUnknown;
     expect_show_in_launcher_ = apps::mojom::OptionalBool::kUnknown;
     expect_show_in_search_ = apps::mojom::OptionalBool::kUnknown;
     ExpectNoChange();
@@ -278,6 +286,28 @@ class AppUpdateTest : public testing::Test {
       CheckExpects(u);
     }
 
+    // IsPlatformApp tests.
+
+    if (state) {
+      state->is_platform_app = apps::mojom::OptionalBool::kFalse;
+      expect_is_platform_app_ = apps::mojom::OptionalBool::kFalse;
+      expect_is_platform_app_changed_ = false;
+      CheckExpects(u);
+    }
+
+    if (delta) {
+      delta->is_platform_app = apps::mojom::OptionalBool::kTrue;
+      expect_is_platform_app_ = apps::mojom::OptionalBool::kTrue;
+      expect_is_platform_app_changed_ = true;
+      CheckExpects(u);
+    }
+
+    if (state) {
+      apps::AppUpdate::Merge(state, delta);
+      ExpectNoChange();
+      CheckExpects(u);
+    }
+
     // ShowInSearch tests.
 
     if (state) {

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

@@ -31,6 +31,10 @@ struct App {
   // Whether the app was installed by sync, policy or as a default app.
   OptionalBool installed_internally;
 
+  // Whether the app is an extensions::Extensions where is_platform_app()
+  // returns true.
+  OptionalBool is_platform_app;
+
   // TODO(nigeltao): be more principled, instead of ad hoc show_in_xxx and
   // show_in_yyy fields?
   OptionalBool show_in_launcher;