Show close buttons on inactive tabs by default, and add feature control for it.

Bug: 855729, 856667, 846430
Change-Id: Id2c9da22f671c94ffce1995477cab0899217ea2b
Reviewed-on: https://chromium-review.googlesource.com/1119538
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571607}

chrome/browser/about_flags.cc

@@ -1447,6 +1447,10 @@ const FeatureEntry kFeatureEntries[] = {
     {"top-chrome-md", flag_descriptions::kTopChromeMd,
      flag_descriptions::kTopChromeMdDescription, kOsDesktop,
      MULTI_VALUE_TYPE(kTopChromeMaterialDesignChoices)},
+    {"close-buttons-inactive-tabs",
+     flag_descriptions::kCloseButtonsInactiveTabs,
+     flag_descriptions::kCloseButtonsInactiveTabsDescription, kOsDesktop,
+     FEATURE_VALUE_TYPE(features::kCloseButtonsInactiveTabs)},
     {"site-settings", flag_descriptions::kSiteSettings,
      flag_descriptions::kSiteSettingsDescription, kOsDesktop,
      FEATURE_VALUE_TYPE(features::kSiteSettings)},

chrome/browser/flag_descriptions.cc

@@ -183,6 +183,10 @@ const char kClipboardContentSettingDescription[] =
     "Enables a site-wide permission in the UI which controls access to the "
     "asynchronous clipboard web API";
 
+const char kCloseButtonsInactiveTabs[] = "Close buttons on inactive tabs";
+const char kCloseButtonsInactiveTabsDescription[] =
+    "Shows close buttons on inactive tabs in non-touch mode.";
+
 const char kCloudImportName[] = "Cloud Import";
 const char kCloudImportDescription[] = "Allows the cloud-import feature.";
 

chrome/browser/flag_descriptions.h

@@ -137,6 +137,9 @@ extern const char kClickToOpenPDFDescription[];
 extern const char kClipboardContentSettingName[];
 extern const char kClipboardContentSettingDescription[];
 
+extern const char kCloseButtonsInactiveTabs[];
+extern const char kCloseButtonsInactiveTabsDescription[];
+
 extern const char kCloudImportName[];
 extern const char kCloudImportDescription[];
 

chrome/browser/ui/views/tabs/tab_close_button.cc

@@ -15,6 +15,7 @@
 #include "chrome/browser/ui/layout_constants.h"
 #include "chrome/browser/ui/views/tabs/tab.h"
 #include "chrome/browser/ui/views/tabs/tab_controller.h"
+#include "chrome/common/chrome_features.h"
 #include "components/strings/grit/components_strings.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/material_design/material_design_controller.h"
@@ -170,7 +171,8 @@ bool TabCloseButton::GetHitTestMask(gfx::Path* mask) const {
 
 SkAlpha TabCloseButton::GetOpacity() {
   Tab* tab = static_cast<Tab*>(parent());
-  if (!MD::IsRefreshUi() || IsMouseHovered() || tab->IsActive())
+  if (base::FeatureList::IsEnabled(features::kCloseButtonsInactiveTabs) ||
+      IsMouseHovered() || tab->IsActive())
     return SK_AlphaOPAQUE;
   const double animation_value = tab->hover_controller()->GetAnimationValue();
   return gfx::Tween::IntValueBetween(animation_value, SK_AlphaTRANSPARENT,

chrome/browser/ui/views/tabs/tab_strip.cc

@@ -35,6 +35,7 @@
 #include "chrome/browser/ui/views/tabs/tab_strip_layout.h"
 #include "chrome/browser/ui/views/tabs/tab_strip_observer.h"
 #include "chrome/browser/ui/views/touch_uma/touch_uma.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/grit/theme_resources.h"
 #include "third_party/skia/include/core/SkColorFilter.h"
@@ -836,9 +837,14 @@ NewTabButtonPosition TabStrip::GetNewTabButtonPosition() const {
 }
 
 bool TabStrip::ShouldHideCloseButtonForTab(Tab* tab) const {
-  if (tab->IsActive())
-    return SingleTabMode();
-  return touch_layout_ != nullptr || MD::IsRefreshUi();
+  if (tab->IsActive()) {
+    // For single-tab mode, the close button looks like it's floating oddly in
+    // space for LEADING/TRAILING NTBs, so hide in that case.
+    return SingleTabMode() &&
+           controller_->GetNewTabButtonPosition() != AFTER_TABS;
+  }
+  return touch_layout_ ||
+         !base::FeatureList::IsEnabled(features::kCloseButtonsInactiveTabs);
 }
 
 bool TabStrip::ShouldShowCloseButtonOnHover() {

chrome/common/chrome_features.cc

@@ -176,6 +176,10 @@ const base::Feature kContentFullscreen{"ContentFullscreen",
 const base::Feature kClipboardContentSetting{"ClipboardContentSetting",
                                              base::FEATURE_ENABLED_BY_DEFAULT};
 
+// Whether inactive tabs show their close buttons by default for non-touch mode.
+const base::Feature kCloseButtonsInactiveTabs{"CloseButtonsInactiveTabs",
+                                              base::FEATURE_ENABLED_BY_DEFAULT};
+
 #if defined(OS_CHROMEOS)
 // Enable project Crostini, Linux VMs on Chrome OS.
 const base::Feature kCrostini{"Crostini", base::FEATURE_DISABLED_BY_DEFAULT};

chrome/common/chrome_features.h

@@ -86,6 +86,8 @@ extern const base::Feature kClickToOpenPDFPlaceholder;
 
 extern const base::Feature kClipboardContentSetting;
 
+extern const base::Feature kCloseButtonsInactiveTabs;
+
 #if defined(OS_MACOSX)
 extern const base::Feature kContentFullscreen;
 #endif

tools/metrics/histograms/enums.xml

@@ -28212,6 +28212,7 @@ from previous Chrome versions.
   <int value="637452937" label="ChromeHomeSurvey:enabled"/>
   <int value="642037198" label="SoleIntegration:disabled"/>
   <int value="643725031" label="disable-touch-feedback"/>
+  <int value="644084236" label="CloseButtonsInactiveTabs:enabled"/>
   <int value="644189071" label="PermissionsBlacklist:enabled"/>
   <int value="646252875" label="ReadItLaterInMenu:enabled"/>
   <int value="646738320" label="disable-gesture-editing"/>
@@ -28673,6 +28674,7 @@ from previous Chrome versions.
   <int value="1612446645" label="enable-weak-memorycache"/>
   <int value="1612871297" label="WebPayments:disabled"/>
   <int value="1612974229" label="allow-insecure-localhost"/>
+  <int value="1614596813" label="CloseButtonsInactiveTabs:disabled"/>
   <int value="1615988672" label="GrantNotificationsToDSE:enabled"/>
   <int value="1617187093" label="enable-improved-a2hs"/>
   <int value="1621298798" label="VrBrowserKeyboard:enabled"/>