[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/+/1504603
Reviewed-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}diff --git a/chrome/browser/chromeos/crostini/crostini_manager.cc b/chrome/browser/chromeos/crostini/crostini_manager.cc
index 6f06558b..cffc74b3 100644
--- a/chrome/browser/chromeos/crostini/crostini_manager.cc
+++ b/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 @@
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);
}
diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn
index 6d146e9..9ec6933f 100644
--- a/chrome/browser/ui/BUILD.gn
+++ b/chrome/browser/ui/BUILD.gn
@@ -3497,6 +3497,8 @@
"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",
diff --git a/chrome/browser/ui/extensions/application_launch.cc b/chrome/browser/ui/extensions/application_launch.cc
index ec228a4..2f1868aef 100644
--- a/chrome/browser/ui/extensions/application_launch.cc
+++ b/chrome/browser/ui/extensions/application_launch.cc
@@ -380,14 +380,15 @@
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* 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) {
diff --git a/chrome/browser/ui/extensions/application_launch.h b/chrome/browser/ui/extensions/application_launch.h
index 80c90bf..6df2cdb 100644
--- a/chrome/browser/ui/extensions/application_launch.h
+++ b/chrome/browser/ui/extensions/application_launch.h
@@ -19,6 +19,8 @@
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 @@
// 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,
diff --git a/chrome/browser/ui/settings_window_manager_chromeos.cc b/chrome/browser/ui/settings_window_manager_chromeos.cc
index 9dd55e3..76e9fa7 100644
--- a/chrome/browser/ui/settings_window_manager_chromeos.cc
+++ b/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 @@
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 @@
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(¶ms);
- 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(¶ms);
- browser = params.browser;
+ return;
+ }
- // 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());
+ // 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(¶ms);
+ browser = params.browser;
- 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));
- }
+ // 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 @@
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 @@
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 {
diff --git a/chrome/browser/ui/web_applications/system_web_app_ui_utils_chromeos.cc b/chrome/browser/ui/web_applications/system_web_app_ui_utils_chromeos.cc
new file mode 100644
index 0000000..5679cca
--- /dev/null
+++ b/chrome/browser/ui/web_applications/system_web_app_ui_utils_chromeos.cc
@@ -0,0 +1,102 @@
+// 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
diff --git a/chrome/browser/ui/web_applications/system_web_app_ui_utils_chromeos.h b/chrome/browser/ui/web_applications/system_web_app_ui_utils_chromeos.h
new file mode 100644
index 0000000..319526c
--- /dev/null
+++ b/chrome/browser/ui/web_applications/system_web_app_ui_utils_chromeos.h
@@ -0,0 +1,34 @@
+// 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_
