Skip to content

Commit

Permalink
CrOS Shelf: Remove home and back ShelfItem types entirely
Browse files Browse the repository at this point in the history
Bug: 971426
Change-Id: I7552371fcfa005489dbc927c057fc0b84466508c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1666465
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Manu Cornet <manucornet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#670933}
  • Loading branch information
Manu Cornet authored and Commit Bot committed Jun 20, 2019
1 parent a544244 commit ff12bb1
Show file tree
Hide file tree
Showing 10 changed files with 16 additions and 80 deletions.
3 changes: 1 addition & 2 deletions ash/metrics/user_metrics_recorder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,7 @@ void RecordShelfItemCounts() {
for (const ShelfItem& item : ShelfModel::Get()->items()) {
if (item.type == TYPE_PINNED_APP || item.type == TYPE_BROWSER_SHORTCUT)
++pinned_item_count;
else if (item.type != TYPE_APP_LIST_DEPRECATED &&
item.type != TYPE_BACK_BUTTON_DEPRECATED)
else
++unpinned_item_count;
}

Expand Down
3 changes: 0 additions & 3 deletions ash/public/cpp/shelf_model.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ static ShelfModel* g_shelf_model = nullptr;

int ShelfItemTypeToWeight(ShelfItemType type) {
switch (type) {
case TYPE_APP_LIST_DEPRECATED:
case TYPE_BACK_BUTTON_DEPRECATED:
return 0;
case TYPE_BROWSER_SHORTCUT:
case TYPE_PINNED_APP:
return 1;
Expand Down
6 changes: 2 additions & 4 deletions ash/public/cpp/shelf_types.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,8 @@ constexpr char kDelimiter[] = "|";
} // namespace

bool IsValidShelfItemType(int64_t type) {
return type == TYPE_PINNED_APP || type == TYPE_APP_LIST_DEPRECATED ||
type == TYPE_BROWSER_SHORTCUT || type == TYPE_APP ||
type == TYPE_DIALOG || type == TYPE_BACK_BUTTON_DEPRECATED ||
type == TYPE_UNDEFINED;
return type == TYPE_PINNED_APP || type == TYPE_BROWSER_SHORTCUT ||
type == TYPE_APP || type == TYPE_DIALOG || type == TYPE_UNDEFINED;
}

bool SamePinState(ShelfItemType a, ShelfItemType b) {
Expand Down
8 changes: 0 additions & 8 deletions ash/public/cpp/shelf_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,6 @@ enum ShelfItemType {
// Represents a pinned shortcut to an app, the app may be running or not.
TYPE_PINNED_APP,

// Toggles visibility of the app list. Deprecated, moved to a separate widget
// instead of being shown as a shelf item.
TYPE_APP_LIST_DEPRECATED,

// The browser shortcut button, the browser may be running or not.
TYPE_BROWSER_SHORTCUT,

Expand All @@ -131,10 +127,6 @@ enum ShelfItemType {
// Represents an open dialog.
TYPE_DIALOG,

// Represents the back button, which is shown in tablet mode. Deprecated,
// shown in a separate widget instead of as a shelf item.
TYPE_BACK_BUTTON_DEPRECATED,

// Default value.
TYPE_UNDEFINED,
};
Expand Down
33 changes: 11 additions & 22 deletions ash/shelf/shelf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,29 +71,18 @@ Shelf* Shelf::ForWindow(aura::Window* window) {

// static
void Shelf::LaunchShelfItem(int item_index) {
const ShelfModel* shelf_model = ShelfModel::Get();
const ShelfItems& items = shelf_model->items();
int item_count = shelf_model->item_count();
int indexes_left = item_index >= 0 ? item_index : item_count;
int found_index = -1;

// Iterating until we have hit the index we are interested in which
// is true once indexes_left becomes negative.
for (int i = 0; i < item_count && indexes_left >= 0; i++) {
if (items[i].type != TYPE_APP_LIST_DEPRECATED &&
items[i].type != TYPE_BACK_BUTTON_DEPRECATED) {
found_index = i;
indexes_left--;
}
}
const int item_count = ShelfModel::Get()->item_count();

// There are two ways how found_index can be valid: a.) the nth item was
// found (which is true when indexes_left is -1) or b.) the last item was
// requested (which is true when index was passed in as a negative number).
if (found_index >= 0 && (indexes_left == -1 || item_index < 0)) {
// Then set this one as active (or advance to the next item of its kind).
ActivateShelfItem(found_index);
}
// A negative argument will launch the last app. A positive argument will
// launch the app at the corresponding index, unless it's higher than the
// total number of apps, in which case we do nothing.
if (item_index >= item_count)
return;

const int found_index = item_index >= 0 ? item_index : item_count - 1;

// Set this one as active (or advance to the next item of its kind).
ActivateShelfItem(found_index);
}

// static
Expand Down
18 changes: 2 additions & 16 deletions ash/shelf/shelf_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,6 @@ bool ShelfButtonIsInDrag(const ShelfItemType item_type,
return static_cast<const ShelfAppButton*>(item_view)->state() &
ShelfAppButton::STATE_DRAGGING;
case TYPE_DIALOG:
case TYPE_BACK_BUTTON_DEPRECATED:
case TYPE_APP_LIST_DEPRECATED:
case TYPE_UNDEFINED:
return false;
}
Expand Down Expand Up @@ -671,12 +669,6 @@ void ShelfView::ButtonPressed(views::Button* sender,
UMA_LAUNCHER_CLICK_ON_APP);
break;

case TYPE_APP_LIST_DEPRECATED:
Shell::Get()->metrics()->RecordUserMetricsAction(
UMA_LAUNCHER_CLICK_ON_APPLIST_BUTTON);
break;

case TYPE_BACK_BUTTON_DEPRECATED:
case TYPE_DIALOG:
break;

Expand Down Expand Up @@ -906,9 +898,6 @@ views::View* ShelfView::CreateViewForItem(const ShelfItem& item) {
break;
}

// The home and back buttons are created separately.
case TYPE_APP_LIST_DEPRECATED:
case TYPE_BACK_BUTTON_DEPRECATED:
case TYPE_UNDEFINED:
return nullptr;
}
Expand Down Expand Up @@ -1598,8 +1587,7 @@ void ShelfView::FinalizeRipOffDrag(bool cancel) {
ShelfView::RemovableState ShelfView::RemovableByRipOff(int index) const {
DCHECK(index >= 0 && index < model_->item_count());
ShelfItemType type = model_->items()[index].type;
if (type == TYPE_APP_LIST_DEPRECATED || type == TYPE_DIALOG ||
type == TYPE_BACK_BUTTON_DEPRECATED)
if (type == TYPE_DIALOG)
return NOT_REMOVABLE;

if (model_->items()[index].pinned_by_policy)
Expand All @@ -1616,9 +1604,7 @@ bool ShelfView::SameDragType(ShelfItemType typea, ShelfItemType typeb) const {
case TYPE_PINNED_APP:
case TYPE_BROWSER_SHORTCUT:
return (typeb == TYPE_PINNED_APP || typeb == TYPE_BROWSER_SHORTCUT);
case TYPE_APP_LIST_DEPRECATED:
case TYPE_APP:
case TYPE_BACK_BUTTON_DEPRECATED:
case TYPE_DIALOG:
return typeb == typea;
case TYPE_UNDEFINED:
Expand Down Expand Up @@ -2205,7 +2191,7 @@ void ShelfView::OnMenuClosed(views::View* source) {
closing_event_time_ = shelf_menu_model_adapter_->GetClosingEventTime();

const ShelfItem* item = ShelfItemForView(source);
if (item && (item->type != TYPE_APP_LIST_DEPRECATED))
if (item)
static_cast<ShelfAppButton*>(source)->OnMenuClosed();

shelf_menu_model_adapter_.reset();
Expand Down
8 changes: 0 additions & 8 deletions ash/shelf/shelf_view_test_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,6 @@ int ShelfViewTestAPI::GetButtonCount() {
}

ShelfAppButton* ShelfViewTestAPI::GetButton(int index) {
// App list and back button are not ShelfAppButtons.
if (shelf_view_->model_->items()[index].type ==
ash::TYPE_APP_LIST_DEPRECATED ||
shelf_view_->model_->items()[index].type ==
ash::TYPE_BACK_BUTTON_DEPRECATED) {
return nullptr;
}

return static_cast<ShelfAppButton*>(GetViewAt(index));
}

Expand Down
9 changes: 0 additions & 9 deletions ash/shelf/shelf_view_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,6 @@ class ShelfViewTest : public AshTestBase {
// since the app list button is not a ShelfAppButton.
ShelfAppButton* SimulateButtonPressed(ShelfView::Pointer pointer,
int button_index) {
EXPECT_NE(TYPE_APP_LIST_DEPRECATED, model_->items()[button_index].type);
ShelfAppButton* button = test_api_->GetButton(button_index);
EXPECT_EQ(button, SimulateViewPressed(pointer, button_index));
return button;
Expand Down Expand Up @@ -744,20 +743,12 @@ TEST_F(ShelfViewTest, EnforceDragType) {
EXPECT_TRUE(test_api_->SameDragType(TYPE_APP, TYPE_APP));
EXPECT_FALSE(test_api_->SameDragType(TYPE_APP, TYPE_PINNED_APP));
EXPECT_FALSE(test_api_->SameDragType(TYPE_APP, TYPE_BROWSER_SHORTCUT));
EXPECT_FALSE(test_api_->SameDragType(TYPE_APP, TYPE_APP_LIST_DEPRECATED));

EXPECT_TRUE(test_api_->SameDragType(TYPE_PINNED_APP, TYPE_PINNED_APP));
EXPECT_TRUE(test_api_->SameDragType(TYPE_PINNED_APP, TYPE_BROWSER_SHORTCUT));
EXPECT_FALSE(
test_api_->SameDragType(TYPE_PINNED_APP, TYPE_APP_LIST_DEPRECATED));

EXPECT_TRUE(
test_api_->SameDragType(TYPE_BROWSER_SHORTCUT, TYPE_BROWSER_SHORTCUT));
EXPECT_FALSE(
test_api_->SameDragType(TYPE_BROWSER_SHORTCUT, TYPE_APP_LIST_DEPRECATED));

EXPECT_TRUE(test_api_->SameDragType(TYPE_APP_LIST_DEPRECATED,
TYPE_APP_LIST_DEPRECATED));
}

// Adds platform app button until overflow and verifies that the last added
Expand Down
2 changes: 0 additions & 2 deletions chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1238,8 +1238,6 @@ void ChromeLauncherController::ShelfItemMoved(int start_index,
int target_index) {
// Update the pin position preference as needed.
const ash::ShelfItem& item = model_->items()[target_index];
DCHECK_NE(ash::TYPE_BACK_BUTTON_DEPRECATED, item.type);
DCHECK_NE(ash::TYPE_APP_LIST_DEPRECATED, item.type);
if (ItemTypeIsPinned(item) && should_sync_pin_changes_)
SyncPinPosition(item.id);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -773,12 +773,6 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest {
case ash::TYPE_BROWSER_SHORTCUT:
result += "Chrome";
break;
case ash::TYPE_APP_LIST_DEPRECATED:
result += "AppList";
break;
case ash::TYPE_BACK_BUTTON_DEPRECATED:
result += "Back";
break;
default:
result += "Unknown";
break;
Expand Down

0 comments on commit ff12bb1

Please sign in to comment.