Revert "cros: Convert switch to feature for new overview ui."

This reverts commit e50412f.

Reason for revert: crbug.com/821999 enabled test now failing on chromeos

Original change's description:
> cros: Convert switch to feature for new overview ui.
> 
> Convert to feature which is enabled by default.
> 
> Test: manual
> Bug: 821608
> Change-Id: If513c4d2a6bdfd4197525d7922d99cfc01f4c616
> Reviewed-on: https://chromium-review.googlesource.com/961721
> Reviewed-by: Scott Violet <sky@chromium.org>
> Commit-Queue: Sammie Quon <sammiequon@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#543139}

TBR=sky@chromium.org,sammiequon@chromium.org

Change-Id: I92ce3f93cb88cef92e242fbbd6338cbdac528cdd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 821608
Reviewed-on: https://chromium-review.googlesource.com/962837
Reviewed-by: Caleb Rouleau <crouleau@chromium.org>
Commit-Queue: Caleb Rouleau <crouleau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543205}

ash/public/cpp/ash_features.cc

@@ -19,9 +19,6 @@ const base::Feature kKeyboardShortcutViewer{"KeyboardShortcutViewer",
 const base::Feature kNewOverviewAnimations{"NewOverviewAnimations",
                                            base::FEATURE_ENABLED_BY_DEFAULT};
 
-const base::Feature kNewOverviewUi{"NewOverviewUi",
-                                   base::FEATURE_ENABLED_BY_DEFAULT};
-
 const base::Feature kPersistentWindowBounds{"PersistentWindowBounds",
                                             base::FEATURE_ENABLED_BY_DEFAULT};
 
@@ -43,6 +40,10 @@ bool IsKeyboardShortcutViewerEnabled() {
   return base::FeatureList::IsEnabled(kKeyboardShortcutViewer);
 }
 
+bool IsNewOverviewAnimationsEnabled() {
+  return base::FeatureList::IsEnabled(kNewOverviewAnimations);
+}
+
 bool IsSystemTrayUnifiedEnabled() {
   return base::FeatureList::IsEnabled(kSystemTrayUnified);
 }

ash/public/cpp/ash_features.h

@@ -31,11 +31,6 @@ ASH_PUBLIC_EXPORT extern const base::Feature kKeyboardShortcutViewer;
 // https://crbug.com/801465.
 ASH_PUBLIC_EXPORT extern const base::Feature kNewOverviewAnimations;
 
-// Enables the new overview animations.
-// TODO(sammiequon): Remove this after the feature is fully launched.
-// https://crbug.com/821608.
-ASH_PUBLIC_EXPORT extern const base::Feature kNewOverviewUi;
-
 // Enables persistent window bounds in multi-displays scenario.
 // TODO(warx): Remove this after the feature is fully launched.
 // https://crbug.com/805046.
@@ -53,6 +48,8 @@ ASH_PUBLIC_EXPORT bool IsDockedMagnifierEnabled();
 
 ASH_PUBLIC_EXPORT bool IsKeyboardShortcutViewerEnabled();
 
+ASH_PUBLIC_EXPORT bool IsNewOverviewAnimationsEnabled();
+
 ASH_PUBLIC_EXPORT bool IsPersistentWindowBoundsEnabled();
 
 ASH_PUBLIC_EXPORT bool IsSystemTrayUnifiedEnabled();

ash/public/cpp/ash_switches.cc

@@ -48,6 +48,10 @@ const char kAshEnableCursorMotionBlur[] = "ash-enable-cursor-motion-blur";
 const char kAshEnableMagnifierKeyScroller[] =
     "ash-enable-magnifier-key-scroller";
 
+// Enables the new overview ui.
+// TODO(sammiequon): Remove this once the feature is launched. crbug.com/782330.
+const char kAshEnableNewOverviewUi[] = "ash-enable-new-overview-ui";
+
 // Enable the Night Light feature.
 const char kAshEnableNightLight[] = "ash-enable-night-light";
 

ash/public/cpp/ash_switches.h

@@ -27,6 +27,7 @@ ASH_PUBLIC_EXPORT extern const char kAshDisableTouchExplorationMode[];
 ASH_PUBLIC_EXPORT extern const char kAshEnableCursorMotionBlur[];
 ASH_PUBLIC_EXPORT extern const char kAshEnableV1AppBackButton[];
 ASH_PUBLIC_EXPORT extern const char kAshEnableMagnifierKeyScroller[];
+ASH_PUBLIC_EXPORT extern const char kAshEnableNewOverviewUi[];
 ASH_PUBLIC_EXPORT extern const char kAshEnableNightLight[];
 ASH_PUBLIC_EXPORT extern const char kAshEnablePaletteOnAllDisplays[];
 ASH_PUBLIC_EXPORT extern const char kAshEnableScaleSettingsTray[];

ash/system/overview/overview_button_tray_unittest.cc

@@ -166,22 +166,28 @@ TEST_F(OverviewButtonTrayTest, PerformDoubleTapAction) {
 TEST_F(OverviewButtonTrayTest, TrayOverviewUserAction) {
   ASSERT_FALSE(Shell::Get()->window_selector_controller()->IsSelecting());
 
+  // Tapping on the control when there are no windows (and thus the user cannot
+  // enter overview mode) should still record the action.
+  base::UserActionTester user_action_tester;
+  GetTray()->PerformAction(CreateTapEvent());
+  ASSERT_FALSE(Shell::Get()->window_selector_controller()->IsSelecting());
+  EXPECT_EQ(1, user_action_tester.GetActionCount(kTrayOverview));
+
   // With one window present, tapping on the control to enter overview mode
   // should record the user action.
-  base::UserActionTester user_action_tester;
   std::unique_ptr<aura::Window> window(
       CreateTestWindowInShellWithBounds(gfx::Rect(5, 5, 20, 20)));
   GetTray()->PerformAction(
       CreateTapEvent(OverviewButtonTray::kDoubleTapThresholdMs));
   ASSERT_TRUE(Shell::Get()->window_selector_controller()->IsSelecting());
-  EXPECT_EQ(1, user_action_tester.GetActionCount(kTrayOverview));
+  EXPECT_EQ(2, user_action_tester.GetActionCount(kTrayOverview));
 
   // Tapping on the control to exit overview mode should record the
   // user action.
   GetTray()->PerformAction(
       CreateTapEvent(OverviewButtonTray::kDoubleTapThresholdMs * 2));
   ASSERT_FALSE(Shell::Get()->window_selector_controller()->IsSelecting());
-  EXPECT_EQ(2, user_action_tester.GetActionCount(kTrayOverview));
+  EXPECT_EQ(3, user_action_tester.GetActionCount(kTrayOverview));
 }
 
 // Tests that a second OverviewButtonTray has been created, and only shows

ash/wm/overview/overview_utils.cc

@@ -10,6 +10,7 @@
 #include "ash/shell.h"
 #include "ash/wm/splitview/split_view_controller.h"
 #include "ash/wm/window_state.h"
+#include "base/command_line.h"
 #include "base/optional.h"
 #include "third_party/skia/include/pathops/SkPathOps.h"
 #include "ui/aura/client/aura_constants.h"
@@ -25,6 +26,11 @@ namespace ash {
 
 namespace {
 
+// Cache the result of the command line lookup so the command line is only
+// looked up once.
+base::Optional<bool> g_enable_new_overview_animations = base::nullopt;
+base::Optional<bool> g_enable_new_overview_ui = base::nullopt;
+
 // BackgroundWith1PxBorder renders a solid background color, with a one pixel
 // border with rounded corners. This accounts for the scaling of the canvas, so
 // that the border is 1 pixel thick regardless of display scaling.
@@ -99,11 +105,30 @@ bool CanCoverAvailableWorkspace(aura::Window* window) {
 }
 
 bool IsNewOverviewAnimationsEnabled() {
-  return base::FeatureList::IsEnabled(features::kNewOverviewAnimations);
+  if (g_enable_new_overview_animations == base::nullopt) {
+    g_enable_new_overview_animations =
+        base::make_optional(ash::features::IsNewOverviewAnimationsEnabled());
+  }
+
+  return g_enable_new_overview_animations.value();
 }
 
 bool IsNewOverviewUi() {
-  return base::FeatureList::IsEnabled(features::kNewOverviewUi);
+  if (g_enable_new_overview_ui == base::nullopt) {
+    g_enable_new_overview_ui =
+        base::make_optional(base::CommandLine::ForCurrentProcess()->HasSwitch(
+            ash::switches::kAshEnableNewOverviewUi));
+  }
+
+  return g_enable_new_overview_ui.value();
+}
+
+void ResetCachedOverviewAnimationsValueForTesting() {
+  g_enable_new_overview_animations = base::nullopt;
+}
+
+void ResetCachedOverviewUiValueForTesting() {
+  g_enable_new_overview_ui = base::nullopt;
 }
 
 std::unique_ptr<views::Widget> CreateBackgroundWidget(aura::Window* root_window,

ash/wm/overview/overview_utils.h

@@ -36,6 +36,11 @@ bool IsNewOverviewAnimationsEnabled();
 // obsolete. See https://crbug.com/782320.
 bool IsNewOverviewUi();
 
+// Resets the stored value so that the next IsNewOverviewAnimations and
+// IsNewOverviewUi call will query the command line arguments again.
+ASH_EXPORT void ResetCachedOverviewAnimationsValueForTesting();
+ASH_EXPORT void ResetCachedOverviewUiValueForTesting();
+
 // Creates and returns a background translucent widget parented in
 // |root_window|'s default container and having |background_color|.
 // When |border_thickness| is non-zero, a border is created having

ash/wm/overview/window_selector_unittest.cc

@@ -40,6 +40,7 @@
 #include "ash/wm/window_util.h"
 #include "ash/wm/wm_event.h"
 #include "ash/wm/workspace/workspace_window_resizer.h"
+#include "base/command_line.h"
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
 #include "base/stl_util.h"
@@ -120,6 +121,13 @@ class WindowSelectorTest : public AshTestBase {
     ScopedTransformOverviewWindow::SetImmediateCloseForTests();
   }
 
+  void TearDown() override {
+    ResetCachedOverviewAnimationsValueForTesting();
+    ResetCachedOverviewUiValueForTesting();
+
+    AshTestBase::TearDown();
+  }
+
   aura::Window* CreateWindow(const gfx::Rect& bounds) {
     aura::Window* window =
         CreateTestWindowInShellWithDelegate(&delegate_, -1, bounds);
@@ -378,15 +386,15 @@ TEST_F(WindowSelectorTest, OverviewScreenRotation) {
   // y: -kTextFilterHeight (since there's no text in the filter) - 2.
   // w: std::min(kTextFilterWidth, total_bounds.width()).
   // h: kTextFilterHeight.
-  gfx::Rect expected_bounds(60, -34, 280, 32);
+  gfx::Rect expected_bounds(60, -42, 280, 40);
   EXPECT_EQ(expected_bounds, text_filter->GetClientAreaBoundsInScreen());
 
   // Rotates the display, which triggers the WindowSelector's
   // RepositionTextFilterOnDisplayMetricsChange method.
   UpdateDisplay("400x300/r");
 
   // Uses the same formulas as above using width = 300, height = 400.
-  expected_bounds = gfx::Rect(10, -34, 280, 32);
+  expected_bounds = gfx::Rect(10, -42, 280, 40);
   EXPECT_EQ(expected_bounds, text_filter->GetClientAreaBoundsInScreen());
 }
 
@@ -1895,6 +1903,9 @@ TEST_F(WindowSelectorTest, TransformedRectIsCenteredWithInset) {
 // Verify that a window which will be displayed like a letter box on the window
 // grid has the correct bounds.
 TEST_F(WindowSelectorTest, TransformingLetteredRect) {
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kAshEnableNewOverviewUi);
+
   // Create a window whose width is more than twice the height.
   const gfx::Rect original_bounds(10, 10, 300, 100);
   const int scale = 3;
@@ -1935,6 +1946,9 @@ TEST_F(WindowSelectorTest, TransformingLetteredRect) {
 // Verify that a window which will be displayed like a pillar box on the window
 // grid has the correct bounds.
 TEST_F(WindowSelectorTest, TransformingPillaredRect) {
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kAshEnableNewOverviewUi);
+
   // Create a window whose height is more than twice the width.
   const gfx::Rect original_bounds(10, 10, 100, 300);
   const int scale = 3;
@@ -1986,11 +2000,20 @@ TEST_F(WindowSelectorTest, OverviewWhileDragging) {
   resizer->RevertDrag();
 }
 
+// Verify that entering overview mode without windows with the old ui is not
+// possible.
+TEST_F(WindowSelectorTest, OverviewNoWindowsIndicatorOldUi) {
+  ToggleOverview();
+  EXPECT_FALSE(window_selector());
+}
+
 // Verify that the overview no windows indicator appears when entering overview
 // mode with no windows.
 TEST_F(WindowSelectorTest, OverviewNoWindowsIndicator) {
   // Verify that by entering overview mode without windows, the no items
   // indicator appears.
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kAshEnableNewOverviewUi);
   ToggleOverview();
   ASSERT_TRUE(window_selector());
   EXPECT_EQ(0u, GetWindowItemsForRoot(0).size());
@@ -2001,6 +2024,9 @@ TEST_F(WindowSelectorTest, OverviewNoWindowsIndicator) {
 
 // Verify that the overview no windows indicator position is as expected.
 TEST_F(WindowSelectorTest, OverviewNoWindowsIndicatorPosition) {
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kAshEnableNewOverviewUi);
+
   UpdateDisplay("400x300");
   // Midpoint of height minus shelf.
   const int expected_y = (300 - kShelfSize) / 2;
@@ -2046,6 +2072,9 @@ TEST_F(WindowSelectorTest, OverviewNoWindowsIndicatorPosition) {
 // Verify that when opening overview mode with multiple displays, the no items
 // indicator appears on the grid(s) if it has no windows.
 TEST_F(WindowSelectorTest, OverviewNoWindowsIndicatorMultiDisplay) {
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kAshEnableNewOverviewUi);
+
   // Create two windows, one each on the first two monitors.
   UpdateDisplay("400x400,400x400,400x400");
   aura::Window::Windows root_windows = Shell::GetAllRootWindows();
@@ -2074,6 +2103,8 @@ TEST_F(WindowSelectorTest, OverviewNoWindowsIndicatorMultiDisplay) {
 // Verify that pressing and releasing keys does not show the overview textbox
 // when there are no windows opened.
 TEST_F(WindowSelectorTest, TextfilterHiddenWhenNoWindows) {
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kAshEnableNewOverviewUi);
   ToggleOverview();
   ASSERT_TRUE(window_selector());
 
@@ -2083,6 +2114,9 @@ TEST_F(WindowSelectorTest, TextfilterHiddenWhenNoWindows) {
 
 // Tests the cases when very wide or tall windows enter overview mode.
 TEST_F(WindowSelectorTest, ExtremeWindowBounds) {
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kAshEnableNewOverviewUi);
+
   // Add three windows which in overview mode will be considered wide, tall and
   // normal. Window |wide|, with size (400, 160) will be resized to (200, 160)
   // when the 400x200 is rotated to 200x400, and should be considered a normal
@@ -2519,6 +2553,9 @@ TEST_F(WindowSelectorTest, WindowItemCanAnimateOnDragRelease) {
 // Verify that the window selector items titlebar and close button change
 // visibility when a item is being dragged.
 TEST_F(WindowSelectorTest, WindowItemTitleCloseVisibilityOnDrag) {
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kAshEnableNewOverviewUi);
+
   UpdateDisplay("400x400");
   const gfx::Rect bounds(10, 10, 200, 200);
   std::unique_ptr<aura::Window> window1(CreateWindow(bounds));
@@ -2652,6 +2689,9 @@ TEST_F(WindowSelectorTest, OverviewWidgetStackingOrder) {
 // Verify that a windows which enter overview mode have a visible backdrop, if
 // the window is to be letter or pillar fitted.
 TEST_F(WindowSelectorTest, Backdrop) {
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kAshEnableNewOverviewUi);
+
   // Add three windows which in overview mode will be considered wide, tall and
   // normal. Window |wide|, with size (400, 160) will be resized to (200, 160)
   // when the 400x200 is rotated to 200x400, and should be considered a normal
@@ -2692,6 +2732,9 @@ TEST_F(WindowSelectorTest, Backdrop) {
 // Verify that the mask that is applied to add rounded corners in overview mode
 // is removed during animations and drags.
 TEST_F(WindowSelectorTest, RoundedEdgeMaskVisibility) {
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kAshEnableNewOverviewUi);
+
   UpdateDisplay("400x400");
   const gfx::Rect bounds(0, 0, 200, 200);
   std::unique_ptr<aura::Window> window1(CreateWindow(bounds));
@@ -3854,6 +3897,9 @@ TEST_F(SplitViewWindowSelectorTest,
 // the item to be dragged or enter splitview, but will still be able to select a
 // window.
 TEST_F(SplitViewWindowSelectorTest, EventsOnOverviewTitleBar) {
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kAshEnableNewOverviewUi);
+
   const gfx::Rect bounds(400, 400);
   std::unique_ptr<aura::Window> window1(CreateWindow(bounds));
 

ash/wm/splitview/split_view_controller_unittest.cc

@@ -194,15 +194,15 @@ TEST_F(SplitViewControllerTest, WindowCloseTest) {
   // Since left window was closed, its default snap position changed to RIGHT.
   EXPECT_EQ(split_view_controller()->default_snap_position(),
             SplitViewController::RIGHT);
-  // Window grid is showing no recent items, and has no windows, but it is still
-  // available.
-  EXPECT_TRUE(Shell::Get()->window_selector_controller()->IsSelecting());
+  // It can't open overview window grid since there is no window to be shown
+  // in the overview window grid.
+  EXPECT_FALSE(Shell::Get()->window_selector_controller()->IsSelecting());
 
   // Now close the other snapped window.
   window2.reset();
   EXPECT_EQ(split_view_controller()->IsSplitViewModeActive(), false);
   EXPECT_EQ(split_view_controller()->state(), SplitViewController::NO_SNAP);
-  EXPECT_TRUE(Shell::Get()->window_selector_controller()->IsSelecting());
+  EXPECT_FALSE(Shell::Get()->window_selector_controller()->IsSelecting());
 
   // 3 - Then test the scenario with more than two windows.
   std::unique_ptr<aura::Window> window3(CreateWindow(bounds));

chrome/browser/about_flags.cc

@@ -3666,7 +3666,7 @@ const FeatureEntry kFeatureEntries[] = {
     {"ash-enable-new-overview-ui",
      flag_descriptions::kAshEnableNewOverviewUiName,
      flag_descriptions::kAshEnableNewOverviewUiDescription, kOsCrOS,
-     FEATURE_VALUE_TYPE(ash::features::kNewOverviewUi)},
+     SINGLE_VALUE_TYPE(ash::switches::kAshEnableNewOverviewUi)},
 #endif  // defined(OS_CHROMEOS)
 
     {"unified-consent", flag_descriptions::kUnifiedConsentName,

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

@@ -15,7 +15,6 @@
 #include "ash/frame/frame_header_origin_text.h"
 #include "ash/frame/frame_header_util.h"
 #include "ash/public/cpp/app_types.h"
-#include "ash/public/cpp/ash_features.h"
 #include "ash/public/cpp/ash_switches.h"
 #include "ash/public/cpp/window_properties.h"
 #include "ash/public/interfaces/constants.mojom.h"
@@ -24,7 +23,6 @@
 #include "ash/wm/overview/window_selector_controller.h"
 #include "ash/wm/window_util.h"
 #include "base/command_line.h"
-#include "base/feature_list.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task_runner.h"
 #include "base/threading/sequenced_task_runner_handle.h"
@@ -444,8 +442,10 @@ void BrowserNonClientFrameViewAsh::OnOverviewModeStarting() {
 
   // Update the window icon so that overview mode can grab the icon from
   // aura::client::kWindowIcon to display.
-  if (base::FeatureList::IsEnabled(ash::features::kNewOverviewUi))
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          ash::switches::kAshEnableNewOverviewUi)) {
     frame()->UpdateWindowIcon();
+  }
 
   frame()->GetNativeWindow()->SetProperty(aura::client::kTopViewColor,
                                           GetFrameColor());

tools/metrics/histograms/enums.xml

@@ -25800,7 +25800,6 @@ from previous Chrome versions.
   <int value="-1751928267" label="disable-icon-ntp"/>
   <int value="-1749176684" label="PauseBackgroundTabs:disabled"/>
   <int value="-1746767834" label="ssl-interstitial-v2-gray"/>
-  <int value="-1746255521" label="NewOverviewUi:enabled"/>
   <int value="-1740519217" label="disable-software-rasterizer"/>
   <int value="-1738416948" label="OptimizationHints:enabled"/>
   <int value="-1736075054" label="EnableFullscreenAppList:enabled"/>
@@ -27295,7 +27294,6 @@ from previous Chrome versions.
   <int value="1932732886" label="OpenVR:enabled"/>
   <int value="1936810062" label="WebVrVsyncAlign:enabled"/>
   <int value="1939413645" label="enable-invalid-cert-collection"/>
-  <int value="1940625534" label="NewOverviewUi:disabled"/>
   <int value="1942911276" label="enable-grouped-history"/>
   <int value="1944156526" label="sync-url"/>
   <int value="1947350992" label="drop-sync-credential:disabled"/>