Remove dependency on Extensions for SystemWebAppManager browser_tests

This CL rewrites some of the SystemWebAppManagerBrowserTest code to use
web_app::AppRegistrar instead of the Extension interface.
This is necessary to migrate web apps off the Extension system.

Bug: 1052348, 876576
Change-Id: If75fdbb1c25b089961b40f47a48d09a8f3ffcdd0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2059356
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742517}

chrome/browser/chromeos/web_applications/system_web_app_integration_test.cc

@@ -13,9 +13,6 @@
 #include "chrome/browser/web_applications/system_web_app_manager.h"
 #include "content/public/browser/web_ui.h"
 #include "content/public/test/test_navigation_observer.h"
-#include "extensions/browser/extension_registry.h"
-#include "extensions/common/extension.h"
-#include "extensions/common/manifest.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/aura/window.h"
 
@@ -33,24 +30,19 @@ void SystemWebAppIntegrationTest::ExpectSystemWebAppValid(
     const GURL& url,
     const std::string& title) {
   Browser* app_browser = WaitForSystemAppInstallAndLaunch(app_type);
-  base::Optional<web_app::AppId> app_id =
-      web_app::FindInstalledAppWithUrlInScope(profile(), url);
-  // TODO(crbug.com/1052711): Avoid dependence on Extensions.
-  const extensions::Extension* installed_app =
-      extensions::ExtensionRegistry::Get(profile())->GetInstalledExtension(
-          *app_id);
 
-  EXPECT_TRUE(GetManager().IsSystemWebApp(installed_app->id()));
-  EXPECT_TRUE(installed_app->from_bookmark());
+  web_app::AppId app_id = app_browser->app_controller()->GetAppId();
+  EXPECT_EQ(GetManager().GetAppIdForSystemApp(app_type), app_id);
+  EXPECT_TRUE(GetManager().IsSystemWebApp(app_id));
 
-  EXPECT_EQ(title, installed_app->name());
+  web_app::AppRegistrar& registrar =
+      web_app::WebAppProviderBase::GetProviderBase(profile())->registrar();
+  EXPECT_EQ(title, registrar.GetAppShortName(app_id));
   EXPECT_EQ(base::ASCIIToUTF16(title),
             app_browser->window()->GetNativeWindow()->GetTitle());
-  EXPECT_EQ(extensions::Manifest::EXTERNAL_COMPONENT,
-            installed_app->location());
+  EXPECT_TRUE(registrar.HasExternalAppWithInstallSource(
+      app_id, web_app::ExternalInstallSource::kSystemInstalled));
 
-  // The installed app should match the opened app window.
-  EXPECT_EQ(installed_app, GetExtensionForAppBrowser(app_browser));
   content::WebContents* web_contents =
       app_browser->tab_strip_model()->GetActiveWebContents();
 

chrome/browser/chromeos/web_applications/system_web_app_integration_test.h

@@ -7,7 +7,7 @@
 
 #include <string>
 
-#include "chrome/browser/web_applications/extensions/system_web_app_manager_browsertest.h"
+#include "chrome/browser/web_applications/system_web_app_manager_browsertest.h"
 #include "url/gurl.h"
 
 namespace web_app {

chrome/browser/ui/views/frame/browser_non_client_frame_view_browsertest.cc

@@ -13,8 +13,8 @@
 #include "chrome/browser/ui/views/location_bar/custom_tab_bar_view.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/browser/ui/web_applications/test/web_app_browsertest_util.h"
-#include "chrome/browser/web_applications/extensions/system_web_app_manager_browsertest.h"
 #include "chrome/browser/web_applications/system_web_app_manager.h"
+#include "chrome/browser/web_applications/system_web_app_manager_browsertest.h"
 #include "chrome/common/web_application_info.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"

chrome/browser/web_applications/BUILD.gn

@@ -216,6 +216,8 @@ source_set("web_applications_browser_tests") {
   sources = [
     "manifest_update_manager_browsertest.cc",
     "pending_app_manager_impl_browsertest.cc",
+    "system_web_app_manager_browsertest.cc",
+    "system_web_app_manager_browsertest.h",
     "web_app_icon_manager_browsertest.cc",
     "web_app_migration_manager_browsertest.cc",
   ]
@@ -232,6 +234,7 @@ source_set("web_applications_browser_tests") {
     "//chrome/browser/web_applications/components",
     "//chrome/test:test_support",
     "//chrome/test:test_support_ui",
+    "//components/permissions:permissions",
   ]
 }
 

chrome/browser/web_applications/extensions/BUILD.gn

@@ -83,8 +83,6 @@ source_set("browser_tests") {
 
   sources = [
     "bookmark_app_registrar_browsertest.cc",
-    "system_web_app_manager_browsertest.cc",
-    "system_web_app_manager_browsertest.h",
     "web_app_audio_focus_browsertest.cc",
   ]
 
@@ -103,7 +101,6 @@ source_set("browser_tests") {
     "//chrome/browser/web_applications/components",
     "//chrome/test:test_support",
     "//chrome/test:test_support_ui",
-    "//components/permissions:permissions",
     "//extensions:test_support",
     "//extensions/browser",
     "//extensions/common",

chrome/browser/web_applications/extensions/system_web_app_manager_browsertest.cc → chrome/browser/web_applications/system_web_app_manager_browsertest.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/web_applications/extensions/system_web_app_manager_browsertest.h"
+#include "chrome/browser/web_applications/system_web_app_manager_browsertest.h"
 
 #include <string>
 #include <utility>
@@ -14,17 +14,14 @@
 #include "base/test/scoped_feature_list.h"
 #include "chrome/browser/apps/app_service/app_launch_params.h"
 #include "chrome/browser/apps/launch_service/launch_service.h"
-#include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/native_file_system/native_file_system_permission_request_manager.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser_finder.h"
-#include "chrome/browser/ui/extensions/application_launch.h"
-#include "chrome/browser/ui/extensions/hosted_app_browser_controller.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "chrome/browser/web_applications/test/test_system_web_app_installation.h"
 #include "chrome/browser/web_applications/test/test_web_app_provider.h"
 #include "chrome/common/chrome_features.h"
-#include "chrome/common/extensions/manifest_handlers/app_theme_color_info.h"
 #include "chrome/common/webui_url_constants.h"
 #include "components/permissions/permission_util.h"
 #include "content/public/browser/web_contents.h"
@@ -54,14 +51,6 @@ SystemWebAppManagerBrowserTest::SystemWebAppManagerBrowserTest(
 
 SystemWebAppManagerBrowserTest::~SystemWebAppManagerBrowserTest() = default;
 
-// static
-const extensions::Extension*
-SystemWebAppManagerBrowserTest::GetExtensionForAppBrowser(Browser* browser) {
-  return static_cast<extensions::HostedAppBrowserController*>(
-             browser->app_controller())
-      ->GetExtensionForTesting();
-}
-
 SystemWebAppManager& SystemWebAppManagerBrowserTest::GetManager() {
   return WebAppProvider::Get(browser()->profile())->system_web_app_manager();
 }
@@ -129,20 +118,31 @@ content::EvalJsResult EvalJs(content::WebContents* web_contents,
 
 // Test that System Apps install correctly with a manifest.
 IN_PROC_BROWSER_TEST_F(SystemWebAppManagerBrowserTest, Install) {
-  const extensions::Extension* app = GetExtensionForAppBrowser(
-      WaitForSystemAppInstallAndLaunch(GetMockAppType()));
-  EXPECT_EQ("Test System App", app->name());
-  EXPECT_EQ(SkColorSetRGB(0, 0xFF, 0),
-            extensions::AppThemeColorInfo::GetThemeColor(app));
-  EXPECT_TRUE(app->from_bookmark());
-  EXPECT_EQ(extensions::Manifest::EXTERNAL_COMPONENT, app->location());
-
-  // The app should be a PWA.
-  base::Optional<AppId> app_id = web_app::FindInstalledAppWithUrlInScope(
-      browser()->profile(), content::GetWebUIURL("test-system-app/"));
-  DCHECK(app_id);
-  EXPECT_EQ(*app_id, app->id());
-  EXPECT_TRUE(GetManager().IsSystemWebApp(app->id()));
+  Browser* app_browser = WaitForSystemAppInstallAndLaunch(GetMockAppType());
+
+  AppId app_id = app_browser->app_controller()->GetAppId();
+  EXPECT_EQ(GetManager().GetAppIdForSystemApp(GetMockAppType()), app_id);
+  EXPECT_TRUE(GetManager().IsSystemWebApp(app_id));
+
+  Profile* profile = app_browser->profile();
+  AppRegistrar& registrar =
+      WebAppProviderBase::GetProviderBase(profile)->registrar();
+
+  EXPECT_EQ("Test System App", registrar.GetAppShortName(app_id));
+  EXPECT_EQ(SkColorSetRGB(0, 0xFF, 0), registrar.GetAppThemeColor(app_id));
+  EXPECT_TRUE(registrar.HasExternalAppWithInstallSource(
+      app_id, web_app::ExternalInstallSource::kSystemInstalled));
+  EXPECT_EQ(
+      registrar.FindAppWithUrlInScope(content::GetWebUIURL("test-system-app/")),
+      app_id);
+
+  if (!base::FeatureList::IsEnabled(features::kDesktopPWAsWithoutExtensions)) {
+    const extensions::Extension* extension =
+        extensions::ExtensionRegistry::Get(profile)->GetInstalledExtension(
+            app_id);
+    EXPECT_TRUE(extension->from_bookmark());
+    EXPECT_EQ(extensions::Manifest::EXTERNAL_COMPONENT, extension->location());
+  }
 }
 
 // Check the toolbar is not shown for system web apps for pages on the chrome://
@@ -198,7 +198,7 @@ IN_PROC_BROWSER_TEST_F(SystemWebAppManagerBrowserTest,
   content::TestNavigationObserver navigation_observer(launch_url);
   navigation_observer.StartWatchingNewWebContents();
   content::WebContents* web_contents =
-      OpenApplication(browser()->profile(), params);
+      apps::LaunchService::Get(browser()->profile())->OpenApplication(params);
   navigation_observer.Wait();
 
   // Set up a Promise that resolves to launchParams, when launchQueue's consumer
@@ -235,7 +235,7 @@ IN_PROC_BROWSER_TEST_F(SystemWebAppManagerBrowserTest,
                                              &temp_file_path2));
   params.launch_files = {temp_file_path2};
   content::WebContents* web_contents2 =
-      OpenApplication(browser()->profile(), params);
+      apps::LaunchService::Get(browser()->profile())->OpenApplication(params);
 
   // WebContents* should be the same because we are passing launchParams to the
   // opened application.
@@ -292,7 +292,7 @@ IN_PROC_BROWSER_TEST_P(SystemWebAppManagerLaunchFilesBrowserTest,
   content::TestNavigationObserver navigation_observer(launch_url);
   navigation_observer.StartWatchingNewWebContents();
   content::WebContents* web_contents =
-      OpenApplication(browser()->profile(), params);
+      apps::LaunchService::Get(browser()->profile())->OpenApplication(params);
   navigation_observer.Wait();
 
   // Set up a Promise that resolves to launchParams, when launchQueue's consumer
@@ -356,7 +356,7 @@ IN_PROC_BROWSER_TEST_P(SystemWebAppManagerLaunchFilesBrowserTest,
                                              &temp_file_path2));
   params.launch_files = {temp_file_path2};
   content::WebContents* web_contents2 =
-      OpenApplication(browser()->profile(), params);
+      apps::LaunchService::Get(browser()->profile())->OpenApplication(params);
 
   // WebContents* should be the same because we are passing launchParams to the
   // opened application.

chrome/browser/web_applications/extensions/system_web_app_manager_browsertest.h → chrome/browser/web_applications/system_web_app_manager_browsertest.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_WEB_APPLICATIONS_EXTENSIONS_SYSTEM_WEB_APP_MANAGER_BROWSERTEST_H_
-#define CHROME_BROWSER_WEB_APPLICATIONS_EXTENSIONS_SYSTEM_WEB_APP_MANAGER_BROWSERTEST_H_
+#ifndef CHROME_BROWSER_WEB_APPLICATIONS_SYSTEM_WEB_APP_MANAGER_BROWSERTEST_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_SYSTEM_WEB_APP_MANAGER_BROWSERTEST_H_
 
 #include <memory>
 
@@ -24,10 +24,6 @@ namespace content {
 class WebContents;
 }
 
-namespace extensions {
-class Extension;
-}
-
 namespace web_app {
 
 enum class SystemAppType;
@@ -42,11 +38,6 @@ class SystemWebAppManagerBrowserTest : public InProcessBrowserTest {
 
   ~SystemWebAppManagerBrowserTest() override;
 
-  // Gets the Extension* from the HostedAppBrowserController associated with
-  // |browser|.
-  static const extensions::Extension* GetExtensionForAppBrowser(
-      Browser* browser);
-
   // Returns the SystemWebAppManager for browser()->profile(). This will be a
   // TestSystemWebAppManager if initialized with |install_mock| true.
   SystemWebAppManager& GetManager();
@@ -80,4 +71,4 @@ class SystemWebAppManagerBrowserTest : public InProcessBrowserTest {
 
 }  // namespace web_app
 
-#endif  // CHROME_BROWSER_WEB_APPLICATIONS_EXTENSIONS_SYSTEM_WEB_APP_MANAGER_BROWSERTEST_H_
+#endif  // CHROME_BROWSER_WEB_APPLICATIONS_SYSTEM_WEB_APP_MANAGER_BROWSERTEST_H_