Desktop PWAs: Web App windows show app title when out of scope

WebAppBrowserController::GetTitle() uses the web app's short
name when the custom tab bar is being shown.

.../extensions/hosted_app_browsertest.cc no longer contains
any web app browser tests - they have all been moved to
.../web_applications/...

Previously landed as https://chromium-review.googlesource.com/c/chromium/src/+/2050286
and reverted due to a failure in another CL's mixed-content browser tests
https://chromium-review.googlesource.com/c/chromium/src/+/2054628

Bug: 966290
Change-Id: Ifc3a1c305eeaf79d8d039bb9edf6c321a6a8738b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2055909
Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Auto-Submit: Eric Willigers <ericwilligers@chromium.org>
Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#741361}

chrome/browser/ui/extensions/hosted_app_browsertest.cc

@@ -222,27 +222,6 @@ class HostedAppTest : public extensions::ExtensionBrowserTest,
 
   static const char* GetInstallableAppName() { return "Manifest test app"; }
 
-  void InstallMixedContentPWA() { return InstallPWA(GetMixedContentAppURL()); }
-
-  void InstallSecurePWA() { return InstallPWA(GetSecureAppURL()); }
-
-  void InstallPWA(const GURL& app_url) {
-    WebApplicationInfo web_app_info;
-    web_app_info.app_url = app_url;
-    web_app_info.scope = app_url.GetWithoutFilename();
-    web_app_info.open_as_window = true;
-
-    app_ = InstallBookmarkApp(web_app_info);
-
-    ui_test_utils::UrlLoadObserver url_observer(
-        app_url, content::NotificationService::AllSources());
-    app_browser_ = LaunchAppBrowser(app_);
-    url_observer.Wait();
-
-    CHECK(app_browser_);
-    CHECK(app_browser_ != browser());
-  }
-
   void SetUpInProcessBrowserTestFixture() override {
     extensions::ExtensionBrowserTest::SetUpInProcessBrowserTestFixture();
     cert_verifier_.SetUpInProcessBrowserTestFixture();
@@ -323,8 +302,6 @@ class HostedAppTest : public extensions::ExtensionBrowserTest,
   DISALLOW_COPY_AND_ASSIGN(HostedAppTest);
 };
 
-using SharedAppTest = HostedAppTest;
-
 // Tests that "Open link in new tab" opens a link in a foreground tab.
 IN_PROC_BROWSER_TEST_P(HostedAppTest, OpenLinkInNewTab) {
   SetupApp("app");
@@ -591,8 +568,8 @@ IN_PROC_BROWSER_TEST_P(HostedAppTest, ShouldShowCustomTabBarForAppWithoutWWW) {
 }
 
 // Check that a subframe on a regular web page can navigate to a URL that
-// redirects to a hosted app.  https://crbug.com/721949.
-IN_PROC_BROWSER_TEST_P(SharedAppTest, SubframeRedirectsToHostedApp) {
+// redirects to a platform app.  https://crbug.com/721949.
+IN_PROC_BROWSER_TEST_P(HostedAppTest, SubframeRedirectsToHostedApp) {
   // This test only applies to hosted apps.
   if (app_type() != AppType::HOSTED_APP)
     return;
@@ -627,27 +604,6 @@ IN_PROC_BROWSER_TEST_P(SharedAppTest, SubframeRedirectsToHostedApp) {
       EvalJs(subframe, "document.body.innerText.trim();").ExtractString());
 }
 
-// Check that no assertions are hit when showing a permission request bubble.
-IN_PROC_BROWSER_TEST_P(HostedAppTest, PermissionBubble) {
-  ASSERT_TRUE(https_server()->Start());
-  ASSERT_TRUE(embedded_test_server()->Start());
-
-  WebApplicationInfo web_app_info;
-  web_app_info.app_url = GetSecureAppURL();
-  const extensions::Extension* app = InstallBookmarkApp(web_app_info);
-
-  ui_test_utils::UrlLoadObserver url_observer(
-      GetSecureAppURL(), content::NotificationService::AllSources());
-  Browser* app_browser = LaunchAppBrowser(app);
-  url_observer.Wait();
-
-  RenderFrameHost* render_frame_host =
-      app_browser->tab_strip_model()->GetActiveWebContents()->GetMainFrame();
-  EXPECT_TRUE(content::ExecuteScript(
-      render_frame_host,
-      "navigator.geolocation.getCurrentPosition(function(){});"));
-}
-
 // Tests that platform apps can still load mixed content.
 IN_PROC_BROWSER_TEST_P(HostedAppTestWithAutoupgradesDisabled,
                        MixedContentInPlatformApp) {
@@ -667,78 +623,6 @@ IN_PROC_BROWSER_TEST_P(HostedAppTestWithAutoupgradesDisabled,
   web_app::CheckMixedContentLoaded(app_browser_);
 }
 
-// Ensure that hosted app windows with blank titles don't display the URL as a
-// default window title.
-IN_PROC_BROWSER_TEST_P(HostedAppTest, EmptyTitlesDoNotDisplayUrl) {
-  ASSERT_TRUE(https_server()->Start());
-  GURL url = https_server()->GetURL("app.site.com", "/empty.html");
-
-  WebApplicationInfo web_app_info;
-  web_app_info.app_url = url;
-  const extensions::Extension* app = InstallBookmarkApp(web_app_info);
-
-  Browser* app_browser = LaunchAppBrowser(app);
-  content::WebContents* web_contents =
-      app_browser->tab_strip_model()->GetActiveWebContents();
-  content::WaitForLoadStop(web_contents);
-  EXPECT_EQ(base::string16(), app_browser->GetWindowTitleForCurrentTab(false));
-  NavigateToURLAndWait(app_browser,
-                       https_server()->GetURL("app.site.com", "/simple.html"));
-  EXPECT_EQ(base::ASCIIToUTF16("OK"),
-            app_browser->GetWindowTitleForCurrentTab(false));
-}
-
-// Ensure that hosted app windows display the app title instead of the page
-// title when off scope.
-IN_PROC_BROWSER_TEST_P(HostedAppTest, OffScopeUrlsDisplayAppTitle) {
-  ASSERT_TRUE(https_server()->Start());
-  GURL url = GetSecureAppURL();
-
-  WebApplicationInfo web_app_info;
-  web_app_info.app_url = url;
-  web_app_info.scope = url.GetWithoutFilename();
-  web_app_info.title = base::ASCIIToUTF16("A Hosted App");
-  const extensions::Extension* app = InstallBookmarkApp(web_app_info);
-
-  Browser* app_browser = LaunchAppBrowser(app);
-  content::WebContents* web_contents =
-      app_browser->tab_strip_model()->GetActiveWebContents();
-  content::WaitForLoadStop(web_contents);
-
-  // When we are within scope, show the page title.
-  EXPECT_EQ(base::ASCIIToUTF16("Google"),
-            app_browser->GetWindowTitleForCurrentTab(false));
-
-  NavigateToURLAndWait(app_browser,
-                       https_server()->GetURL("app.site.com", "/simple.html"));
-
-  // When we are off scope, show the app title.
-  EXPECT_EQ(base::ASCIIToUTF16("A Hosted App"),
-            app_browser->GetWindowTitleForCurrentTab(false));
-}
-
-// Ensure that hosted app windows display the app title instead of the page
-// title when using http.
-IN_PROC_BROWSER_TEST_P(HostedAppTest, InScopeHttpUrlsDisplayAppTitle) {
-  ASSERT_TRUE(embedded_test_server()->Start());
-  GURL url = embedded_test_server()->GetURL("app.site.com", "/simple.html");
-  WebApplicationInfo web_app_info;
-  web_app_info.app_url = url;
-  web_app_info.title = base::ASCIIToUTF16("A Hosted App");
-
-  const extensions::Extension* app = InstallBookmarkApp(web_app_info);
-
-  Browser* app_browser = LaunchAppBrowser(app);
-  content::WebContents* web_contents =
-      app_browser->tab_strip_model()->GetActiveWebContents();
-  content::WaitForLoadStop(web_contents);
-
-  // The page title is "OK" but the page is being served over HTTP, so the app
-  // title should be used instead.
-  EXPECT_EQ(base::ASCIIToUTF16("A Hosted App"),
-            app_browser->GetWindowTitleForCurrentTab(false));
-}
-
 IN_PROC_BROWSER_TEST_P(HostedAppTest, CreatedForInstalledPwaForNonPwas) {
   SetupApp("https_app");
 
@@ -1872,12 +1756,6 @@ INSTANTIATE_TEST_SUITE_P(All,
                          ::testing::Values(AppType::HOSTED_APP,
                                            AppType::BOOKMARK_APP));
 
-INSTANTIATE_TEST_SUITE_P(All,
-                         SharedAppTest,
-                         ::testing::Values(AppType::HOSTED_APP,
-                                           AppType::BOOKMARK_APP,
-                                           AppType::WEB_APP));
-
 INSTANTIATE_TEST_SUITE_P(All,
                          HostedAppTestWithAutoupgradesDisabled,
                          ::testing::Values(AppType::HOSTED_APP));

chrome/browser/ui/web_applications/web_app_browser_controller.cc

@@ -109,6 +109,17 @@ WebAppBrowserController* WebAppBrowserController::AsWebAppBrowserController() {
   return this;
 }
 
+base::string16 WebAppBrowserController::GetTitle() const {
+  // When showing the toolbar, display the name of the app, instead of the
+  // current page as the title.
+  if (ShouldShowCustomTabBar()) {
+    // TODO(crbug.com/1051379): Use name instead of short_name.
+    return base::UTF8ToUTF16(registrar().GetAppShortName(GetAppId()));
+  }
+
+  return AppBrowserController::GetTitle();
+}
+
 std::string WebAppBrowserController::GetAppShortName() const {
   return registrar().GetAppShortName(GetAppId());
 }

chrome/browser/ui/web_applications/web_app_browser_controller.h

@@ -46,6 +46,7 @@ class WebAppBrowserController : public AppBrowserController,
   gfx::ImageSkia GetWindowAppIcon() const override;
   gfx::ImageSkia GetWindowIcon() const override;
   base::Optional<SkColor> GetThemeColor() const override;
+  base::string16 GetTitle() const override;
   std::string GetAppShortName() const override;
   base::string16 GetFormattedUrlOrigin() const override;
   GURL GetAppLaunchURL() const override;

chrome/browser/ui/web_applications/web_app_browsertest.cc

@@ -412,7 +412,7 @@ IN_PROC_BROWSER_TEST_P(WebAppBrowserTest, OverscrollEnabled) {
 #endif
 }
 
-// Check the 'Copy URL' menu button for Hosted App windows.
+// Check the 'Copy URL' menu button for Web App windows.
 IN_PROC_BROWSER_TEST_P(WebAppBrowserTest, CopyURL) {
   const GURL app_url(kExampleURL);
   const AppId app_id = InstallPWA(app_url);
@@ -645,6 +645,122 @@ IN_PROC_BROWSER_TEST_P(WebAppBrowserTest, InstallToShelfContainsAppName) {
             base::UTF8ToUTF16("Install Manifest test app\xE2\x80\xA6"));
 }
 
+// Check that no assertions are hit when showing a permission request bubble.
+IN_PROC_BROWSER_TEST_P(WebAppBrowserTest, PermissionBubble) {
+  ASSERT_TRUE(https_server()->Start());
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  const GURL app_url = GetSecureAppURL();
+  const AppId app_id = InstallPWA(app_url);
+  Browser* const app_browser = LaunchWebAppBrowserAndWait(app_id);
+
+  content::RenderFrameHost* const render_frame_host =
+      app_browser->tab_strip_model()->GetActiveWebContents()->GetMainFrame();
+  EXPECT_TRUE(content::ExecuteScript(
+      render_frame_host,
+      "navigator.geolocation.getCurrentPosition(function(){});"));
+}
+
+// Ensure that web app windows with blank titles don't display the URL as a
+// default window title.
+IN_PROC_BROWSER_TEST_P(WebAppBrowserTest, EmptyTitlesDoNotDisplayUrl) {
+  ASSERT_TRUE(https_server()->Start());
+  const GURL app_url = https_server()->GetURL("app.site.com", "/empty.html");
+  const AppId app_id = InstallPWA(app_url);
+  Browser* const app_browser = LaunchWebAppBrowser(app_id);
+  content::WebContents* const web_contents =
+      app_browser->tab_strip_model()->GetActiveWebContents();
+  content::WaitForLoadStop(web_contents);
+  EXPECT_EQ(base::string16(), app_browser->GetWindowTitleForCurrentTab(false));
+  NavigateToURLAndWait(app_browser,
+                       https_server()->GetURL("app.site.com", "/simple.html"));
+  EXPECT_EQ(base::ASCIIToUTF16("OK"),
+            app_browser->GetWindowTitleForCurrentTab(false));
+}
+
+// Ensure that web app windows display the app title instead of the page
+// title when off scope.
+IN_PROC_BROWSER_TEST_P(WebAppBrowserTest, OffScopeUrlsDisplayAppTitle) {
+  ASSERT_TRUE(https_server()->Start());
+  const GURL app_url = GetSecureAppURL();
+  const base::string16 app_title = base::ASCIIToUTF16("A Web App");
+
+  auto web_app_info = std::make_unique<WebApplicationInfo>();
+  web_app_info->app_url = app_url;
+  web_app_info->scope = app_url.GetWithoutFilename();
+  web_app_info->title = app_title;
+  const AppId app_id = InstallWebApp(std::move(web_app_info));
+
+  Browser* const app_browser = LaunchWebAppBrowser(app_id);
+  content::WebContents* const web_contents =
+      app_browser->tab_strip_model()->GetActiveWebContents();
+  content::WaitForLoadStop(web_contents);
+
+  // When we are within scope, show the page title.
+  EXPECT_EQ(base::ASCIIToUTF16("Google"),
+            app_browser->GetWindowTitleForCurrentTab(false));
+
+  NavigateToURLAndWait(app_browser,
+                       https_server()->GetURL("app.site.com", "/simple.html"));
+
+  // When we are off scope, show the app title.
+  EXPECT_EQ(app_title, app_browser->GetWindowTitleForCurrentTab(false));
+}
+
+// Ensure that web app windows display the app title instead of the page
+// title when using http.
+IN_PROC_BROWSER_TEST_P(WebAppBrowserTest, InScopeHttpUrlsDisplayAppTitle) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  const GURL app_url =
+      embedded_test_server()->GetURL("app.site.com", "/simple.html");
+  const base::string16 app_title = base::ASCIIToUTF16("A Web App");
+
+  auto web_app_info = std::make_unique<WebApplicationInfo>();
+  web_app_info->app_url = app_url;
+  web_app_info->title = app_title;
+  const AppId app_id = InstallWebApp(std::move(web_app_info));
+
+  Browser* const app_browser = LaunchWebAppBrowser(app_id);
+  content::WebContents* const web_contents =
+      app_browser->tab_strip_model()->GetActiveWebContents();
+  content::WaitForLoadStop(web_contents);
+
+  // The page title is "OK" but the page is being served over HTTP, so the app
+  // title should be used instead.
+  EXPECT_EQ(app_title, app_browser->GetWindowTitleForCurrentTab(false));
+}
+
+// Check that a subframe on a regular web page can navigate to a URL that
+// redirects to a web app.  https://crbug.com/721949.
+IN_PROC_BROWSER_TEST_P(WebAppBrowserTest, SubframeRedirectsToWebApp) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  // Set up a web app which covers app.com URLs.
+  GURL app_url = embedded_test_server()->GetURL("app.com", "/title1.html");
+  const AppId app_id = InstallPWA(app_url);
+
+  // Navigate a regular tab to a page with a subframe.
+  const GURL url = embedded_test_server()->GetURL("foo.com", "/iframe.html");
+  content::WebContents* const tab =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  NavigateToURLAndWait(browser(), url);
+
+  // Navigate the subframe to a URL that redirects to a URL in the web app's
+  // web extent.
+  const GURL redirect_url = embedded_test_server()->GetURL(
+      "bar.com", "/server-redirect?" + app_url.spec());
+  EXPECT_TRUE(NavigateIframeToURL(tab, "test", redirect_url));
+
+  // Ensure that the frame navigated successfully and that it has correct
+  // content.
+  content::RenderFrameHost* const subframe =
+      content::ChildFrameAt(tab->GetMainFrame(), 0);
+  EXPECT_EQ(app_url, subframe->GetLastCommittedURL());
+  EXPECT_EQ(
+      "This page has no title.",
+      EvalJs(subframe, "document.body.innerText.trim();").ExtractString());
+}
+
 INSTANTIATE_TEST_SUITE_P(
     All,
     WebAppBrowserTest,

chrome/browser/ui/web_applications/web_app_menu_model.h

@@ -8,7 +8,7 @@
 #include "base/macros.h"
 #include "chrome/browser/ui/toolbar/app_menu_model.h"
 
-// Menu model for the menu button in a hosted app browser window.
+// Menu model for the menu button in a web app browser window.
 class WebAppMenuModel : public AppMenuModel {
  public:
   static constexpr int kUninstallAppCommandId = 1;