Remove InternalAppModelBuilder.

AppServiceAppModelBuilder is used to replace InternalAppModelBuilder.
AppService integration to ui/app_list has been enabled for a couple
of months, so it should be safe to remove InternalAppModelBuilder.

Unit tests are updated to use AppServiceAppModelBuilder. I will use a
separate CL to move the internal_app_model_builder_unittest.cc to
src/chrome/browser/ui/app_list/app_service/, and convert it to
AppService unit tests. This CL still uses
internal_app_model_builder_unittest.cc, as it is more clear to show
the change.

BUG=1016159

Change-Id: I2d537aa940eae29201dd64bb7b345d3ae820d777
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1919534Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Nancy Wang <nancylingwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#716371}

chrome/browser/apps/app_service/app_service_test.cc

@@ -23,6 +23,25 @@ void AppServiceTest::SetUp(Profile* profile) {
   WaitForAppService();
 }
 
+void AppServiceTest::UninstallAllApps(Profile* profile) {
+  AppServiceProxy* app_service_proxy_ =
+      apps::AppServiceProxyFactory::GetForProfile(profile);
+  DCHECK(app_service_proxy_);
+  std::vector<apps::mojom::AppPtr> apps;
+  app_service_proxy_->AppRegistryCache().ForEachApp(
+      [&apps](const apps::AppUpdate& update) {
+        apps::mojom::AppPtr app = apps::mojom::App::New();
+        app->app_type = update.AppType();
+        app->app_id = update.AppId();
+        app->readiness = apps::mojom::Readiness::kUninstalledByUser;
+        apps.push_back(app.Clone());
+      });
+  app_service_proxy_->AppRegistryCache().OnApps(std::move(apps));
+
+  // Allow async callbacks to run.
+  WaitForAppService();
+}
+
 std::string AppServiceTest::GetAppName(const std::string& app_id) const {
   std::string name;
   if (!app_service_proxy_)

chrome/browser/apps/app_service/app_service_test.h

@@ -25,6 +25,8 @@ class AppServiceTest {
 
   void SetUp(Profile* profile);
 
+  void UninstallAllApps(Profile* profile);
+
   std::string GetAppName(const std::string& app_id) const;
 
   // Allow AppService async callbacks to run.

chrome/browser/ui/BUILD.gn

@@ -3589,8 +3589,6 @@ jumbo_static_library("ui") {
       "app_list/internal_app/internal_app_item.h",
       "app_list/internal_app/internal_app_metadata.cc",
       "app_list/internal_app/internal_app_metadata.h",
-      "app_list/internal_app/internal_app_model_builder.cc",
-      "app_list/internal_app/internal_app_model_builder.h",
       "app_list/md_icon_normalizer.cc",
       "app_list/md_icon_normalizer.h",
       "app_list/page_break_app_item.cc",

chrome/browser/ui/app_list/app_list_syncable_service.cc

@@ -35,7 +35,6 @@
 #include "chrome/browser/ui/app_list/crostini/crostini_app_model_builder.h"
 #include "chrome/browser/ui/app_list/extension_app_item.h"
 #include "chrome/browser/ui/app_list/extension_app_model_builder.h"
-#include "chrome/browser/ui/app_list/internal_app/internal_app_model_builder.h"
 #include "chrome/browser/ui/app_list/page_break_app_item.h"
 #include "chrome/browser/ui/app_list/page_break_constants.h"
 #include "chrome/common/chrome_features.h"
@@ -438,8 +437,6 @@ void AppListSyncableService::BuildModel() {
       crostini_apps_builder_ =
           std::make_unique<CrostiniAppModelBuilder>(controller);
     }
-    internal_apps_builder_ =
-        std::make_unique<InternalAppModelBuilder>(controller);
   }
 
   DCHECK(profile_);
@@ -453,7 +450,6 @@ void AppListSyncableService::BuildModel() {
       arc_apps_builder_->Initialize(this, profile_, model_updater_.get());
     if (crostini_apps_builder_.get())
       crostini_apps_builder_->Initialize(this, profile_, model_updater_.get());
-    internal_apps_builder_->Initialize(this, profile_, model_updater_.get());
   }
 
   HandleUpdateFinished();
@@ -1041,7 +1037,6 @@ void AppListSyncableService::Shutdown() {
     app_service_apps_builder_.reset();
     return;
   }
-  internal_apps_builder_.reset();
   crostini_apps_builder_.reset();
   arc_apps_builder_.reset();
   ext_apps_builder_.reset();

chrome/browser/ui/app_list/app_list_syncable_service.h

@@ -31,7 +31,6 @@ class ArcAppModelBuilder;
 class ChromeAppListItem;
 class CrostiniAppModelBuilder;
 class ExtensionAppModelBuilder;
-class InternalAppModelBuilder;
 class Profile;
 
 namespace extensions {
@@ -326,7 +325,6 @@ class AppListSyncableService : public syncer::SyncableService,
   std::unique_ptr<ExtensionAppModelBuilder> ext_apps_builder_;
   std::unique_ptr<ArcAppModelBuilder> arc_apps_builder_;
   std::unique_ptr<CrostiniAppModelBuilder> crostini_apps_builder_;
-  std::unique_ptr<InternalAppModelBuilder> internal_apps_builder_;
   std::unique_ptr<syncer::SyncChangeProcessor> sync_processor_;
   std::unique_ptr<syncer::SyncErrorFactory> sync_error_handler_;
   SyncItemMap sync_items_;

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

-// Copyright 2018 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/internal_app/internal_app_model_builder.h"
-
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/ui/app_list/app_list_controller_delegate.h"
-#include "chrome/browser/ui/app_list/internal_app/internal_app_item.h"
-#include "chrome/browser/ui/app_list/internal_app/internal_app_metadata.h"
-
-InternalAppModelBuilder::InternalAppModelBuilder(
-    AppListControllerDelegate* controller)
-    : AppListModelBuilder(controller, InternalAppItem::kItemType) {}
-
-void InternalAppModelBuilder::BuildModel() {
-  for (const auto& internal_app : app_list::GetInternalAppList(profile())) {
-    if (!internal_app.show_in_launcher)
-      continue;
-
-    InsertApp(std::make_unique<InternalAppItem>(
-        profile(), model_updater(), GetSyncItem(internal_app.app_id),
-        internal_app));
-  }
-}

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

-// Copyright 2018 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_INTERNAL_APP_INTERNAL_APP_MODEL_BUILDER_H_
-#define CHROME_BROWSER_UI_APP_LIST_INTERNAL_APP_INTERNAL_APP_MODEL_BUILDER_H_
-
-#include "base/macros.h"
-#include "chrome/browser/ui/app_list/app_list_model_builder.h"
-
-class AppListControllerDelegate;
-
-// This class populates and maintains internal apps.
-class InternalAppModelBuilder : public AppListModelBuilder {
- public:
-  explicit InternalAppModelBuilder(AppListControllerDelegate* controller);
-
-  ~InternalAppModelBuilder() override = default;
-
- private:
-  // AppListModelBuilder:
-  void BuildModel() override;
-
-  DISALLOW_COPY_AND_ASSIGN(InternalAppModelBuilder);
-};
-
-#endif  // CHROME_BROWSER_UI_APP_LIST_INTERNAL_APP_INTERNAL_APP_MODEL_BUILDER_H_

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

@@ -2,12 +2,14 @@
 // 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/internal_app/internal_app_model_builder.h"
-
 #include "base/macros.h"
 #include "base/strings/string_util.h"
+#include "chrome/browser/apps/app_service/app_service_proxy.h"
+#include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
+#include "chrome/browser/apps/app_service/app_service_test.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/app_list/app_list_test_util.h"
+#include "chrome/browser/ui/app_list/app_service/app_service_app_model_builder.h"
 #include "chrome/browser/ui/app_list/chrome_app_list_item.h"
 #include "chrome/browser/ui/app_list/internal_app/internal_app_metadata.h"
 #include "chrome/browser/ui/app_list/test/fake_app_list_model_updater.h"
@@ -25,6 +27,22 @@ std::string GetModelContent(AppListModelUpdater* model_updater) {
   return base::JoinString(app_names, ",");
 }
 
+// For testing purposes, we want to pretend there are only BuiltIn apps on the
+// system. This method removes the others.
+void RemoveNonBuiltInApps(Profile* profile,
+                          FakeAppListModelUpdater* model_updater) {
+  apps::AppServiceProxy* proxy =
+      apps::AppServiceProxyFactory::GetForProfile(profile);
+  DCHECK(proxy);
+  proxy->FlushMojoCallsForTesting();
+  proxy->AppRegistryCache().ForEachApp(
+      [&model_updater](const apps::AppUpdate& update) {
+        if (update.AppType() != apps::mojom::AppType::kBuiltIn) {
+          model_updater->RemoveItem(update.AppId());
+        }
+      });
+}
+
 }  // namespace
 
 class InternalAppModelBuilderTest : public AppListTestBase {
@@ -47,11 +65,14 @@ class InternalAppModelBuilderTest : public AppListTestBase {
   void CreateBuilder(bool guest_mode) {
     ResetBuilder();  // Destroy any existing builder in the correct order.
 
+    app_service_test_.UninstallAllApps(profile());
     testing_profile()->SetGuestSession(guest_mode);
+    app_service_test_.SetUp(testing_profile());
     model_updater_ = std::make_unique<FakeAppListModelUpdater>();
     controller_ = std::make_unique<test::TestAppListControllerDelegate>();
-    builder_ = std::make_unique<InternalAppModelBuilder>(controller_.get());
-    builder_->Initialize(nullptr, profile(), model_updater_.get());
+    builder_ = std::make_unique<AppServiceAppModelBuilder>(controller_.get());
+    builder_->Initialize(nullptr, testing_profile(), model_updater_.get());
+    RemoveNonBuiltInApps(testing_profile(), model_updater_.get());
   }
 
   std::unique_ptr<FakeAppListModelUpdater> model_updater_;
@@ -63,8 +84,9 @@ class InternalAppModelBuilderTest : public AppListTestBase {
     model_updater_.reset();
   }
 
+  apps::AppServiceTest app_service_test_;
   std::unique_ptr<test::TestAppListControllerDelegate> controller_;
-  std::unique_ptr<InternalAppModelBuilder> builder_;
+  std::unique_ptr<AppServiceAppModelBuilder> builder_;
 
   DISALLOW_COPY_AND_ASSIGN(InternalAppModelBuilderTest);
 };