From ff12bb1415a9d09340898a224d5009dcaee2e32b Mon Sep 17 00:00:00 2001 From: Manu Cornet Date: Thu, 20 Jun 2019 16:52:06 +0000 Subject: [PATCH] CrOS Shelf: Remove home and back ShelfItem types entirely Bug: 971426 Change-Id: I7552371fcfa005489dbc927c057fc0b84466508c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1666465 Reviewed-by: Xiyuan Xia Commit-Queue: Manu Cornet Cr-Commit-Position: refs/heads/master@{#670933} --- ash/metrics/user_metrics_recorder.cc | 3 +- ash/public/cpp/shelf_model.cc | 3 -- ash/public/cpp/shelf_types.cc | 6 ++-- ash/public/cpp/shelf_types.h | 8 ----- ash/shelf/shelf.cc | 33 +++++++------------ ash/shelf/shelf_view.cc | 18 ++-------- ash/shelf/shelf_view_test_api.cc | 8 ----- ash/shelf/shelf_view_unittest.cc | 9 ----- .../launcher/chrome_launcher_controller.cc | 2 -- .../chrome_launcher_controller_unittest.cc | 6 ---- 10 files changed, 16 insertions(+), 80 deletions(-) diff --git a/ash/metrics/user_metrics_recorder.cc b/ash/metrics/user_metrics_recorder.cc index 388baae48b3158..d0386cea59f036 100644 --- a/ash/metrics/user_metrics_recorder.cc +++ b/ash/metrics/user_metrics_recorder.cc @@ -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; } diff --git a/ash/public/cpp/shelf_model.cc b/ash/public/cpp/shelf_model.cc index ed43043d0189d5..2b6efce94a658f 100644 --- a/ash/public/cpp/shelf_model.cc +++ b/ash/public/cpp/shelf_model.cc @@ -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; diff --git a/ash/public/cpp/shelf_types.cc b/ash/public/cpp/shelf_types.cc index 9606c45c99bc3e..2d2370860b9db1 100644 --- a/ash/public/cpp/shelf_types.cc +++ b/ash/public/cpp/shelf_types.cc @@ -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) { diff --git a/ash/public/cpp/shelf_types.h b/ash/public/cpp/shelf_types.h index 571f0ec967c257..7941219f0e25f7 100644 --- a/ash/public/cpp/shelf_types.h +++ b/ash/public/cpp/shelf_types.h @@ -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, @@ -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, }; diff --git a/ash/shelf/shelf.cc b/ash/shelf/shelf.cc index 937c7cbe8c4fdc..df2b89dc06848f 100644 --- a/ash/shelf/shelf.cc +++ b/ash/shelf/shelf.cc @@ -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 diff --git a/ash/shelf/shelf_view.cc b/ash/shelf/shelf_view.cc index 2130b20c4bd676..7177167b2eca83 100644 --- a/ash/shelf/shelf_view.cc +++ b/ash/shelf/shelf_view.cc @@ -254,8 +254,6 @@ bool ShelfButtonIsInDrag(const ShelfItemType item_type, return static_cast(item_view)->state() & ShelfAppButton::STATE_DRAGGING; case TYPE_DIALOG: - case TYPE_BACK_BUTTON_DEPRECATED: - case TYPE_APP_LIST_DEPRECATED: case TYPE_UNDEFINED: return false; } @@ -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; @@ -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; } @@ -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) @@ -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: @@ -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(source)->OnMenuClosed(); shelf_menu_model_adapter_.reset(); diff --git a/ash/shelf/shelf_view_test_api.cc b/ash/shelf/shelf_view_test_api.cc index b42b8c0a05717a..d42ade5ce22765 100644 --- a/ash/shelf/shelf_view_test_api.cc +++ b/ash/shelf/shelf_view_test_api.cc @@ -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(GetViewAt(index)); } diff --git a/ash/shelf/shelf_view_unittest.cc b/ash/shelf/shelf_view_unittest.cc index 9e8f0cb47c1da2..bc51aad565787e 100644 --- a/ash/shelf/shelf_view_unittest.cc +++ b/ash/shelf/shelf_view_unittest.cc @@ -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; @@ -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 diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc index 63b243346cc168..44d146c1280423 100644 --- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc +++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc @@ -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); } diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc index 57bba6d286c15d..d2867997b78cbb 100644 --- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc +++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc @@ -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;