Skip to content

Commit

Permalink
Restrict HideShelfControlsInTabletMode to an allowlist of boards
Browse files Browse the repository at this point in the history
BUG=1114825

Change-Id: I6ea30ae392c10ecba2cb5d265829ddfe07f30b72
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2352527
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797857}
  • Loading branch information
Toni Barzic authored and Commit Bot committed Aug 13, 2020
1 parent 675407d commit e77ebb6
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 18 deletions.
40 changes: 37 additions & 3 deletions ash/public/cpp/ash_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,34 @@

#include "ash/public/cpp/ash_features.h"

#include <vector>

#include "ash/public/cpp/ash_switches.h"
#include "base/command_line.h"
#include "base/feature_list.h"
#include "base/strings/string_split.h"
#include "base/system/sys_info.h"
#include "build/build_config.h"
#include "chromeos/constants/chromeos_switches.h"

namespace ash {
namespace features {

namespace {

bool HideShelfControlButtonsEnabledForCurrentBoard() {
std::vector<std::string> board =
base::SplitString(base::SysInfo::GetLsbReleaseBoard(), "-",
base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
if (board.empty())
return false;
return board[0] == "kukui" || board[0] == "hatch" || board[0] == "eve" ||
board[0] == "meowth" || board[0] == "hana" || board[0] == "cyan" ||
board[0] == "scarlet";
}

} // namespace

const base::Feature kAllowAmbientEQ{"AllowAmbientEQ",
base::FEATURE_DISABLED_BY_DEFAULT};

Expand Down Expand Up @@ -104,7 +123,11 @@ const base::Feature kEnableBackgroundBlur{"EnableBackgroundBlur",
base::FEATURE_ENABLED_BY_DEFAULT};

const base::Feature kHideShelfControlsInTabletMode{
"HideShelfControlsInTabletMode", base::FEATURE_ENABLED_BY_DEFAULT};
"HideShelfControlsInTabletMode", base::FEATURE_DISABLED_BY_DEFAULT};

const base::Feature kHideShelfControlsInTabletModeForAllowedBoards{
"HideShelfControlsInTabletModeForAllowedBoard",
base::FEATURE_ENABLED_BY_DEFAULT};

const base::Feature kSystemTrayMicGainSetting{"SystemTrayMicGainSetting",
base::FEATURE_ENABLED_BY_DEFAULT};
Expand Down Expand Up @@ -222,8 +245,19 @@ bool IsReduceDisplayNotificationsEnabled() {
}

bool IsHideShelfControlsInTabletModeEnabled() {
return base::FeatureList::IsEnabled(kHideShelfControlsInTabletMode) &&
IsDragFromShelfToHomeOrOverviewEnabled();
if (!IsDragFromShelfToHomeOrOverviewEnabled())
return false;

// Use enabled by default feature it the current board is in the
// default-by-default allowlist.
static const bool enabled_for_board =
HideShelfControlButtonsEnabledForCurrentBoard();
if (enabled_for_board) {
return base::FeatureList::IsEnabled(
kHideShelfControlsInTabletModeForAllowedBoards);
}

return base::FeatureList::IsEnabled(kHideShelfControlsInTabletMode);
}

bool AreContextualNudgesEnabled() {
Expand Down
5 changes: 5 additions & 0 deletions ash/public/cpp/ash_features.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,11 @@ ASH_PUBLIC_EXPORT extern const base::Feature kEnableBackgroundBlur;
// preferences, or policy).
ASH_PUBLIC_EXPORT extern const base::Feature kHideShelfControlsInTabletMode;

// Same as kHideShelfCOntrolsInTabletMode, but enabled by default. Used if the
// current board is allowlisted for the feature.
ASH_PUBLIC_EXPORT extern const base::Feature
kHideShelfControlsInTabletModeForAllowedBoards;

// Enables sliders for setting mic gain levels in the more audio settings
// section in the system tray.
ASH_PUBLIC_EXPORT extern const base::Feature kSystemTrayMicGainSetting;
Expand Down
6 changes: 4 additions & 2 deletions ash/shelf/contextual_tooltip_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@ class ContextualTooltipTest : public AshTestBase,
public:
ContextualTooltipTest() {
if (GetParam()) {
scoped_feature_list_.InitAndEnableFeature(
ash::features::kContextualNudges);
scoped_feature_list_.InitWithFeatures(
{ash::features::kContextualNudges,
ash::features::kHideShelfControlsInTabletMode},
{});

} else {
scoped_feature_list_.InitAndDisableFeature(
Expand Down
5 changes: 4 additions & 1 deletion ash/shelf/shelf_drag_handle_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ ShelfLayoutManager* GetShelfLayoutManager() {
class DragHandleContextualNudgeTest : public ShelfLayoutManagerTestBase {
public:
DragHandleContextualNudgeTest() {
scoped_feature_list_.InitAndEnableFeature(ash::features::kContextualNudges);
scoped_feature_list_.InitWithFeatures(
{ash::features::kContextualNudges,
ash::features::kHideShelfControlsInTabletMode},
{});
}
~DragHandleContextualNudgeTest() override = default;

Expand Down
36 changes: 29 additions & 7 deletions ash/shelf/shelf_widget_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "ash/keyboard/ui/keyboard_ui_controller.h"
#include "ash/keyboard/ui/keyboard_util.h"
#include "ash/keyboard/ui/test/keyboard_test_util.h"
#include "ash/public/cpp/ash_features.h"
#include "ash/public/cpp/ash_switches.h"
#include "ash/public/cpp/keyboard/keyboard_switches.h"
#include "ash/public/cpp/shelf_config.h"
Expand All @@ -28,6 +29,7 @@
#include "ash/wm/window_util.h"
#include "base/bind_helpers.h"
#include "base/run_loop.h"
#include "base/test/scoped_feature_list.h"
#include "components/session_manager/session_manager_types.h"
#include "ui/aura/window_event_dispatcher.h"
#include "ui/compositor/scoped_animation_duration_scale_mode.h"
Expand Down Expand Up @@ -138,21 +140,33 @@ TEST_F(ShelfWidgetTest, TestAlignmentForMultipleDisplays) {
}
}

class ShelfWidgetLayoutBasicsTest : public ShelfWidgetTest,
public testing::WithParamInterface<bool> {
class ShelfWidgetLayoutBasicsTest
: public ShelfWidgetTest,
public testing::WithParamInterface<std::tuple<bool, bool>> {
public:
ShelfWidgetLayoutBasicsTest() = default;
ShelfWidgetLayoutBasicsTest() {
scoped_feature_list_.InitWithFeatureState(
features::kHideShelfControlsInTabletMode, std::get<1>(GetParam()));
}
~ShelfWidgetLayoutBasicsTest() override = default;

void SetUp() override {
ShelfWidgetTest::SetUp();

if (GetParam())
Shell::Get()->tablet_mode_controller()->SetEnabledForTest(true);
Shell::Get()->tablet_mode_controller()->SetEnabledForTest(
std::get<0>(GetParam()));
}

private:
base::test::ScopedFeatureList scoped_feature_list_;
};

INSTANTIATE_TEST_SUITE_P(All, ShelfWidgetLayoutBasicsTest, testing::Bool());
// First parameter - whether test should run in tablet mode.
// Second parameter - whether shelf navigation buttons are enabled in tablet
// mode.
INSTANTIATE_TEST_SUITE_P(All,
ShelfWidgetLayoutBasicsTest,
testing::Combine(testing::Bool(), testing::Bool()));

// Makes sure the shelf is initially sized correctly in clamshell/tablet.
TEST_P(ShelfWidgetLayoutBasicsTest, LauncherInitiallySized) {
Expand All @@ -169,8 +183,15 @@ TEST_P(ShelfWidgetLayoutBasicsTest, LauncherInitiallySized) {
screen_util::GetDisplayBoundsWithShelf(shelf_widget->GetNativeWindow())
.width();

const gfx::Rect nav_widget_clip_rect =
shelf_widget->navigation_widget()->GetLayer()->clip_rect();
if (!nav_widget_clip_rect.IsEmpty())
EXPECT_EQ(gfx::Point(), nav_widget_clip_rect.origin());

int nav_width =
shelf_widget->navigation_widget()->GetWindowBoundsInScreen().width();
nav_widget_clip_rect.IsEmpty()
? shelf_widget->navigation_widget()->GetWindowBoundsInScreen().width()
: nav_widget_clip_rect.width();
ASSERT_GE(nav_width, 0);
if (nav_width) {
nav_width -= ShelfConfig::Get()->control_button_edge_spacing(
Expand All @@ -180,6 +201,7 @@ TEST_P(ShelfWidgetLayoutBasicsTest, LauncherInitiallySized) {
const int hotseat_width =
shelf_widget->hotseat_widget()->GetWindowBoundsInScreen().width();
const int margins = 2 * ShelfConfig::Get()->GetAppIconGroupMargin();

EXPECT_EQ(status_width, total_width - nav_width - hotseat_width - margins);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ static constexpr int kSwipingDistanceForGoingBack = 80;
class BackGestureContextualNudgeControllerTest : public NoSessionAshTestBase {
public:
explicit BackGestureContextualNudgeControllerTest(bool can_go_back = true)
: can_go_back_(can_go_back) {}
: can_go_back_(can_go_back) {
scoped_feature_list_.InitWithFeatures(
{features::kContextualNudges, features::kHideShelfControlsInTabletMode},
{});
}
~BackGestureContextualNudgeControllerTest() override = default;

// NoSessionAshTestBase:
Expand All @@ -49,10 +53,6 @@ class BackGestureContextualNudgeControllerTest : public NoSessionAshTestBase {
}
NoSessionAshTestBase::SetUp(std::move(delegate));

scoped_feature_list_.InitWithFeatures(
{features::kContextualNudges, features::kHideShelfControlsInTabletMode},
{});

GetSessionControllerClient()->AddUserSession(kUser1Email);
GetSessionControllerClient()->AddUserSession(kUser2Email);

Expand Down

0 comments on commit e77ebb6

Please sign in to comment.