AppBrowserController TitlebarToolbar visibility controls

Create a default implementation of
AppBrowserController::HasTitlebarToolbar which removes the toolbar
for System Apps (except Terminal), but shows it for all other apps.

Added controls for app origin text, and content settings to remove
these for System Apps.

Bug: 846546
Change-Id: I9a4083cd78c81736533a2dd275aba70d32a6c233
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1763268Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690439}

chrome/browser/ui/extensions/hosted_app_browser_controller.cc

@@ -205,11 +205,6 @@ bool HostedAppBrowserController::ShouldShowCustomTabBar() const {
   return false;
 }
 
-bool HostedAppBrowserController::HasTitlebarToolbar() const {
-  // System Web Apps don't have a toolbar.
-  return IsForWebAppBrowser(browser()) && !IsForSystemWebApp();
-}
-
 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

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

chrome/browser/ui/manifest_web_app_browser_controller.cc

@@ -52,10 +52,6 @@ bool ManifestWebAppBrowserController::ShouldShowCustomTabBar() const {
   return false;
 }
 
-bool ManifestWebAppBrowserController::HasTitlebarToolbar() const {
-  return true;
-}
-
 gfx::ImageSkia ManifestWebAppBrowserController::GetWindowAppIcon() const {
   gfx::ImageSkia page_icon = browser()->GetCurrentPageIcon().AsImageSkia();
   if (!page_icon.isNull())

chrome/browser/ui/manifest_web_app_browser_controller.h

@@ -32,7 +32,6 @@ class ManifestWebAppBrowserController : public web_app::AppBrowserController {
   // web_app::AppBrowserController:
   base::Optional<std::string> GetAppId() const override;
   bool ShouldShowCustomTabBar() const override;
-  bool HasTitlebarToolbar() const override;
   gfx::ImageSkia GetWindowAppIcon() const override;
   gfx::ImageSkia GetWindowIcon() const override;
   std::string GetAppShortName() const override;

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

@@ -202,13 +202,18 @@ HostedAppButtonContainer::HostedAppButtonContainer(
   layout.set_cross_axis_alignment(
       views::BoxLayout::CrossAxisAlignment::kCenter);
 
+  auto* app_controller = browser_view_->browser()->app_controller();
+  if (app_controller->HasTitlebarAppOriginText()) {
     hosted_app_origin_text_ = AddChildView(
         std::make_unique<HostedAppOriginText>(browser_view->browser()));
+  }
 
+  if (app_controller->HasTitlebarContentSettings()) {
     content_settings_container_ =
         AddChildView(std::make_unique<ContentSettingsContainer>(this));
     views::SetHitTestComponent(content_settings_container_,
                                static_cast<int>(HTCLIENT));
+  }
 
   OmniboxPageActionIconContainerView::Params params;
   params.types_enabled.push_back(PageActionIconType::kFind);
@@ -259,6 +264,7 @@ HostedAppButtonContainer::~HostedAppButtonContainer() {
 }
 
 void HostedAppButtonContainer::UpdateStatusIconsVisibility() {
+  if (content_settings_container_)
     content_settings_container_->UpdateContentSettingViewsVisibility();
   omnibox_page_action_icon_container_view_->UpdateAll();
 }
@@ -351,6 +357,7 @@ void HostedAppButtonContainer::OnContentSettingImageBubbleShown(
 void HostedAppButtonContainer::OnImmersiveRevealStarted() {
   // Don't wait for the fade in animation to make content setting icons visible
   // once in immersive mode.
+  if (content_settings_container_)
     content_settings_container_->EnsureVisible();
 }
 
@@ -427,6 +434,7 @@ void HostedAppButtonContainer::OnWidgetVisibilityChanged(views::Widget* widget,
     return;
   pending_widget_visibility_ = false;
   if (ShouldAnimate()) {
+    if (content_settings_container_)
       content_settings_container_->SetUpForFadeIn();
     animation_start_delay_.Start(
         FROM_HERE, kTitlebarAnimationDelay, this,
@@ -462,6 +470,7 @@ void HostedAppButtonContainer::StartTitlebarAnimation() {
   if (!ShouldAnimate())
     return;
 
+  if (hosted_app_origin_text_)
     hosted_app_origin_text_->StartFadeAnimation();
   app_menu_button_->StartHighlightAnimation();
   icon_fade_in_delay_.Start(
@@ -470,6 +479,7 @@ void HostedAppButtonContainer::StartTitlebarAnimation() {
 }
 
 void HostedAppButtonContainer::FadeInContentSettingIcons() {
+  if (content_settings_container_)
     content_settings_container_->FadeIn();
 }
 
@@ -492,7 +502,9 @@ SkColor HostedAppButtonContainer::GetCaptionColor() const {
 
 void HostedAppButtonContainer::UpdateChildrenColor() {
   SkColor icon_color = GetCaptionColor();
+  if (hosted_app_origin_text_)
     hosted_app_origin_text_->SetTextColor(icon_color);
+  if (content_settings_container_)
     content_settings_container_->SetIconColor(icon_color);
   omnibox_page_action_icon_container_view_->SetIconColor(icon_color);
   app_menu_button_->SetColor(icon_color);

chrome/browser/ui/web_applications/app_browser_controller.cc

@@ -96,6 +96,27 @@ bool AppBrowserController::HasTabStrip() const {
                                  SystemAppType::TERMINAL) == GetAppId();
 }
 
+bool AppBrowserController::HasTitlebarToolbar() const {
+  // Show titlebar toolbar for Terminal System App, but not other system apps.
+  // TODO(crbug.com/846546): Generalise this as a SystemWebApp capability.
+  if (IsForSystemWebApp()) {
+    return GetAppIdForSystemWebApp(browser()->profile(),
+                                   SystemAppType::TERMINAL) == GetAppId();
+  }
+  // Show for all other apps.
+  return true;
+}
+
+bool AppBrowserController::HasTitlebarAppOriginText() const {
+  // Do not show origin text for System Apps.
+  return !IsForSystemWebApp();
+}
+
+bool AppBrowserController::HasTitlebarContentSettings() const {
+  // Do not show content settings for System Apps.
+  return !IsForSystemWebApp();
+}
+
 bool AppBrowserController::IsInstalled() const {
   return false;
 }

chrome/browser/ui/web_applications/app_browser_controller.h

@@ -62,7 +62,13 @@ class AppBrowserController : public TabStripModelObserver,
 
   // Whether the browser toolbar is present.
   // Note: web app windows have their browser toolbar inline in their titlebar.
-  virtual bool HasTitlebarToolbar() const = 0;
+  virtual bool HasTitlebarToolbar() const;
+
+  // Whether to show app origin text in the titlebar toolbar.
+  virtual bool HasTitlebarAppOriginText() const;
+
+  // Whether to show content settings in the titlebar toolbar.
+  virtual bool HasTitlebarContentSettings() const;
 
   // Returns the app icon for the window to use in the task list.
   virtual gfx::ImageSkia GetWindowAppIcon() const = 0;

chrome/browser/ui/web_applications/web_app_browser_controller.cc

@@ -38,10 +38,6 @@ bool WebAppBrowserController::ShouldShowCustomTabBar() const {
   return false;
 }
 
-bool WebAppBrowserController::HasTitlebarToolbar() const {
-  return true;
-}
-
 gfx::ImageSkia WebAppBrowserController::GetWindowAppIcon() const {
   // TODO(https://crbug.com/966290): Complete implementation.
   return gfx::ImageSkia();

chrome/browser/ui/web_applications/web_app_browser_controller.h

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