Make Browser::SupportsWindowFeatureImpl more declarative.

This CL pivots the logic of SupportsWindowFeatureImpl in order to make
the calculation of each feature simpler and more declarative.
Dependencies for each feature in each Browser Type are now easier to read
and will be easier to extend in the future.

More context is available at
https://docs.google.com/document/d/1n25kZA3KEeKGDqaf8BSevrhW3OVW7oaTQhulG0C8f6U/edit

Bug: 992834,990158
Change-Id: I8f181bedf0b36c11afa50c4a832c4265aa9f7bfe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1741568Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686310}

chrome/browser/ui/browser.cc

@@ -918,11 +918,15 @@ void Browser::ToggleFullscreenModeWithExtension(const GURL& extension_url) {
 }
 
 bool Browser::SupportsWindowFeature(WindowFeature feature) const {
-  return SupportsWindowFeatureImpl(feature, true);
+  bool supports =
+      SupportsWindowFeatureImpl(feature, /*check_can_support=*/false);
+  // Supported features imply CanSupportWindowFeature.
+  DCHECK(!supports || CanSupportWindowFeature(feature));
+  return supports;
 }
 
 bool Browser::CanSupportWindowFeature(WindowFeature feature) const {
-  return SupportsWindowFeatureImpl(feature, false);
+  return SupportsWindowFeatureImpl(feature, /*check_can_support=*/true);
 }
 
 void Browser::OpenFile() {
@@ -2596,62 +2600,101 @@ void Browser::UpdateWindowForLoadingStateChanged(content::WebContents* source,
   }
 }
 
-bool Browser::SupportsLocationBar() const {
-  // Tabbed browser always show a location bar.
-  if (is_type_tabbed())
-    return true;
-
-  // Non-app windows that aren't tabbed or system windows should always show a
-  // location bar, unless they are from a trusted source.
-  if (!is_app())
-    return !is_trusted_source();
+bool Browser::TabbedBrowserSupportsWindowFeature(WindowFeature feature,
+                                                 bool check_can_support) const {
+  bool fullscreen = ShouldHideUIForFullscreen();
+  switch (feature) {
+    case FEATURE_INFOBAR:
+    case FEATURE_DOWNLOADSHELF:
+    case FEATURE_BOOKMARKBAR:
+      return true;
+    case FEATURE_TABSTRIP:
+    case FEATURE_TOOLBAR:
+    case FEATURE_LOCATIONBAR:
+      return check_can_support || !fullscreen;
+    case FEATURE_TITLEBAR:
+    case FEATURE_NONE:
+      return false;
+  }
+}
 
-  // Web apps always support a location bar.
-  if (app_controller_)
-    return true;
+bool Browser::PopupBrowserSupportsWindowFeature(WindowFeature feature,
+                                                bool check_can_support) const {
+  bool fullscreen = ShouldHideUIForFullscreen();
 
-  return false;
+  switch (feature) {
+    case FEATURE_INFOBAR:
+    case FEATURE_DOWNLOADSHELF:
+      return true;
+    case FEATURE_TITLEBAR:
+    case FEATURE_LOCATIONBAR:
+      return check_can_support || (!fullscreen && !is_trusted_source());
+    case FEATURE_TABSTRIP:
+    case FEATURE_TOOLBAR:
+    case FEATURE_BOOKMARKBAR:
+    case FEATURE_NONE:
+      return false;
+  }
+}
+
+bool Browser::LegacyAppBrowserSupportsWindowFeature(
+    WindowFeature feature,
+    bool check_can_support) const {
+  bool fullscreen = ShouldHideUIForFullscreen();
+  switch (feature) {
+    case FEATURE_TITLEBAR:
+      return check_can_support || !fullscreen;
+    case FEATURE_LOCATIONBAR:
+      return false;
+    default:
+      return PopupBrowserSupportsWindowFeature(feature, check_can_support);
+  }
+}
+
+bool Browser::WebAppBrowserSupportsWindowFeature(WindowFeature feature,
+                                                 bool check_can_support) const {
+  DCHECK(app_controller_);
+  bool fullscreen = ShouldHideUIForFullscreen();
+  switch (feature) {
+    // Web apps should always support the toolbar, so the title/origin of the
+    // current page can be shown when browsing a url that is not inside the app.
+    // Note: Final determination of whether or not the toolbar is shown is made
+    // by the |AppBrowserController|.
+    // TODO(crbug.com/992834): Make this control the visibility of Browser
+    // Controls more generally.
+    case FEATURE_TOOLBAR:
+    case FEATURE_INFOBAR:
+    case FEATURE_DOWNLOADSHELF:
+      return true;
+    case FEATURE_TITLEBAR:
+    // TODO(crbug.com/992834): Make this control the visibility of
+    // CustomTabBarView.
+    case FEATURE_LOCATIONBAR:
+      return check_can_support || !fullscreen;
+    case FEATURE_TABSTRIP:
+      return app_controller_->HasTabStrip();
+    case FEATURE_BOOKMARKBAR:
+    case FEATURE_NONE:
+      return false;
+  }
 }
 
 bool Browser::SupportsWindowFeatureImpl(WindowFeature feature,
-                                        bool check_fullscreen) const {
-  bool hide_ui_for_fullscreen = check_fullscreen && ShouldHideUIForFullscreen();
-
-  unsigned int features = FEATURE_INFOBAR | FEATURE_DOWNLOADSHELF;
-
-  // Bookmark bar could be present even if the top UI is hidden in fullscreen
-  // mode, such as when it is in 'detached' mode on NTP. Therefore we
-  // support this feature regardless of |hide_ui_for_fullscreen| and manage
-  // its visibility based on its own state.
-  if (is_type_tabbed())
-    features |= FEATURE_BOOKMARKBAR;
-
-  if (!hide_ui_for_fullscreen) {
-    if (!is_type_tabbed())
-      features |= FEATURE_TITLEBAR;
-
-    if (is_type_tabbed())
-      features |= FEATURE_TABSTRIP;
+                                        bool check_can_support) const {
+  // TODO(crbug.com/992834): Change to TYPE_WEB_APP.
+  if (app_controller_)
+    return WebAppBrowserSupportsWindowFeature(feature, check_can_support);
 
-    if (is_type_tabbed())
-      features |= FEATURE_TOOLBAR;
+  // TODO(crbug.com/992834): Change to TYPE_LEGACY_APP.
+  if (is_app())
+    return LegacyAppBrowserSupportsWindowFeature(feature, check_can_support);
 
-    if (SupportsLocationBar())
-      features |= FEATURE_LOCATIONBAR;
+  switch (type_) {
+    case TYPE_TABBED:
+      return TabbedBrowserSupportsWindowFeature(feature, check_can_support);
+    case TYPE_POPUP:
+      return PopupBrowserSupportsWindowFeature(feature, check_can_support);
   }
-
-  // Web apps should always support the toolbar, so the title/origin of the
-  // current page can be shown when browsing a url that is not inside the app.
-  // Note: Final determination of whether or not the toolbar is shown is made by
-  // the |AppBrowserController|.
-  if (web_app::AppBrowserController::IsForWebAppBrowser(this))
-    features |= FEATURE_TOOLBAR;
-
-  // Some types of web apps will have a tabstrip.
-  if (app_controller_ && app_controller_->HasTabStrip())
-    features |= FEATURE_TABSTRIP;
-
-  return !!(features & feature);
 }
 
 void Browser::UpdateBookmarkBarState(BookmarkBarStateChangeReason reason) {

chrome/browser/ui/browser.h

@@ -134,13 +134,17 @@ class Browser : public TabStripModelObserver,
   // Possible elements of the Browser window.
   enum WindowFeature {
     FEATURE_NONE = 0,
-    FEATURE_TITLEBAR = 1,
-    FEATURE_TABSTRIP = 2,
-    FEATURE_TOOLBAR = 4,
-    FEATURE_LOCATIONBAR = 8,
-    FEATURE_BOOKMARKBAR = 16,
-    FEATURE_INFOBAR = 32,
-    FEATURE_DOWNLOADSHELF = 64,
+    FEATURE_TITLEBAR = 1 << 0,
+    FEATURE_TABSTRIP = 1 << 1,
+    FEATURE_TOOLBAR = 1 << 2,
+    FEATURE_LOCATIONBAR = 1 << 3,
+    FEATURE_BOOKMARKBAR = 1 << 4,
+    FEATURE_INFOBAR = 1 << 5,
+    FEATURE_DOWNLOADSHELF = 1 << 6,
+    // TODO(crbug.com/992834): Add FEATURE_PAGECONTROLS to describe the presence
+    // of per-page controls such as Content Settings Icons, which should be
+    // decoupled from FEATURE_LOCATIONBAR as they have independent presence in
+    // Web App browsers.
   };
 
   // The context for a download blocked notification from
@@ -929,9 +933,17 @@ class Browser : public TabStripModelObserver,
   // Shared code between Reload() and ReloadBypassingCache().
   void ReloadInternal(WindowOpenDisposition disposition, bool bypass_cache);
 
-  // Returns true if the Browser window supports a location bar. Having support
-  // for the location bar does not mean it will be visible.
-  bool SupportsLocationBar() const;
+  bool TabbedBrowserSupportsWindowFeature(WindowFeature feature,
+                                          bool check_can_support) const;
+
+  bool PopupBrowserSupportsWindowFeature(WindowFeature feature,
+                                         bool check_can_support) const;
+
+  bool LegacyAppBrowserSupportsWindowFeature(WindowFeature feature,
+                                             bool check_can_support) const;
+
+  bool WebAppBrowserSupportsWindowFeature(WindowFeature feature,
+                                          bool check_can_support) const;
 
   // Implementation of SupportsWindowFeature and CanSupportWindowFeature. If
   // |check_fullscreen| is true, the set of features reflect the actual state of