[System Web Apps] Extract launch utilities, fix launch disposition.

This CL extracts System Web App launch utilities into its own file as
suggested in https://chromium-review.googlesource.com/c/chromium/src/+/1370239.

It also changes the launch disposition for System Web Apps to CURRENT_TAB to
correctly provide singleton behavior.

Bug: 836128
Change-Id: Iefd691ed3a5c1f1858197599cff5282e22a254ff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1504603Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Commit-Queue: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#641112}

chrome/browser/chromeos/crostini/crostini_manager.cc

@@ -59,6 +59,7 @@
 #include "services/network/public/cpp/network_connection_tracker.h"
 #include "services/service_manager/public/cpp/connector.h"
 #include "storage/browser/fileapi/external_mount_points.h"
+#include "ui/base/window_open_disposition.h"
 
 namespace crostini {
 
@@ -1665,7 +1666,8 @@ void CrostiniManager::ShowContainerTerminal(
     const AppLaunchParams& launch_params,
     const GURL& vsh_in_crosh_url,
     Browser* browser) {
-  ShowApplicationWindow(launch_params, vsh_in_crosh_url, browser);
+  ShowApplicationWindow(launch_params, vsh_in_crosh_url, browser,
+                        WindowOpenDisposition::NEW_FOREGROUND_TAB);
   browser->window()->GetNativeWindow()->SetProperty(
       kOverrideWindowIconResourceIdKey, IDR_LOGO_CROSTINI_TERMINAL);
 }

chrome/browser/ui/BUILD.gn

@@ -3497,6 +3497,8 @@ jumbo_split_static_library("ui") {
       "views/extensions/media_galleries_dialog_views.h",
       "views/extensions/media_gallery_checkbox_view.cc",
       "views/extensions/media_gallery_checkbox_view.h",
+      "web_applications/system_web_app_ui_utils_chromeos.cc",
+      "web_applications/system_web_app_ui_utils_chromeos.h",
       "web_applications/web_app_dialog_utils.cc",
       "web_applications/web_app_dialog_utils.h",
       "web_applications/web_app_metrics.cc",

chrome/browser/ui/extensions/application_launch.cc

@@ -380,14 +380,15 @@ Browser* CreateApplicationWindow(const AppLaunchParams& params,
 
 WebContents* ShowApplicationWindow(const AppLaunchParams& params,
                                    const GURL& url,
-                                   Browser* browser) {
+                                   Browser* browser,
+                                   WindowOpenDisposition disposition) {
   const Extension* const extension = GetExtension(params);
   ui::PageTransition transition =
       (extension ? ui::PAGE_TRANSITION_AUTO_BOOKMARK
                  : ui::PAGE_TRANSITION_AUTO_TOPLEVEL);
 
   NavigateParams nav_params(browser, url, transition);
-  nav_params.disposition = WindowOpenDisposition::NEW_FOREGROUND_TAB;
+  nav_params.disposition = disposition;
   nav_params.opener = params.opener;
   Navigate(&nav_params);
 
@@ -413,7 +414,8 @@ WebContents* ShowApplicationWindow(const AppLaunchParams& params,
 WebContents* OpenApplicationWindow(const AppLaunchParams& params,
                                    const GURL& url) {
   Browser* browser = CreateApplicationWindow(params, url);
-  return ShowApplicationWindow(params, url, browser);
+  return ShowApplicationWindow(params, url, browser,
+                               WindowOpenDisposition::NEW_FOREGROUND_TAB);
 }
 
 void OpenApplicationWithReenablePrompt(const AppLaunchParams& params) {

chrome/browser/ui/extensions/application_launch.h

@@ -19,6 +19,8 @@ namespace extensions {
 class Extension;
 }
 
+enum class WindowOpenDisposition;
+
 // Opens the application, possibly prompting the user to re-enable it.
 void OpenApplicationWithReenablePrompt(const AppLaunchParams& params);
 
@@ -33,7 +35,8 @@ Browser* CreateApplicationWindow(const AppLaunchParams& params,
 // Show the application window that's already created.
 content::WebContents* ShowApplicationWindow(const AppLaunchParams& params,
                                             const GURL& url,
-                                            Browser* browser);
+                                            Browser* browser,
+                                            WindowOpenDisposition disposition);
 
 // Open the application in a way specified by |params| in a new window.
 content::WebContents* OpenApplicationWindow(const AppLaunchParams& params,

chrome/browser/ui/settings_window_manager_chromeos.cc

@@ -7,38 +7,22 @@
 #include "ash/public/cpp/app_types.h"
 #include "ash/public/cpp/resources/grit/ash_public_unscaled_resources.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.h"
 #include "chrome/browser/ui/ash/window_properties.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_navigator.h"
 #include "chrome/browser/ui/browser_navigator_params.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/chrome_pages.h"
-#include "chrome/browser/ui/extensions/app_launch_params.h"
-#include "chrome/browser/ui/extensions/application_launch.h"
 #include "chrome/browser/ui/extensions/hosted_app_browser_controller.h"
 #include "chrome/browser/ui/settings_window_manager_observer_chromeos.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/web_applications/system_web_app_ui_utils_chromeos.h"
 #include "chrome/browser/web_applications/system_web_app_manager.h"
-#include "chrome/browser/web_applications/web_app_provider.h"
 #include "content/public/browser/web_contents.h"
-#include "extensions/browser/extension_registry.h"
-#include "extensions/common/extension.h"
 #include "ui/aura/client/aura_constants.h"
-#include "ui/base/mojo/window_open_disposition.mojom.h"
 #include "ui/base/ui_base_features.h"
 #include "url/gurl.h"
 
-namespace {
-
-base::Optional<std::string> GetSettingsAppId(Profile* profile) {
-  return web_app::WebAppProvider::Get(profile)
-      ->system_web_app_manager()
-      .GetAppIdForSystemApp(web_app::SystemAppType::SETTINGS);
-}
-
-}  // namespace
-
 namespace chrome {
 
 // static
@@ -63,6 +47,20 @@ void SettingsWindowManager::ShowChromePageForProfile(Profile* profile,
   if (!profile->IsGuestSession() && profile->IsOffTheRecord())
     profile = profile->GetOriginalProfile();
 
+  // TODO(calamity): Auto-launch the settings app on install if not found, and
+  // figure out how to invoke OnNewSettingsWindow() in that case.
+  if (web_app::SystemWebAppManager::IsEnabled()) {
+    Browser* browser = web_app::LaunchSystemWebApp(
+        profile, web_app::SystemAppType::SETTINGS, gurl);
+    if (!browser)
+      return;
+
+    for (SettingsWindowManagerObserver& observer : observers_)
+      observer.OnNewSettingsWindow(browser);
+
+    return;
+  }
+
   // Look for an existing browser window.
   Browser* browser = FindBrowserForProfile(profile);
   if (browser) {
@@ -74,61 +72,35 @@ void SettingsWindowManager::ShowChromePageForProfile(Profile* profile,
       return;
     }
   }
-  if (web_app::SystemWebAppManager::IsEnabled()) {
-    base::Optional<std::string> settings_app_id = GetSettingsAppId(profile);
-    // TODO(calamity): Queue a task to launch app after it is installed.
-    if (!settings_app_id)
-      return;
-
-    const extensions::Extension* extension =
-        extensions::ExtensionRegistry::Get(profile)->GetExtensionById(
-            settings_app_id.value(), extensions::ExtensionRegistry::ENABLED);
-    DCHECK(extension);
-
-    // TODO(calamity): Plumb through better launch sources from callsites.
-    AppLaunchParams params = CreateAppLaunchParamsWithEventFlags(
-        profile, extension, 0, extensions::SOURCE_CHROME_INTERNAL,
-        display::kInvalidDisplayId);
-    params.override_url = gurl;
-    if (browser) {
-      ShowApplicationWindow(params, gurl, browser);
-      return;
-    }
-
-    browser = CreateApplicationWindow(params, gurl);
-    ShowApplicationWindow(params, gurl, browser);
-  } else {
-    if (browser) {
-      NavigateParams params(browser, gurl, ui::PAGE_TRANSITION_AUTO_BOOKMARK);
-      params.window_action = NavigateParams::SHOW_WINDOW;
-      params.user_gesture = true;
-      Navigate(&params);
-      return;
-    }
-
-    // No existing browser window, create one.
-    NavigateParams params(profile, gurl, ui::PAGE_TRANSITION_AUTO_BOOKMARK);
-    params.disposition = WindowOpenDisposition::NEW_POPUP;
-    params.trusted_source = true;
+  if (browser) {
+    NavigateParams params(browser, gurl, ui::PAGE_TRANSITION_AUTO_BOOKMARK);
     params.window_action = NavigateParams::SHOW_WINDOW;
     params.user_gesture = true;
-    params.path_behavior = NavigateParams::IGNORE_AND_NAVIGATE;
     Navigate(&params);
-    browser = params.browser;
-
-    // operator[] not used because SessionID has no default constructor.
-    settings_session_map_.emplace(profile, SessionID::InvalidValue())
-        .first->second = browser->session_id();
-    DCHECK(browser->is_trusted_source());
-
-    auto* window = browser->window()->GetNativeWindow();
-    window->SetProperty(kOverrideWindowIconResourceIdKey,
-                        IDR_SETTINGS_LOGO_192);
-    // For Mash, this is set by BrowserFrameMash.
-    if (!features::IsUsingWindowService()) {
-      window->SetProperty(aura::client::kAppType,
-                          static_cast<int>(ash::AppType::CHROME_APP));
-    }
+    return;
+  }
+
+  // No existing browser window, create one.
+  NavigateParams params(profile, gurl, ui::PAGE_TRANSITION_AUTO_BOOKMARK);
+  params.disposition = WindowOpenDisposition::NEW_POPUP;
+  params.trusted_source = true;
+  params.window_action = NavigateParams::SHOW_WINDOW;
+  params.user_gesture = true;
+  params.path_behavior = NavigateParams::IGNORE_AND_NAVIGATE;
+  Navigate(&params);
+  browser = params.browser;
+
+  // operator[] not used because SessionID has no default constructor.
+  settings_session_map_.emplace(profile, SessionID::InvalidValue())
+      .first->second = browser->session_id();
+  DCHECK(browser->is_trusted_source());
+
+  auto* window = browser->window()->GetNativeWindow();
+  window->SetProperty(kOverrideWindowIconResourceIdKey, IDR_SETTINGS_LOGO_192);
+  // For Mash, this is set by BrowserFrameMash.
+  if (!features::IsUsingWindowService()) {
+    window->SetProperty(aura::client::kAppType,
+                        static_cast<int>(ash::AppType::CHROME_APP));
   }
 
   for (SettingsWindowManagerObserver& observer : observers_)
@@ -137,20 +109,8 @@ void SettingsWindowManager::ShowChromePageForProfile(Profile* profile,
 
 Browser* SettingsWindowManager::FindBrowserForProfile(Profile* profile) {
   if (web_app::SystemWebAppManager::IsEnabled()) {
-    // TODO(calamity): Determine whether, during startup, we need to wait for
-    // app install and then provide a valid answer here.
-    base::Optional<std::string> settings_app_id = GetSettingsAppId(profile);
-    if (!settings_app_id)
-      return nullptr;
-
-    // Check if any settings windows are already running.
-    std::vector<content::WebContents*> running_apps =
-        AppShortcutLauncherItemController::GetRunningApplications(
-            settings_app_id.value());
-    if (!running_apps.empty())
-      return chrome::FindBrowserWithWebContents(running_apps[0]);
-
-    return nullptr;
+    return web_app::FindSystemWebAppBrowser(profile,
+                                            web_app::SystemAppType::SETTINGS);
   }
 
   auto iter = settings_session_map_.find(profile);
@@ -165,11 +125,10 @@ bool SettingsWindowManager::IsSettingsBrowser(Browser* browser) const {
   if (web_app::SystemWebAppManager::IsEnabled()) {
     // TODO(calamity): Determine whether, during startup, we need to wait for
     // app install and then provide a valid answer here.
-    base::Optional<std::string> settings_app_id = GetSettingsAppId(profile);
-    if (!settings_app_id)
-      return false;
-
-    return browser->hosted_app_controller() &&
+    base::Optional<std::string> settings_app_id =
+        web_app::GetAppIdForSystemWebApp(profile,
+                                         web_app::SystemAppType::SETTINGS);
+    return settings_app_id && browser->hosted_app_controller() &&
            browser->hosted_app_controller()->app_id() ==
                settings_app_id.value();
   } else {

chrome/browser/ui/web_applications/system_web_app_ui_utils_chromeos.cc

+// Copyright 2019 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/web_applications/system_web_app_ui_utils_chromeos.h"
+
+#include <string>
+
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/ui/browser_navigator.h"
+#include "chrome/browser/ui/browser_navigator_params.h"
+#include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/extensions/app_launch_params.h"
+#include "chrome/browser/ui/extensions/application_launch.h"
+#include "chrome/browser/web_applications/components/web_app_helpers.h"
+#include "chrome/browser/web_applications/system_web_app_manager.h"
+#include "chrome/browser/web_applications/web_app_provider.h"
+#include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
+#include "content/public/browser/web_contents.h"
+#include "extensions/browser/extension_registry.h"
+#include "extensions/common/extension.h"
+#include "ui/base/mojo/window_open_disposition.mojom.h"
+#include "ui/display/types/display_constants.h"
+
+namespace web_app {
+
+base::Optional<std::string> GetAppIdForSystemWebApp(Profile* profile,
+                                                    SystemAppType app_type) {
+  return WebAppProvider::Get(profile)
+      ->system_web_app_manager()
+      .GetAppIdForSystemApp(app_type);
+}
+
+Browser* LaunchSystemWebApp(Profile* profile,
+                            SystemAppType app_type,
+                            const GURL& url) {
+  Browser* browser = FindSystemWebAppBrowser(profile, app_type);
+  if (browser) {
+    content::WebContents* web_contents =
+        browser->tab_strip_model()->GetWebContentsAt(0);
+    if (web_contents && web_contents->GetURL() == url) {
+      browser->window()->Show();
+      return browser;
+    }
+  }
+
+  base::Optional<std::string> app_id =
+      GetAppIdForSystemWebApp(profile, app_type);
+  // TODO(calamity): Queue a task to launch app after it is installed.
+  if (!app_id)
+    return nullptr;
+
+  const extensions::Extension* extension =
+      extensions::ExtensionRegistry::Get(profile)->GetExtensionById(
+          app_id.value(), extensions::ExtensionRegistry::ENABLED);
+  DCHECK(extension);
+
+  // TODO(calamity): Plumb through better launch sources from callsites.
+  AppLaunchParams params = CreateAppLaunchParamsWithEventFlags(
+      profile, extension, 0, extensions::SOURCE_CHROME_INTERNAL,
+      display::kInvalidDisplayId);
+  params.override_url = url;
+
+  if (!browser)
+    browser = CreateApplicationWindow(params, url);
+
+  ShowApplicationWindow(params, url, browser,
+                        WindowOpenDisposition::CURRENT_TAB);
+  return browser;
+}
+
+Browser* FindSystemWebAppBrowser(Profile* profile, SystemAppType app_type) {
+  // TODO(calamity): Determine whether, during startup, we need to wait for
+  // app install and then provide a valid answer here.
+  base::Optional<std::string> app_id =
+      GetAppIdForSystemWebApp(profile, app_type);
+  if (!app_id)
+    return nullptr;
+
+  const extensions::Extension* extension =
+      extensions::ExtensionRegistry::Get(profile)->GetExtensionById(
+          app_id.value(), extensions::ExtensionRegistry::ENABLED);
+  DCHECK(extension);
+
+  for (auto* browser : *BrowserList::GetInstance()) {
+    if (browser->profile() != profile || !browser->is_app())
+      continue;
+
+    const extensions::Extension* browser_extension =
+        extensions::ExtensionRegistry::Get(browser->profile())
+            ->GetExtensionById(GetAppIdFromApplicationName(browser->app_name()),
+                               extensions::ExtensionRegistry::EVERYTHING);
+    if (browser_extension == extension)
+      return browser;
+  }
+
+  return nullptr;
+}
+
+}  // namespace web_app

chrome/browser/ui/web_applications/system_web_app_ui_utils_chromeos.h

+// Copyright 2019 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_WEB_APPLICATIONS_SYSTEM_WEB_APP_UI_UTILS_CHROMEOS_H_
+#define CHROME_BROWSER_UI_WEB_APPLICATIONS_SYSTEM_WEB_APP_UI_UTILS_CHROMEOS_H_
+
+#include "base/optional.h"
+#include "chrome/browser/web_applications/system_web_app_manager.h"
+#include "url/gurl.h"
+
+class Browser;
+class Profile;
+
+namespace web_app {
+
+// Returns the PWA system App ID for the given system app type.
+base::Optional<std::string> GetAppIdForSystemWebApp(Profile* profile,
+                                                    SystemAppType app_type);
+
+// Launches a System App to the given URL, reusing any existing window for the
+// app. Returns the browser for the System App, or nullptr if launch/focus
+// failed.
+Browser* LaunchSystemWebApp(Profile* profile,
+                            SystemAppType app_type,
+                            const GURL& url = GURL());
+
+// Returns a browser that is hosting the given system app type, or nullptr if
+// not found.
+Browser* FindSystemWebAppBrowser(Profile* profile, SystemAppType app_type);
+
+}  // namespace web_app
+
+#endif  // CHROME_BROWSER_UI_WEB_APPLICATIONS_SYSTEM_WEB_APP_UI_UTILS_CHROMEOS_H_