Remove MaterialDesignController::GetMode() and old MD mode enum values.

De-parameterizes one test that doesn't care about the MD mode.

Bug: none
Change-Id: I8a89eda9e66a9fc59b2c61668d1fdebc4ee7879b
Reviewed-on: https://chromium-review.googlesource.com/c/1297647Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Allen Bauer <kylixrd@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602535}

ash/public/cpp/ash_layout_constants.cc

@@ -18,9 +18,8 @@ gfx::Size GetAshLayoutSize(AshLayoutSize size) {
   // |kBrowserMaximizedCaptionButtonHeight| should be kept in sync with those
   // for TAB_HEIGHT in // chrome/browser/ui/layout_constants.cc.
   // TODO: Ideally these values should be obtained from a common location.
-  constexpr int kBrowserMaximizedCaptionButtonHeight[] = {29, 33, 41, 34, 41};
-  const int mode = ui::MaterialDesignController::GetMode();
-  int height = kBrowserMaximizedCaptionButtonHeight[mode];
+  const bool touch = ui::MaterialDesignController::IsTouchOptimizedUiEnabled();
+  int height = touch ? 41 : 34;
   if (size == AshLayoutSize::kBrowserCaptionRestored)
     height += 8;  // Restored window titlebars are 8 DIP taller than maximized.
   return gfx::Size(kButtonWidth, height);

chrome/browser/extensions/extension_action_icon_factory_unittest.cc

@@ -25,9 +25,7 @@
 #include "extensions/common/image_util.h"
 #include "skia/ext/image_operations.h"
 #include "testing/gtest/include/gtest/gtest.h"
-#include "ui/base/material_design/material_design_controller.h"
 #include "ui/base/resource/resource_bundle.h"
-#include "ui/base/test/material_design_controller_test_api.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/skia_util.h"
@@ -82,7 +80,7 @@ gfx::Image LoadIcon(const std::string& filename) {
 }
 
 class ExtensionActionIconFactoryTest
-    : public testing::TestWithParam<ui::MaterialDesignController::Mode>,
+    : public testing::Test,
       public ExtensionActionIconFactory::Observer {
  public:
   ExtensionActionIconFactoryTest() : quit_in_icon_updated_(false) {}
@@ -134,12 +132,9 @@ class ExtensionActionIconFactoryTest
     extension_service_ = static_cast<extensions::TestExtensionSystem*>(
         extensions::ExtensionSystem::Get(profile_.get()))->
         CreateExtensionService(&command_line, base::FilePath(), false);
-    material_design_state_.reset(
-        new ui::test::MaterialDesignControllerTestAPI(GetParam()));
   }
 
   void TearDown() override {
-    material_design_state_.reset();
     profile_.reset();  // Get all DeleteSoon calls sent to ui_loop_.
     base::RunLoop().RunUntilIdle();
   }
@@ -166,8 +161,6 @@ class ExtensionActionIconFactoryTest
   bool quit_in_icon_updated_;
   std::unique_ptr<TestingProfile> profile_;
   ExtensionService* extension_service_;
-  std::unique_ptr<ui::test::MaterialDesignControllerTestAPI>
-      material_design_state_;
 
 #if defined OS_CHROMEOS
   chromeos::ScopedCrosSettingsTestHelper cros_settings_test_helper_;
@@ -177,15 +170,9 @@ class ExtensionActionIconFactoryTest
   DISALLOW_COPY_AND_ASSIGN(ExtensionActionIconFactoryTest);
 };
 
-INSTANTIATE_TEST_CASE_P(
-    ExtensionActionIconFactoryTest_MaterialDesign,
-    ExtensionActionIconFactoryTest,
-    testing::Values(ui::MaterialDesignController::MATERIAL_NORMAL,
-                    ui::MaterialDesignController::MATERIAL_HYBRID));
-
 // If there is no default icon, and the icon has not been set using |SetIcon|,
 // the factory should return the placeholder icon.
-TEST_P(ExtensionActionIconFactoryTest, NoIcons) {
+TEST_F(ExtensionActionIconFactoryTest, NoIcons) {
   // Load an extension that has browser action without default icon set in the
   // manifest and does not call |SetIcon| by default.
   scoped_refptr<Extension> extension(
@@ -209,7 +196,7 @@ TEST_P(ExtensionActionIconFactoryTest, NoIcons) {
 
 // If the explicitly-set icon is invisible, |ExtensionAction::GetIcon| should
 // return the placeholder icon.
-TEST_P(ExtensionActionIconFactoryTest, InvisibleIcon) {
+TEST_F(ExtensionActionIconFactoryTest, InvisibleIcon) {
   // Load an extension that has browser action with a default icon set in the
   // manifest, but that icon is not sufficiently visible.
   scoped_refptr<Extension> extension(
@@ -246,7 +233,7 @@ TEST_P(ExtensionActionIconFactoryTest, InvisibleIcon) {
 
 // If the icon has been set using |SetIcon|, the factory should return that
 // icon.
-TEST_P(ExtensionActionIconFactoryTest, AfterSetIcon) {
+TEST_F(ExtensionActionIconFactoryTest, AfterSetIcon) {
   // Load an extension that has browser action without default icon set in the
   // manifest and does not call |SetIcon| by default (but has an browser action
   // icon resource).
@@ -285,7 +272,7 @@ TEST_P(ExtensionActionIconFactoryTest, AfterSetIcon) {
 
 // If there is a default icon, and the icon has not been set using |SetIcon|,
 // the factory should return the default icon.
-TEST_P(ExtensionActionIconFactoryTest, DefaultIcon) {
+TEST_F(ExtensionActionIconFactoryTest, DefaultIcon) {
   // Load an extension that has browser action without default icon set in the
   // manifest and does not call |SetIcon| by default (but has an browser action
   // icon resource).

chrome/browser/flag_descriptions.cc

@@ -1855,12 +1855,10 @@ const char kTintGlCompositedContentDescription[] =
 
 const char kTopChromeMd[] = "UI Layout for the browser's top chrome";
 const char kTopChromeMdDescription[] =
-    "Toggles between 1) Material Design refresh, 2) Touchable Material Design "
-    "refresh and 3) Switching automatically between Touchable and "
-    "non-Touchable modes.";
-const char kTopChromeMdMaterialRefresh[] = "Refresh";
-const char kTopChromeMdMaterialRefreshTouchOptimized[] = "Touchable Refresh";
-const char kTopChromeMdMaterialRefreshDynamic[] = "Dynamic Refresh";
+    "Toggles between normal, touch-optimized, and dynamically switching UI.";
+const char kTopChromeMdMaterialRefresh[] = "Normal";
+const char kTopChromeMdMaterialRefreshTouchOptimized[] = "Touch-optimized";
+const char kTopChromeMdMaterialRefreshDynamic[] = "Dynamically switching";
 
 const char kThreadedScrollingName[] = "Threaded scrolling";
 const char kThreadedScrollingDescription[] =

chrome/browser/flag_descriptions.h

@@ -1127,10 +1127,6 @@ extern const char kTintGlCompositedContentDescription[];
 
 extern const char kTopChromeMd[];
 extern const char kTopChromeMdDescription[];
-extern const char kTopChromeMdMaterial[];
-extern const char kTopChromeMdMaterialAuto[];
-extern const char kTopChromeMdMaterialHybrid[];
-extern const char kTopChromeMdMaterialTouchOptimized[];
 extern const char kTopChromeMdMaterialRefresh[];
 extern const char kTopChromeMdMaterialRefreshTouchOptimized[];
 extern const char kTopChromeMdMaterialRefreshDynamic[];

chrome/browser/ui/extensions/extension_action_view_controller_unittest.cc

@@ -410,10 +410,7 @@ TEST_P(ExtensionActionViewControllerGrayscaleTest,
 INSTANTIATE_TEST_CASE_P(
     ,
     ExtensionActionViewControllerGrayscaleTest,
-    testing::Values(ui::MaterialDesignController::MATERIAL_NORMAL,
-                    ui::MaterialDesignController::MATERIAL_HYBRID,
-                    ui::MaterialDesignController::MATERIAL_TOUCH_OPTIMIZED,
-                    ui::MaterialDesignController::MATERIAL_REFRESH,
+    testing::Values(ui::MaterialDesignController::MATERIAL_REFRESH,
                     ui::MaterialDesignController::MATERIAL_TOUCH_REFRESH));
 
 TEST_P(ToolbarActionsBarUnitTest, RuntimeHostsTooltip) {

ui/base/material_design/material_design_controller.cc

@@ -33,40 +33,30 @@ namespace ui {
 bool MaterialDesignController::is_mode_initialized_ = false;
 
 MaterialDesignController::Mode MaterialDesignController::mode_ =
-    MaterialDesignController::MATERIAL_NORMAL;
+    MaterialDesignController::MATERIAL_REFRESH;
 
 bool MaterialDesignController::is_refresh_dynamic_ui_ = false;
 
 // static
 void MaterialDesignController::Initialize() {
   TRACE_EVENT0("startup", "MaterialDesignController::InitializeMode");
-  CHECK(!is_mode_initialized_);
+  DCHECK(!is_mode_initialized_);
 
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
   const std::string switch_value =
       command_line->GetSwitchValueASCII(switches::kTopChromeMD);
+  const bool touch =
+      switch_value == switches::kTopChromeMDMaterialRefreshTouchOptimized;
+  SetMode(touch ? MATERIAL_TOUCH_REFRESH : MATERIAL_REFRESH);
 
-  if (switch_value == switches::kTopChromeMDMaterialRefresh) {
-    SetMode(MATERIAL_REFRESH);
-  } else if (switch_value ==
-             switches::kTopChromeMDMaterialRefreshTouchOptimized) {
-    SetMode(MATERIAL_TOUCH_REFRESH);
-  } else if (switch_value == switches::kTopChromeMDMaterialRefreshDynamic) {
-    is_refresh_dynamic_ui_ = true;
-
-    // TabletModeClient's default state is in non-tablet mode.
-    SetMode(MATERIAL_REFRESH);
-  } else {
-    if (!switch_value.empty()) {
-      LOG(ERROR) << "Invalid value='" << switch_value
-                 << "' for command line switch '" << switches::kTopChromeMD
-                 << "'.";
-    }
+  // ChromeOS switches modes dynamically unless the mode is explicitly forced.
+  // Other platforms only switch dynamically if explicitly requested.
+  is_refresh_dynamic_ui_ =
 #if defined(OS_CHROMEOS)
-    is_refresh_dynamic_ui_ = true;
+      !touch && (switch_value != switches::kTopChromeMDMaterialRefresh);
+#else
+      switch_value == switches::kTopChromeMDMaterialRefreshDynamic;
 #endif
-    SetMode(MATERIAL_REFRESH);
-  }
 
   // Ideally, there would be a more general, "initialize random stuff here"
   // function into which these things and a call to this function can be placed.
@@ -81,16 +71,10 @@ void MaterialDesignController::Initialize() {
   }
 }
 
-// static
-MaterialDesignController::Mode MaterialDesignController::GetMode() {
-  CHECK(is_mode_initialized_);
-  return mode_;
-}
-
 // static
 bool MaterialDesignController::IsTouchOptimizedUiEnabled() {
-  return GetMode() == MATERIAL_TOUCH_OPTIMIZED ||
-         GetMode() == MATERIAL_TOUCH_REFRESH;
+  DCHECK(is_mode_initialized_);
+  return mode_ == MATERIAL_TOUCH_REFRESH;
 }
 
 // static

ui/base/material_design/material_design_controller.h

@@ -25,27 +25,17 @@ class MaterialDesignControllerTestAPI;
 // Central controller to handle material design modes.
 class UI_BASE_EXPORT MaterialDesignController {
  public:
-  // The different material design modes. The order cannot be changed without
-  // updating references as these are used as array indices.
+  // The different material design modes.
   enum Mode {
-    // Basic material design.
-    MATERIAL_NORMAL = 0,
-    // Material design targeted at mouse/touch hybrid devices.
-    MATERIAL_HYBRID = 1,
-    // Material design that is more optimized for touch devices.
-    MATERIAL_TOUCH_OPTIMIZED = 2,
     // Material Refresh design targeted at mouse devices.
-    MATERIAL_REFRESH = 3,
+    MATERIAL_REFRESH,
     // Material Refresh design optimized for touch devices.
-    MATERIAL_TOUCH_REFRESH = 4,
+    MATERIAL_TOUCH_REFRESH,
   };
 
   // Initializes |mode_|. Must be called before checking |mode_|.
   static void Initialize();
 
-  // Get the current Mode that should be used by the system.
-  static Mode GetMode();
-
   // Returns true if the touch-optimized UI material design mode is enabled.
   static bool IsTouchOptimizedUiEnabled();
 
@@ -65,7 +55,6 @@ class UI_BASE_EXPORT MaterialDesignController {
   friend class test::MaterialDesignControllerTestAPI;
 
   MaterialDesignController();
-
   ~MaterialDesignController() = delete;
 
   // Resets the initialization state to uninitialized. To be used by tests to

ui/base/material_design/material_design_controller_unittest.cc

@@ -139,9 +139,9 @@ TEST(MaterialDesignControllerObserver, InitializationOnMdModeChanged) {
 TEST(MaterialDesignControllerObserver, TabletOnMdModeChanged) {
   // Verifies that the MaterialDesignControllerObserver gets called back when
   // the tablet mode toggles.
+  MaterialDesignController::Initialize();
   test::MaterialDesignControllerTestAPI::SetDynamicRefreshUi(true);
 
-  MaterialDesignController::Initialize();
   TestObserver tablet_enabled_observer;
   MaterialDesignController::GetInstance()->AddObserver(
       &tablet_enabled_observer);
@@ -166,13 +166,13 @@ TEST(MaterialDesignControllerObserver, TabletOnMdModeChanged) {
 
   EXPECT_TRUE(tablet_disabled_observer.on_md_mode_changed_called());
 
-  test::MaterialDesignControllerTestAPI::Uninitialize();
   MaterialDesignController::GetInstance()->RemoveObserver(
       &tablet_disabled_observer);
   MaterialDesignController::GetInstance()->RemoveObserver(
       &tablet_enabled_observer);
 
   test::MaterialDesignControllerTestAPI::SetDynamicRefreshUi(false);
+  test::MaterialDesignControllerTestAPI::Uninitialize();
 }
 
 }  // namespace ui