desktop-pwas: Support custom tab bar for BMO

The implementation of ShouldShowCustomTabBar is moved up
from HostedAppBrowserController to AppBrowserController, with
dependencies on extensions removed.

The CustomTabBarViewBrowserTest tests now pass with BMO.

TBR=bsep@chromium.org

Bug: 966290
Change-Id: I541964d2deb5d30e231bdd79d199c14b26321040
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1871154
Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708096}

chrome/browser/ui/extensions/hosted_app_browser_controller.cc

@@ -110,82 +110,6 @@ bool HostedAppBrowserController::IsHostedApp() const {
   return true;
 }
 
-bool HostedAppBrowserController::ShouldShowCustomTabBar() const {
-  const Extension* extension = GetExtension();
-  if (!extension)
-    return false;
-
-  DCHECK(extension->is_hosted_app());
-
-  content::WebContents* web_contents =
-      browser()->tab_strip_model()->GetActiveWebContents();
-
-  if (!web_contents)
-    return false;
-
-  GURL launch_url = AppLaunchInfo::GetLaunchWebURL(extension);
-  base::StringPiece launch_scheme = launch_url.scheme_piece();
-
-  bool is_internal_launch_scheme = launch_scheme == kExtensionScheme ||
-                                   launch_scheme == content::kChromeUIScheme;
-
-  // The current page must be secure for us to hide the toolbar. However,
-  // the chrome-extension:// and chrome:// launch URL apps can hide the toolbar,
-  // if the current WebContents URLs are the same as the launch scheme.
-  //
-  // Note that the launch scheme may be insecure, but as long as the current
-  // page's scheme is secure, we can hide the toolbar.
-  base::StringPiece secure_page_scheme =
-      is_internal_launch_scheme ? launch_scheme : url::kHttpsScheme;
-
-  auto should_show_toolbar_for_url = [&](const GURL& url) -> bool {
-    // If the url is unset, it doesn't give a signal as to whether the toolbar
-    // should be shown or not. In lieu of more information, do not show the
-    // toolbar.
-    if (url.is_empty())
-      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.
-    bool out_of_scope = !IsUrlInAppScope(url);
-
-    if (url.scheme_piece() != secure_page_scheme) {
-      // Some origins are (such as localhost) are considered secure even when
-      // served over non-secure schemes. However, in order to hide the toolbar,
-      // the 'considered secure' origin must also be in the app's scope.
-      return out_of_scope || !InstallableManager::IsOriginConsideredSecure(url);
-    }
-
-    if (IsForSystemWebApp()) {
-      DCHECK_EQ(url.scheme_piece(), content::kChromeUIScheme);
-      return false;
-    }
-
-    return out_of_scope;
-  };
-
-  GURL visible_url = web_contents->GetVisibleURL();
-  GURL last_committed_url = web_contents->GetLastCommittedURL();
-
-  if (last_committed_url.is_empty() && visible_url.is_empty())
-    return should_show_toolbar_for_url(initial_url());
-
-  if (should_show_toolbar_for_url(visible_url) ||
-      should_show_toolbar_for_url(last_committed_url)) {
-    return true;
-  }
-
-  // Insecure external web sites show the toolbar.
-  // Note: IsContentSecure is false until a navigation is committed.
-  if (!last_committed_url.is_empty() && !is_internal_launch_scheme &&
-      !InstallableManager::IsContentSecure(web_contents)) {
-    return true;
-  }
-
-  return false;
-}
-
 gfx::ImageSkia HostedAppBrowserController::GetWindowAppIcon() const {
   // TODO(calamity): Use the app name to retrieve the app icon without using the
   // extensions tab helper to make icon load more immediate.

chrome/browser/ui/extensions/hosted_app_browser_controller.h

@@ -42,7 +42,6 @@ class HostedAppBrowserController : public web_app::AppBrowserController {
   // web_app::AppBrowserController:
   base::Optional<std::string> GetAppId() const override;
   bool CreatedForInstalledPwa() const override;
-  bool ShouldShowCustomTabBar() const override;
   gfx::ImageSkia GetWindowAppIcon() const override;
   gfx::ImageSkia GetWindowIcon() const override;
   base::Optional<SkColor> GetThemeColor() const override;

chrome/browser/ui/views/frame/immersive_mode_controller_ash_browsertest.cc

@@ -45,8 +45,10 @@ class ImmersiveModeControllerAshWebAppBrowserTest
     https_server_.AddDefaultHandlers(GetChromeTestDataDir());
     ASSERT_TRUE(https_server_.Start());
 
+    const GURL app_url = GetAppUrl();
     auto web_app_info = std::make_unique<WebApplicationInfo>();
-    web_app_info->app_url = GetAppUrl();
+    web_app_info->app_url = app_url;
+    web_app_info->scope = app_url.GetWithoutFilename();
     web_app_info->theme_color = SK_ColorBLUE;
 
     app_id = InstallWebApp(std::move(web_app_info));

chrome/browser/ui/views/location_bar/custom_tab_bar_view_browsertest.cc

@@ -213,6 +213,7 @@ class CustomTabBarViewBrowserTest
   void InstallBookmark(const GURL& app_url) {
     auto web_app_info = std::make_unique<WebApplicationInfo>();
     web_app_info->app_url = app_url;
+    web_app_info->scope = app_url.GetOrigin();
     web_app_info->open_as_window = true;
     Install(std::move(web_app_info));
   }
@@ -676,8 +677,10 @@ IN_PROC_BROWSER_TEST_P(CustomTabBarViewBrowserTest, InterstitialCanHideOrigin) {
       app_view->toolbar()->custom_tab_bar()->IsShowingOriginForTesting());
 }
 
-// TODO(crbug.com/966290): Support kUnifiedControllerWithWebApp/BookmarkApp
 INSTANTIATE_TEST_SUITE_P(
     /* no prefix */,
     CustomTabBarViewBrowserTest,
-    ::testing::Values(web_app::ControllerType::kHostedAppController));
+    ::testing::Values(
+        web_app::ControllerType::kHostedAppController,
+        web_app::ControllerType::kUnifiedControllerWithBookmarkApp,
+        web_app::ControllerType::kUnifiedControllerWithWebApp));

chrome/browser/ui/web_applications/app_browser_controller.cc

@@ -5,6 +5,8 @@
 #include "chrome/browser/ui/web_applications/app_browser_controller.h"
 
 #include "base/feature_list.h"
+#include "base/strings/string_piece.h"
+#include "chrome/browser/installable/installable_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ssl/security_state_tab_helper.h"
 #include "chrome/browser/ui/browser.h"
@@ -25,7 +27,9 @@
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/url_constants.h"
 #include "extensions/browser/extension_registry.h"
+#include "extensions/common/constants.h"
 #include "net/base/escape.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/favicon_size.h"
@@ -91,6 +95,80 @@ bool AppBrowserController::CreatedForInstalledPwa() const {
   return false;
 }
 
+bool AppBrowserController::ShouldShowCustomTabBar() const {
+  if (!IsInstalled())
+    return false;
+
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  if (!web_contents)
+    return false;
+
+  GURL launch_url = GetAppLaunchURL();
+  base::StringPiece launch_scheme = launch_url.scheme_piece();
+
+  bool is_internal_launch_scheme =
+      launch_scheme == extensions::kExtensionScheme ||
+      launch_scheme == content::kChromeUIScheme;
+
+  // The current page must be secure for us to hide the toolbar. However,
+  // chrome:// launch URL apps can hide the toolbar,
+  // if the current WebContents URLs are the same as the launch scheme.
+  //
+  // Note that the launch scheme may be insecure, but as long as the current
+  // page's scheme is secure, we can hide the toolbar.
+  base::StringPiece secure_page_scheme =
+      is_internal_launch_scheme ? launch_scheme : url::kHttpsScheme;
+
+  auto should_show_toolbar_for_url = [&](const GURL& url) -> bool {
+    // If the url is unset, it doesn't give a signal as to whether the toolbar
+    // should be shown or not. In lieu of more information, do not show the
+    // toolbar.
+    if (url.is_empty())
+      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.
+    bool out_of_scope = !IsUrlInAppScope(url);
+
+    if (url.scheme_piece() != secure_page_scheme) {
+      // Some origins are (such as localhost) are considered secure even when
+      // served over non-secure schemes. However, in order to hide the toolbar,
+      // the 'considered secure' origin must also be in the app's scope.
+      return out_of_scope || !InstallableManager::IsOriginConsideredSecure(url);
+    }
+
+    if (IsForSystemWebApp()) {
+      DCHECK_EQ(url.scheme_piece(), content::kChromeUIScheme);
+      return false;
+    }
+
+    return out_of_scope;
+  };
+
+  GURL visible_url = web_contents->GetVisibleURL();
+  GURL last_committed_url = web_contents->GetLastCommittedURL();
+
+  if (last_committed_url.is_empty() && visible_url.is_empty())
+    return should_show_toolbar_for_url(initial_url());
+
+  if (should_show_toolbar_for_url(visible_url) ||
+      should_show_toolbar_for_url(last_committed_url)) {
+    return true;
+  }
+
+  // Insecure external web sites show the toolbar.
+  // Note: IsContentSecure is false until a navigation is committed.
+  if (!last_committed_url.is_empty() && !is_internal_launch_scheme &&
+      !InstallableManager::IsContentSecure(web_contents)) {
+    return true;
+  }
+
+  return false;
+}
+
 bool AppBrowserController::HasTabStrip() const {
   // Show tabs for Terminal only.
   // TODO(crbug.com/846546): Generalise this as a SystemWebApp capability.

chrome/browser/ui/web_applications/app_browser_controller.h

@@ -54,7 +54,7 @@ class AppBrowserController : public TabStripModelObserver,
   virtual bool CreatedForInstalledPwa() const;
 
   // Whether the custom tab bar should be visible.
-  virtual bool ShouldShowCustomTabBar() const = 0;
+  virtual bool ShouldShowCustomTabBar() const;
 
   // Whether the browser should include the tab strip.
   virtual bool HasTabStrip() const;

chrome/browser/ui/web_applications/web_app_browser_controller.cc

@@ -11,6 +11,7 @@
 #include "chrome/browser/web_applications/components/app_icon_manager.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "chrome/browser/web_applications/web_app_provider.h"
+#include "content/public/browser/web_contents.h"
 #include "ui/gfx/favicon_size.h"
 #include "ui/gfx/image/image.h"
 #include "url/gurl.h"
@@ -41,11 +42,6 @@ void WebAppBrowserController::SetReadIconCallbackForTesting(
   callback_for_testing_ = std::move(callback);
 }
 
-bool WebAppBrowserController::ShouldShowCustomTabBar() const {
-  // TODO(https://crbug.com/966290): Complete implementation.
-  return false;
-}
-
 gfx::ImageSkia WebAppBrowserController::GetWindowAppIcon() const {
   if (app_icon_)
     return *app_icon_;

chrome/browser/ui/web_applications/web_app_browser_controller.h

@@ -39,7 +39,6 @@ class WebAppBrowserController : public AppBrowserController {
   // AppBrowserController:
   base::Optional<AppId> GetAppId() const override;
   bool CreatedForInstalledPwa() const override;
-  bool ShouldShowCustomTabBar() const override;
   gfx::ImageSkia GetWindowAppIcon() const override;
   gfx::ImageSkia GetWindowIcon() const override;
   base::Optional<SkColor> GetThemeColor() const override;