[System Web Apps] Make application_launch defer to web_app::LaunchSystemWebApp.

This CL fixes an issue where the app list would launch new instances of
the Settings App because it didn't run through the same System Web App
launch code path that the rest of Chrome OS does.

Bug: 1012558
Change-Id: I60aead0adee4cc13082648ae67a50a03f811f673
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1868354Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Commit-Queue: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710199}

chrome/browser/ui/extensions/application_launch.cc

@@ -31,8 +31,10 @@
 #include "chrome/browser/ui/extensions/extension_enable_flow.h"
 #include "chrome/browser/ui/extensions/extension_enable_flow_delegate.h"
 #include "chrome/browser/ui/extensions/hosted_app_browser_controller.h"
+#include "chrome/browser/ui/web_applications/system_web_app_ui_utils.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "chrome/browser/web_applications/components/web_app_tab_helper.h"
+#include "chrome/browser/web_applications/system_web_app_manager.h"
 #include "chrome/browser/web_launch/web_launch_files_helper.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
@@ -301,6 +303,15 @@ WebContents* OpenEnabledApplication(Profile* profile,
 
   GURL url = UrlForExtension(extension, params.override_url);
 
+  // System Web Apps go through their own launch path.
+  base::Optional<web_app::SystemAppType> system_app_type =
+      web_app::GetSystemWebAppTypeForAppId(profile, extension->id());
+  if (system_app_type) {
+    Browser* browser =
+        web_app::LaunchSystemWebApp(profile, *system_app_type, url);
+    return browser->tab_strip_model()->GetActiveWebContents();
+  }
+
   // Record v1 app launch. Platform app launch is recorded when dispatching
   // the onLaunched event.
   prefs->SetLastLaunchTime(extension->id(), base::Time::Now());

chrome/browser/ui/settings_window_manager_browsertest_chromeos.cc

@@ -11,8 +11,10 @@
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/chrome_pages.h"
+#include "chrome/browser/ui/extensions/application_launch.h"
 #include "chrome/browser/ui/settings_window_manager_chromeos.h"
 #include "chrome/browser/ui/settings_window_manager_observer_chromeos.h"
+#include "chrome/browser/ui/web_applications/system_web_app_ui_utils.h"
 #include "chrome/browser/web_applications/system_web_app_manager.h"
 #include "chrome/browser/web_applications/web_app_provider.h"
 #include "chrome/common/chrome_features.h"
@@ -20,6 +22,7 @@
 #include "chrome/common/webui_url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chromeos/constants/chromeos_features.h"
+#include "content/public/browser/web_contents.h"
 #include "url/gurl.h"
 
 namespace {
@@ -116,6 +119,22 @@ IN_PROC_BROWSER_TEST_P(SettingsWindowManagerTest, OpenSettingsWindow) {
             settings_manager_->FindBrowserForProfile(browser()->profile()));
   EXPECT_EQ(1u, observer_.new_settings_count());
 
+  // Launching via application_launch.h should also dedupe to the same browser.
+  if (EnableSystemWebApps()) {
+    web_app::AppId settings_app_id = *web_app::GetAppIdForSystemWebApp(
+        browser()->profile(), web_app::SystemAppType::SETTINGS);
+    content::WebContents* contents = OpenApplication(
+        browser()->profile(),
+        apps::AppLaunchParams(
+            settings_app_id,
+            apps::mojom::LaunchContainer::kLaunchContainerWindow,
+            WindowOpenDisposition::NEW_WINDOW,
+            apps::mojom::AppLaunchSource::kSourceCommandLine));
+    EXPECT_EQ(contents,
+              settings_browser->tab_strip_model()->GetActiveWebContents());
+    EXPECT_EQ(1u, observer_.new_settings_count());
+  }
+
   // Close the settings window.
   CloseBrowserSynchronously(settings_browser);
   EXPECT_FALSE(settings_manager_->FindBrowserForProfile(browser()->profile()));

chrome/browser/ui/web_applications/system_web_app_ui_utils.cc

@@ -37,6 +37,15 @@
 
 namespace web_app {
 
+base::Optional<SystemAppType> GetSystemWebAppTypeForAppId(
+    Profile* profile,
+    web_app::AppId app_id) {
+  auto* provider = WebAppProvider::Get(profile);
+  return provider ? provider->system_web_app_manager().GetSystemAppTypeForAppId(
+                        app_id)
+                  : base::Optional<SystemAppType>();
+}
+
 base::Optional<web_app::AppId> GetAppIdForSystemWebApp(Profile* profile,
                                                        SystemAppType app_type) {
   auto* provider = WebAppProvider::Get(profile);

chrome/browser/ui/web_applications/system_web_app_ui_utils.h

@@ -21,6 +21,11 @@ class WebUIDataSource;
 
 namespace web_app {
 
+// Returns the system app type for the given App ID.
+base::Optional<SystemAppType> GetSystemWebAppTypeForAppId(
+    Profile* profile,
+    web_app::AppId app_id);
+
 // Returns the PWA system App ID for the given system app type.
 base::Optional<web_app::AppId> GetAppIdForSystemWebApp(Profile* profile,
                                                        SystemAppType app_type);

chrome/browser/ui/web_applications/web_app_launch_manager.cc

@@ -11,9 +11,11 @@
 #include "chrome/browser/engagement/site_engagement_service.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/web_applications/system_web_app_ui_utils.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "chrome/browser/web_applications/components/web_app_install_utils.h"
 #include "chrome/browser/web_applications/components/web_app_tab_helper.h"
+#include "chrome/browser/web_applications/system_web_app_manager.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/browser/web_applications/web_app_provider.h"
 #include "chrome/browser/web_applications/web_app_registrar.h"
@@ -96,6 +98,15 @@ content::WebContents* WebAppLaunchManager::OpenApplication(
   if (!provider_->registrar().IsInstalled(params.app_id))
     return nullptr;
 
+  // System Web Apps go through their own launch path.
+  base::Optional<web_app::SystemAppType> system_app_type =
+      web_app::GetSystemWebAppTypeForAppId(profile(), params.app_id);
+  if (system_app_type) {
+    Browser* browser =
+        web_app::LaunchSystemWebApp(profile(), *system_app_type, GURL());
+    return browser->tab_strip_model()->GetActiveWebContents();
+  }
+
   Browser* browser = CreateWebApplicationWindow(profile(), params.app_id);
 
   const GURL url = params.override_url.is_empty()

chrome/browser/web_applications/system_web_app_manager.cc

@@ -201,6 +201,15 @@ base::Optional<AppId> SystemWebAppManager::GetAppIdForSystemApp(
   return registrar_->LookupExternalAppId(app_url_it->second.install_url);
 }
 
+base::Optional<SystemAppType> SystemWebAppManager::GetSystemAppTypeForAppId(
+    AppId app_id) const {
+  auto it = app_id_to_app_type_.find(app_id);
+  if (it == app_id_to_app_type_.end())
+    return base::nullopt;
+
+  return it->second;
+}
+
 bool SystemWebAppManager::IsSystemWebApp(const AppId& app_id) const {
   return registrar_->HasExternalAppWithInstallSource(
       app_id, ExternalInstallSource::kSystemInstalled);

chrome/browser/web_applications/system_web_app_manager.h

@@ -108,6 +108,9 @@ class SystemWebAppManager {
   // Returns the app id for the given System App |type|.
   base::Optional<AppId> GetAppIdForSystemApp(SystemAppType type) const;
 
+  // Returns the System App Type for the given |app_id|.
+  base::Optional<SystemAppType> GetSystemAppTypeForAppId(AppId app_id) const;
+
   // Returns whether |app_id| points to an installed System App.
   bool IsSystemWebApp(const AppId& app_id) const;