[CrOS settings] Fix issue where os settings opens in browser

Before this CL, os settings could be opened in the browser. This
CL removes a check that prevented os settings from only being
opened in PWA (settings app).

Removed Disposition_OSSettings_UseNonIncognitoWindow because this test
checks if os-settings is opened in a browser but not opened when browser
is in incognito mode. As os-settings no longer opens in browser this
fails every time.

Fixed: 871144
Change-Id: Idc142e47e7c058597afd6bc7d5402a511119d696
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2086316
Commit-Queue: Nnamdi Theodore Johnson-kanu <tjohnsonkanu@google.com>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751819}

chrome/browser/ui/browser_navigator.cc

@@ -89,6 +89,8 @@ class BrowserNavigatorWebContentsAdoption {
 
 namespace {
 
+bool allow_os_settings_in_tab = false;
+
 // Returns true if the specified Browser can open tabs. Not all Browsers support
 // multiple tabs, such as app frames and popups. This function returns false for
 // those types of Browser.
@@ -536,11 +538,9 @@ void Navigate(NavigateParams* params) {
   }
 #if defined(OS_CHROMEOS)
   if (source_browser) {
-    // If OS Settings is accessed in any means other than explicitly typing the
-    // URL into the URL bar, open OS Settings in its own standalone surface.
+    // Open OS settings in PWA, even when user types in URL bar.
     if (params->url.host() == chrome::kChromeUIOSSettingsHost &&
-        !PageTransitionCoreTypeIs(params->transition,
-                                  ui::PageTransition::PAGE_TRANSITION_TYPED)) {
+        !allow_os_settings_in_tab) {
       chrome::SettingsWindowManager* settings_window_manager =
           chrome::SettingsWindowManager::GetInstance();
       if (!settings_window_manager->IsSettingsBrowser(source_browser)) {
@@ -801,3 +801,7 @@ bool IsURLAllowedInIncognito(const GURL& url,
 
   return IsHostAllowedInIncognito(url);
 }
+
+void SetAllowOsSettingsInTabForTesting(bool is_allowed) {
+  allow_os_settings_in_tab = is_allowed;
+}

chrome/browser/ui/browser_navigator.h

@@ -20,4 +20,8 @@ void Navigate(NavigateParams* params);
 bool IsURLAllowedInIncognito(const GURL& url,
                              content::BrowserContext* browser_context);
 
+// Allows browsertests to open OS settings in a tab. Real users can only open
+// settings in a system app.
+void SetAllowOsSettingsInTabForTesting(bool is_allowed);
+
 #endif  // CHROME_BROWSER_UI_BROWSER_NAVIGATOR_H_

chrome/browser/ui/browser_navigator_browsertest_chromeos.cc

@@ -38,17 +38,13 @@ GURL GetGoogleURL() {
 
 using BrowserNavigatorTestChromeOS = BrowserNavigatorTest;
 
-// This test verifies that the OS Settings page isn't opened in the incognito
-// window.
-IN_PROC_BROWSER_TEST_F(BrowserNavigatorTestChromeOS,
-                       Disposition_OSSettings_UseNonIncognitoWindow) {
-  RunUseNonIncognitoWindowTest(GURL(chrome::kChromeUIOSSettingsURL),
-                               ui::PageTransition::PAGE_TRANSITION_TYPED);
-}
-
 // Verifies that the OS settings page opens in a standalone surface when
 // accessed via link or url.
 IN_PROC_BROWSER_TEST_F(BrowserNavigatorTestChromeOS, NavigateToOSSettings) {
+  // By default, browsertests open settings in a browser tab. For this test, we
+  // verify that if this flag is not set, settings opens in the settings app.
+  // This simulates the default case users see.
+  SetAllowOsSettingsInTabForTesting(false);
   // Install the Settings App.
   web_app::WebAppProvider::Get(browser()->profile())
       ->system_web_app_manager()
@@ -63,10 +59,10 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTestChromeOS, NavigateToOSSettings) {
   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/"),
+  // Verify that navigating to chrome://os-settings/ via typing does not cause
+  // the browser itself to navigate to the OS Settings page.
+  EXPECT_NE(1u, chrome::GetTotalBrowserCount());
+  EXPECT_NE(GURL("chrome://os-settings/"),
             browser()->tab_strip_model()->GetActiveWebContents()->GetURL());
 
   // Navigate to OS Settings page via clicking a link on another page.

chrome/test/base/in_process_browser_test.cc

@@ -245,6 +245,10 @@ void InProcessBrowserTest::SetUp() {
                                       chrome::kTestUserProfileDir);
     }
   }
+
+  // By default, OS settings are not opened in a browser tab but in settings
+  // app. OS browsertests require OS settings to be opened in a browser tab.
+  SetAllowOsSettingsInTabForTesting(true);
 #endif
 
   SetScreenInstance();