Commit 8e096d3a authored by Regan Hsu's avatar Regan Hsu Committed by Commit Bot

[CrOS SplitSettings] Open all chrome://os-settings in standalone surface

With the exception of typing "chrome://os-settings" (or any subpages)
into the url bar, any navigation to OS Settings or its subpages
should open the standalone chrome OS settings surface.

Bug: 990540
Change-Id: I6b868d4f96c045b536afa5e2426f01fe216fd0f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1747071
Commit-Queue: Regan Hsu <hsuregan@chromium.org>
Reviewed-by: default avatarJames Cook <jamescook@chromium.org>
Reviewed-by: default avatarMichael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#687351}
parent 1f5d7092
...@@ -51,6 +51,8 @@ ...@@ -51,6 +51,8 @@
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
#include "ash/public/cpp/multi_user_window_manager.h" #include "ash/public/cpp/multi_user_window_manager.h"
#include "chrome/browser/ui/ash/multi_user/multi_user_window_manager_helper.h" #include "chrome/browser/ui/ash/multi_user/multi_user_window_manager_helper.h"
#include "chrome/browser/ui/settings_window_manager_chromeos.h"
#include "chromeos/constants/chromeos_features.h"
#include "components/account_id/account_id.h" #include "components/account_id/account_id.h"
#endif #endif
...@@ -521,24 +523,41 @@ void Navigate(NavigateParams* params) { ...@@ -521,24 +523,41 @@ void Navigate(NavigateParams* params) {
return; return;
} }
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
if (source_browser && source_browser != params->browser) { if (source_browser) {
// When the newly created browser was spawned by a browser which visits // If OS Settings is accessed in any means other than explicitly typing the
// another user's desktop, it should be shown on the same desktop as the // URL into the URL bar, open OS Settings in its own standalone surface.
// originating one. (This is part of the desktop separation per profile). if (chromeos::features::IsSplitSettingsEnabled() &&
auto* window_manager = MultiUserWindowManagerHelper::GetWindowManager(); params->url.host() == chrome::kChromeUIOSSettingsHost &&
// Some unit tests have no client instantiated. !PageTransitionCoreTypeIs(params->transition,
if (window_manager) { ui::PageTransition::PAGE_TRANSITION_TYPED)) {
aura::Window* src_window = source_browser->window()->GetNativeWindow(); chrome::SettingsWindowManager* settings_window_manager =
aura::Window* new_window = params->browser->window()->GetNativeWindow(); chrome::SettingsWindowManager::GetInstance();
const AccountId& src_account_id = if (!settings_window_manager->IsSettingsBrowser(source_browser)) {
window_manager->GetUserPresentingWindow(src_window); settings_window_manager->ShowChromePageForProfile(
if (src_account_id != GetSourceProfile(params), params->url);
window_manager->GetUserPresentingWindow(new_window)) { return;
// Once the window gets presented, it should be shown on the same }
// desktop as the desktop of the creating browser. Note that this }
// command will not show the window if it wasn't shown yet by the
// browser creation. if (source_browser != params->browser) {
window_manager->ShowWindowForUser(new_window, src_account_id); // When the newly created browser was spawned by a browser which visits
// another user's desktop, it should be shown on the same desktop as the
// originating one. (This is part of the desktop separation per profile).
auto* window_manager = MultiUserWindowManagerHelper::GetWindowManager();
// Some unit tests have no client instantiated.
if (window_manager) {
aura::Window* src_window = source_browser->window()->GetNativeWindow();
aura::Window* new_window = params->browser->window()->GetNativeWindow();
const AccountId& src_account_id =
window_manager->GetUserPresentingWindow(src_window);
if (src_account_id !=
window_manager->GetUserPresentingWindow(new_window)) {
// Once the window gets presented, it should be shown on the same
// desktop as the desktop of the creating browser. Note that this
// command will not show the window if it wasn't shown yet by the
// browser creation.
window_manager->ShowWindowForUser(new_window, src_account_id);
}
} }
} }
} }
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "ash/public/cpp/window_pin_type.h" #include "ash/public/cpp/window_pin_type.h"
#include "ash/public/cpp/window_properties.h" #include "ash/public/cpp/window_properties.h"
#include "base/command_line.h" #include "base/command_line.h"
#include "base/test/scoped_feature_list.h"
#include "chrome/browser/chromeos/login/chrome_restart_request.h" #include "chrome/browser/chromeos/login/chrome_restart_request.h"
#include "chrome/browser/ui/ash/multi_user/multi_user_util.h" #include "chrome/browser/ui/ash/multi_user/multi_user_util.h"
#include "chrome/browser/ui/ash/multi_user/multi_user_window_manager_helper.h" #include "chrome/browser/ui/ash/multi_user/multi_user_window_manager_helper.h"
...@@ -22,6 +23,7 @@ ...@@ -22,6 +23,7 @@
#include "chrome/common/chrome_switches.h" #include "chrome/common/chrome_switches.h"
#include "chrome/common/webui_url_constants.h" #include "chrome/common/webui_url_constants.h"
#include "chrome/test/base/ui_test_utils.h" #include "chrome/test/base/ui_test_utils.h"
#include "chromeos/constants/chromeos_features.h"
#include "chromeos/constants/chromeos_switches.h" #include "chromeos/constants/chromeos_switches.h"
#include "components/account_id/account_id.h" #include "components/account_id/account_id.h"
#include "content/public/browser/notification_service.h" #include "content/public/browser/notification_service.h"
...@@ -37,6 +39,49 @@ GURL GetGoogleURL() { ...@@ -37,6 +39,49 @@ GURL GetGoogleURL() {
using BrowserNavigatorTestChromeOS = BrowserNavigatorTest; using BrowserNavigatorTestChromeOS = BrowserNavigatorTest;
// Verifies that the OS settings page opens in a standalone surface when
// accessed via link or url.
IN_PROC_BROWSER_TEST_F(BrowserNavigatorTestChromeOS, NavigateToOSSettings) {
// Enable SplitSettings feature.
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(chromeos::features::kSplitSettings);
// Install the Settings App.
web_app::WebAppProvider::Get(browser()->profile())
->system_web_app_manager()
.InstallSystemAppsForTesting();
// Verify that only one window is upon before navigating to OS settings.
EXPECT_EQ(1u, chrome::GetTotalBrowserCount());
// Navigate to OS Settings page via typing URL into URL bar.
NavigateParams params(MakeNavigateParams(browser()));
params.url = GURL("chrome://os-settings/");
params.transition = ui::PageTransition::PAGE_TRANSITION_TYPED;
Navigate(&params);
// Verify that navigating to chrome://os-settings/ via typing causes the
// browser itself to navigate to the OS Settings page.
EXPECT_EQ(1u, chrome::GetTotalBrowserCount());
EXPECT_EQ(GURL("chrome://os-settings/"),
browser()->tab_strip_model()->GetActiveWebContents()->GetURL());
// Navigate to OS Settings page via clicking a link on another page.
params.transition = ui::PageTransition::PAGE_TRANSITION_LINK;
Navigate(&params);
Browser* os_settings_browser =
chrome::SettingsWindowManager::GetInstance()->FindBrowserForProfile(
browser()->profile());
// Verify that navigating to chrome://os-settings/ via a link from another
// page opens a standalone surface.
EXPECT_EQ(2u, chrome::GetTotalBrowserCount());
EXPECT_EQ(
GURL("chrome://os-settings/"),
os_settings_browser->tab_strip_model()->GetActiveWebContents()->GetURL());
EXPECT_NE(browser(), os_settings_browser);
}
// This test verifies that the settings page is opened in a new browser window. // This test verifies that the settings page is opened in a new browser window.
IN_PROC_BROWSER_TEST_F(BrowserNavigatorTestChromeOS, NavigateToSettings) { IN_PROC_BROWSER_TEST_F(BrowserNavigatorTestChromeOS, NavigateToSettings) {
// Install the Settings App. // Install the Settings App.
......
...@@ -5,8 +5,10 @@ ...@@ -5,8 +5,10 @@
#include "chrome/browser/ui/settings_window_manager_chromeos.h" #include "chrome/browser/ui/settings_window_manager_chromeos.h"
#include "ash/public/cpp/app_types.h" #include "ash/public/cpp/app_types.h"
#include "ash/public/cpp/multi_user_window_manager.h"
#include "ash/public/cpp/resources/grit/ash_public_unscaled_resources.h" #include "ash/public/cpp/resources/grit/ash_public_unscaled_resources.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/ash/multi_user/multi_user_window_manager_helper.h"
#include "chrome/browser/ui/ash/window_properties.h" #include "chrome/browser/ui/ash/window_properties.h"
#include "chrome/browser/ui/browser_finder.h" #include "chrome/browser/ui/browser_finder.h"
#include "chrome/browser/ui/browser_navigator.h" #include "chrome/browser/ui/browser_navigator.h"
...@@ -26,6 +28,21 @@ ...@@ -26,6 +28,21 @@
namespace chrome { namespace chrome {
namespace {
// This method handles the case of resurfacing the user's OS Settings
// standalone window that may be at the time located on another user's desktop.
void ShowSettingsOnCurrentDesktop(Browser* browser) {
auto* window_manager = MultiUserWindowManagerHelper::GetWindowManager();
if (window_manager && browser) {
window_manager->ShowWindowForUser(browser->window()->GetNativeWindow(),
window_manager->CurrentAccountId());
browser->window()->Show();
}
}
} // namespace
// static // static
SettingsWindowManager* SettingsWindowManager::GetInstance() { SettingsWindowManager* SettingsWindowManager::GetInstance() {
return base::Singleton<SettingsWindowManager>::get(); return base::Singleton<SettingsWindowManager>::get();
...@@ -54,6 +71,7 @@ void SettingsWindowManager::ShowChromePageForProfile(Profile* profile, ...@@ -54,6 +71,7 @@ void SettingsWindowManager::ShowChromePageForProfile(Profile* profile,
bool did_create; bool did_create;
Browser* browser = web_app::LaunchSystemWebApp( Browser* browser = web_app::LaunchSystemWebApp(
profile, web_app::SystemAppType::SETTINGS, gurl, &did_create); profile, web_app::SystemAppType::SETTINGS, gurl, &did_create);
ShowSettingsOnCurrentDesktop(browser);
// Only notify if we created a new browser. // Only notify if we created a new browser.
if (!did_create || !browser) if (!did_create || !browser)
return; return;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment