Commit e5ad4adf authored by Findit's avatar Findit

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

This reverts commit 8096ec8c.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 602535 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzgwOTZlYzhjYjkyZDI1MjRhYzM0ZTRhNTQ2ZGQ0YWEzZWRmZmNmNjgM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.linux/Cast%20Audio%20Linux/23855

Sample Failed Step: ui_base_unittests

Original change's description:
> 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/1297647
> Reviewed-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}

Change-Id: Ie2378fa9f3d81841fc418a7ec77c2bddb76fde9b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: none
Reviewed-on: https://chromium-review.googlesource.com/c/1298641
Cr-Commit-Position: refs/heads/master@{#602552}
parent bfe1eaac
...@@ -18,8 +18,9 @@ gfx::Size GetAshLayoutSize(AshLayoutSize size) { ...@@ -18,8 +18,9 @@ gfx::Size GetAshLayoutSize(AshLayoutSize size) {
// |kBrowserMaximizedCaptionButtonHeight| should be kept in sync with those // |kBrowserMaximizedCaptionButtonHeight| should be kept in sync with those
// for TAB_HEIGHT in // chrome/browser/ui/layout_constants.cc. // for TAB_HEIGHT in // chrome/browser/ui/layout_constants.cc.
// TODO: Ideally these values should be obtained from a common location. // TODO: Ideally these values should be obtained from a common location.
const bool touch = ui::MaterialDesignController::IsTouchOptimizedUiEnabled(); constexpr int kBrowserMaximizedCaptionButtonHeight[] = {29, 33, 41, 34, 41};
int height = touch ? 41 : 34; const int mode = ui::MaterialDesignController::GetMode();
int height = kBrowserMaximizedCaptionButtonHeight[mode];
if (size == AshLayoutSize::kBrowserCaptionRestored) if (size == AshLayoutSize::kBrowserCaptionRestored)
height += 8; // Restored window titlebars are 8 DIP taller than maximized. height += 8; // Restored window titlebars are 8 DIP taller than maximized.
return gfx::Size(kButtonWidth, height); return gfx::Size(kButtonWidth, height);
......
...@@ -25,7 +25,9 @@ ...@@ -25,7 +25,9 @@
#include "extensions/common/image_util.h" #include "extensions/common/image_util.h"
#include "skia/ext/image_operations.h" #include "skia/ext/image_operations.h"
#include "testing/gtest/include/gtest/gtest.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/resource/resource_bundle.h"
#include "ui/base/test/material_design_controller_test_api.h"
#include "ui/gfx/codec/png_codec.h" #include "ui/gfx/codec/png_codec.h"
#include "ui/gfx/image/image_skia.h" #include "ui/gfx/image/image_skia.h"
#include "ui/gfx/skia_util.h" #include "ui/gfx/skia_util.h"
...@@ -80,7 +82,7 @@ gfx::Image LoadIcon(const std::string& filename) { ...@@ -80,7 +82,7 @@ gfx::Image LoadIcon(const std::string& filename) {
} }
class ExtensionActionIconFactoryTest class ExtensionActionIconFactoryTest
: public testing::Test, : public testing::TestWithParam<ui::MaterialDesignController::Mode>,
public ExtensionActionIconFactory::Observer { public ExtensionActionIconFactory::Observer {
public: public:
ExtensionActionIconFactoryTest() : quit_in_icon_updated_(false) {} ExtensionActionIconFactoryTest() : quit_in_icon_updated_(false) {}
...@@ -132,9 +134,12 @@ class ExtensionActionIconFactoryTest ...@@ -132,9 +134,12 @@ class ExtensionActionIconFactoryTest
extension_service_ = static_cast<extensions::TestExtensionSystem*>( extension_service_ = static_cast<extensions::TestExtensionSystem*>(
extensions::ExtensionSystem::Get(profile_.get()))-> extensions::ExtensionSystem::Get(profile_.get()))->
CreateExtensionService(&command_line, base::FilePath(), false); CreateExtensionService(&command_line, base::FilePath(), false);
material_design_state_.reset(
new ui::test::MaterialDesignControllerTestAPI(GetParam()));
} }
void TearDown() override { void TearDown() override {
material_design_state_.reset();
profile_.reset(); // Get all DeleteSoon calls sent to ui_loop_. profile_.reset(); // Get all DeleteSoon calls sent to ui_loop_.
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
} }
...@@ -161,6 +166,8 @@ class ExtensionActionIconFactoryTest ...@@ -161,6 +166,8 @@ class ExtensionActionIconFactoryTest
bool quit_in_icon_updated_; bool quit_in_icon_updated_;
std::unique_ptr<TestingProfile> profile_; std::unique_ptr<TestingProfile> profile_;
ExtensionService* extension_service_; ExtensionService* extension_service_;
std::unique_ptr<ui::test::MaterialDesignControllerTestAPI>
material_design_state_;
#if defined OS_CHROMEOS #if defined OS_CHROMEOS
chromeos::ScopedCrosSettingsTestHelper cros_settings_test_helper_; chromeos::ScopedCrosSettingsTestHelper cros_settings_test_helper_;
...@@ -170,9 +177,15 @@ class ExtensionActionIconFactoryTest ...@@ -170,9 +177,15 @@ class ExtensionActionIconFactoryTest
DISALLOW_COPY_AND_ASSIGN(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|, // If there is no default icon, and the icon has not been set using |SetIcon|,
// the factory should return the placeholder icon. // the factory should return the placeholder icon.
TEST_F(ExtensionActionIconFactoryTest, NoIcons) { TEST_P(ExtensionActionIconFactoryTest, NoIcons) {
// Load an extension that has browser action without default icon set in the // Load an extension that has browser action without default icon set in the
// manifest and does not call |SetIcon| by default. // manifest and does not call |SetIcon| by default.
scoped_refptr<Extension> extension( scoped_refptr<Extension> extension(
...@@ -196,7 +209,7 @@ TEST_F(ExtensionActionIconFactoryTest, NoIcons) { ...@@ -196,7 +209,7 @@ TEST_F(ExtensionActionIconFactoryTest, NoIcons) {
// If the explicitly-set icon is invisible, |ExtensionAction::GetIcon| should // If the explicitly-set icon is invisible, |ExtensionAction::GetIcon| should
// return the placeholder icon. // return the placeholder icon.
TEST_F(ExtensionActionIconFactoryTest, InvisibleIcon) { TEST_P(ExtensionActionIconFactoryTest, InvisibleIcon) {
// Load an extension that has browser action with a default icon set in the // Load an extension that has browser action with a default icon set in the
// manifest, but that icon is not sufficiently visible. // manifest, but that icon is not sufficiently visible.
scoped_refptr<Extension> extension( scoped_refptr<Extension> extension(
...@@ -233,7 +246,7 @@ TEST_F(ExtensionActionIconFactoryTest, InvisibleIcon) { ...@@ -233,7 +246,7 @@ TEST_F(ExtensionActionIconFactoryTest, InvisibleIcon) {
// If the icon has been set using |SetIcon|, the factory should return that // If the icon has been set using |SetIcon|, the factory should return that
// icon. // icon.
TEST_F(ExtensionActionIconFactoryTest, AfterSetIcon) { TEST_P(ExtensionActionIconFactoryTest, AfterSetIcon) {
// Load an extension that has browser action without default icon set in the // 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 // manifest and does not call |SetIcon| by default (but has an browser action
// icon resource). // icon resource).
...@@ -272,7 +285,7 @@ TEST_F(ExtensionActionIconFactoryTest, AfterSetIcon) { ...@@ -272,7 +285,7 @@ TEST_F(ExtensionActionIconFactoryTest, AfterSetIcon) {
// If there is a default icon, and the icon has not been set using |SetIcon|, // If there is a default icon, and the icon has not been set using |SetIcon|,
// the factory should return the default icon. // the factory should return the default icon.
TEST_F(ExtensionActionIconFactoryTest, DefaultIcon) { TEST_P(ExtensionActionIconFactoryTest, DefaultIcon) {
// Load an extension that has browser action without default icon set in the // 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 // manifest and does not call |SetIcon| by default (but has an browser action
// icon resource). // icon resource).
......
...@@ -1855,10 +1855,12 @@ const char kTintGlCompositedContentDescription[] = ...@@ -1855,10 +1855,12 @@ const char kTintGlCompositedContentDescription[] =
const char kTopChromeMd[] = "UI Layout for the browser's top chrome"; const char kTopChromeMd[] = "UI Layout for the browser's top chrome";
const char kTopChromeMdDescription[] = const char kTopChromeMdDescription[] =
"Toggles between normal, touch-optimized, and dynamically switching UI."; "Toggles between 1) Material Design refresh, 2) Touchable Material Design "
const char kTopChromeMdMaterialRefresh[] = "Normal"; "refresh and 3) Switching automatically between Touchable and "
const char kTopChromeMdMaterialRefreshTouchOptimized[] = "Touch-optimized"; "non-Touchable modes.";
const char kTopChromeMdMaterialRefreshDynamic[] = "Dynamically switching"; const char kTopChromeMdMaterialRefresh[] = "Refresh";
const char kTopChromeMdMaterialRefreshTouchOptimized[] = "Touchable Refresh";
const char kTopChromeMdMaterialRefreshDynamic[] = "Dynamic Refresh";
const char kThreadedScrollingName[] = "Threaded scrolling"; const char kThreadedScrollingName[] = "Threaded scrolling";
const char kThreadedScrollingDescription[] = const char kThreadedScrollingDescription[] =
......
...@@ -1127,6 +1127,10 @@ extern const char kTintGlCompositedContentDescription[]; ...@@ -1127,6 +1127,10 @@ extern const char kTintGlCompositedContentDescription[];
extern const char kTopChromeMd[]; extern const char kTopChromeMd[];
extern const char kTopChromeMdDescription[]; 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 kTopChromeMdMaterialRefresh[];
extern const char kTopChromeMdMaterialRefreshTouchOptimized[]; extern const char kTopChromeMdMaterialRefreshTouchOptimized[];
extern const char kTopChromeMdMaterialRefreshDynamic[]; extern const char kTopChromeMdMaterialRefreshDynamic[];
......
...@@ -410,7 +410,10 @@ TEST_P(ExtensionActionViewControllerGrayscaleTest, ...@@ -410,7 +410,10 @@ TEST_P(ExtensionActionViewControllerGrayscaleTest,
INSTANTIATE_TEST_CASE_P( INSTANTIATE_TEST_CASE_P(
, ,
ExtensionActionViewControllerGrayscaleTest, ExtensionActionViewControllerGrayscaleTest,
testing::Values(ui::MaterialDesignController::MATERIAL_REFRESH, testing::Values(ui::MaterialDesignController::MATERIAL_NORMAL,
ui::MaterialDesignController::MATERIAL_HYBRID,
ui::MaterialDesignController::MATERIAL_TOUCH_OPTIMIZED,
ui::MaterialDesignController::MATERIAL_REFRESH,
ui::MaterialDesignController::MATERIAL_TOUCH_REFRESH)); ui::MaterialDesignController::MATERIAL_TOUCH_REFRESH));
TEST_P(ToolbarActionsBarUnitTest, RuntimeHostsTooltip) { TEST_P(ToolbarActionsBarUnitTest, RuntimeHostsTooltip) {
......
...@@ -33,30 +33,40 @@ namespace ui { ...@@ -33,30 +33,40 @@ namespace ui {
bool MaterialDesignController::is_mode_initialized_ = false; bool MaterialDesignController::is_mode_initialized_ = false;
MaterialDesignController::Mode MaterialDesignController::mode_ = MaterialDesignController::Mode MaterialDesignController::mode_ =
MaterialDesignController::MATERIAL_REFRESH; MaterialDesignController::MATERIAL_NORMAL;
bool MaterialDesignController::is_refresh_dynamic_ui_ = false; bool MaterialDesignController::is_refresh_dynamic_ui_ = false;
// static // static
void MaterialDesignController::Initialize() { void MaterialDesignController::Initialize() {
TRACE_EVENT0("startup", "MaterialDesignController::InitializeMode"); TRACE_EVENT0("startup", "MaterialDesignController::InitializeMode");
DCHECK(!is_mode_initialized_); CHECK(!is_mode_initialized_);
base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
const std::string switch_value = const std::string switch_value =
command_line->GetSwitchValueASCII(switches::kTopChromeMD); command_line->GetSwitchValueASCII(switches::kTopChromeMD);
const bool touch =
switch_value == switches::kTopChromeMDMaterialRefreshTouchOptimized;
SetMode(touch ? MATERIAL_TOUCH_REFRESH : MATERIAL_REFRESH);
// ChromeOS switches modes dynamically unless the mode is explicitly forced. if (switch_value == switches::kTopChromeMDMaterialRefresh) {
// Other platforms only switch dynamically if explicitly requested. SetMode(MATERIAL_REFRESH);
is_refresh_dynamic_ui_ = } 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
<< "'.";
}
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
!touch && (switch_value != switches::kTopChromeMDMaterialRefresh); is_refresh_dynamic_ui_ = true;
#else
switch_value == switches::kTopChromeMDMaterialRefreshDynamic;
#endif #endif
SetMode(MATERIAL_REFRESH);
}
// Ideally, there would be a more general, "initialize random stuff here" // 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. // function into which these things and a call to this function can be placed.
...@@ -71,10 +81,16 @@ void MaterialDesignController::Initialize() { ...@@ -71,10 +81,16 @@ void MaterialDesignController::Initialize() {
} }
} }
// static
MaterialDesignController::Mode MaterialDesignController::GetMode() {
CHECK(is_mode_initialized_);
return mode_;
}
// static // static
bool MaterialDesignController::IsTouchOptimizedUiEnabled() { bool MaterialDesignController::IsTouchOptimizedUiEnabled() {
DCHECK(is_mode_initialized_); return GetMode() == MATERIAL_TOUCH_OPTIMIZED ||
return mode_ == MATERIAL_TOUCH_REFRESH; GetMode() == MATERIAL_TOUCH_REFRESH;
} }
// static // static
......
...@@ -25,17 +25,27 @@ class MaterialDesignControllerTestAPI; ...@@ -25,17 +25,27 @@ class MaterialDesignControllerTestAPI;
// Central controller to handle material design modes. // Central controller to handle material design modes.
class UI_BASE_EXPORT MaterialDesignController { class UI_BASE_EXPORT MaterialDesignController {
public: public:
// The different material design modes. // The different material design modes. The order cannot be changed without
// updating references as these are used as array indices.
enum Mode { 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 design targeted at mouse devices.
MATERIAL_REFRESH, MATERIAL_REFRESH = 3,
// Material Refresh design optimized for touch devices. // Material Refresh design optimized for touch devices.
MATERIAL_TOUCH_REFRESH, MATERIAL_TOUCH_REFRESH = 4,
}; };
// Initializes |mode_|. Must be called before checking |mode_|. // Initializes |mode_|. Must be called before checking |mode_|.
static void Initialize(); 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. // Returns true if the touch-optimized UI material design mode is enabled.
static bool IsTouchOptimizedUiEnabled(); static bool IsTouchOptimizedUiEnabled();
...@@ -55,6 +65,7 @@ class UI_BASE_EXPORT MaterialDesignController { ...@@ -55,6 +65,7 @@ class UI_BASE_EXPORT MaterialDesignController {
friend class test::MaterialDesignControllerTestAPI; friend class test::MaterialDesignControllerTestAPI;
MaterialDesignController(); MaterialDesignController();
~MaterialDesignController() = delete; ~MaterialDesignController() = delete;
// Resets the initialization state to uninitialized. To be used by tests to // Resets the initialization state to uninitialized. To be used by tests to
......
...@@ -139,9 +139,9 @@ TEST(MaterialDesignControllerObserver, InitializationOnMdModeChanged) { ...@@ -139,9 +139,9 @@ TEST(MaterialDesignControllerObserver, InitializationOnMdModeChanged) {
TEST(MaterialDesignControllerObserver, TabletOnMdModeChanged) { TEST(MaterialDesignControllerObserver, TabletOnMdModeChanged) {
// Verifies that the MaterialDesignControllerObserver gets called back when // Verifies that the MaterialDesignControllerObserver gets called back when
// the tablet mode toggles. // the tablet mode toggles.
MaterialDesignController::Initialize();
test::MaterialDesignControllerTestAPI::SetDynamicRefreshUi(true); test::MaterialDesignControllerTestAPI::SetDynamicRefreshUi(true);
MaterialDesignController::Initialize();
TestObserver tablet_enabled_observer; TestObserver tablet_enabled_observer;
MaterialDesignController::GetInstance()->AddObserver( MaterialDesignController::GetInstance()->AddObserver(
&tablet_enabled_observer); &tablet_enabled_observer);
...@@ -166,13 +166,13 @@ TEST(MaterialDesignControllerObserver, TabletOnMdModeChanged) { ...@@ -166,13 +166,13 @@ TEST(MaterialDesignControllerObserver, TabletOnMdModeChanged) {
EXPECT_TRUE(tablet_disabled_observer.on_md_mode_changed_called()); EXPECT_TRUE(tablet_disabled_observer.on_md_mode_changed_called());
test::MaterialDesignControllerTestAPI::Uninitialize();
MaterialDesignController::GetInstance()->RemoveObserver( MaterialDesignController::GetInstance()->RemoveObserver(
&tablet_disabled_observer); &tablet_disabled_observer);
MaterialDesignController::GetInstance()->RemoveObserver( MaterialDesignController::GetInstance()->RemoveObserver(
&tablet_enabled_observer); &tablet_enabled_observer);
test::MaterialDesignControllerTestAPI::SetDynamicRefreshUi(false); test::MaterialDesignControllerTestAPI::SetDynamicRefreshUi(false);
test::MaterialDesignControllerTestAPI::Uninitialize();
} }
} // namespace ui } // namespace ui
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment