Stops showing the toolbar for SystemWebApps

Bug: 906508
Change-Id: I7a1484874fd2afe1d7b4603b0de1ae6f20d08bbf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1624066
Commit-Queue: Jay Harris <harrisjay@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662986}

chrome/browser/ui/extensions/hosted_app_browser_controller.cc

@@ -183,6 +183,11 @@ bool HostedAppBrowserController::ShouldShowToolbar() const {
     return true;
   }
 
+  if (IsForSystemWebApp()) {
+    DCHECK_EQ(last_committed_url.scheme_piece(), content::kChromeUIScheme);
+    return false;
+  }
+
   // Page URLs that are not within scope
   // (https://www.w3.org/TR/appmanifest/#dfn-within-scope) of the app
   // corresponding to |launch_url| show the toolbar.

chrome/browser/ui/extensions/hosted_app_browsertest.cc

@@ -65,6 +65,7 @@
 #include "content/public/common/content_features.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/context_menu_params.h"
+#include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "content/public/test/test_utils.h"
@@ -1613,6 +1614,43 @@ IN_PROC_BROWSER_TEST_P(HostedAppPWAOnlyTest, ShouldShowToolbarForSystemApp) {
   NavigateAndCheckForToolbar(app_browser_, app_url, false);
 }
 
+// Check the toolbar is not shown for system web apps for pages on the chrome://
+// scheme.
+IN_PROC_BROWSER_TEST_P(HostedAppPWAOnlyTest,
+                       ShouldNotShowToolbarForSystemWebAppInChrome) {
+  const GURL app_url(chrome::kChromeUISettingsURL);
+
+  SetupSystemAppWithURL(app_url);
+
+  // Navigate to the app's launch page; the toolbar should be hidden.
+  NavigateAndCheckForToolbar(app_browser_, app_url, false);
+
+  // Because the first part of the url is on a different origin (settings vs.
+  // foo) a toolbar would normally be shown. However, because settings is a
+  // SystemWebApp and foo is served via chrome:// it is okay not to show the
+  // toolbar.
+  GURL out_of_scope_chrome_page(app_url.spec() + "://foo");
+  LOG(ERROR) << out_of_scope_chrome_page;
+  NavigateAndCheckForToolbar(app_browser_, out_of_scope_chrome_page, false);
+}
+
+// Check the toolbar is shown for system web apps on pages not on the chrome://
+// scheme. Note: We should never normally get into this scenario.
+IN_PROC_BROWSER_TEST_P(HostedAppPWAOnlyTest,
+                       ShouldShowToolbarForSystemWebAppOnInternet) {
+  const GURL app_url(chrome::kChromeUISettingsURL);
+
+  SetupSystemAppWithURL(app_url);
+
+  // Navigate to the app's launch page; the toolbar should be hidden.
+  NavigateAndCheckForToolbar(app_browser_, app_url, false);
+
+  // Even though the url is secure it is not being served over chrome:// so a
+  // toolbar should be shown.
+  GURL off_scheme_page("https://example.com");
+  NavigateAndCheckForToolbar(app_browser_, off_scheme_page, true);
+}
+
 // Common app manifest for HostedAppProcessModelTests.
 constexpr const char kHostedAppProcessModelManifest[] =
     R"( { "name": "Hosted App Process Model Test",