diff options
33 files changed, 852 insertions, 430 deletions
diff --git a/ash/launcher/launcher.cc b/ash/launcher/launcher.cc index f37b05c..952c6f5 100644 --- a/ash/launcher/launcher.cc +++ b/ash/launcher/launcher.cc @@ -63,14 +63,16 @@ Launcher::~Launcher() { // static Launcher* Launcher::ForPrimaryDisplay() { - return internal::RootWindowController::ForLauncher( - Shell::GetPrimaryRootWindow())->shelf()->launcher(); + ShelfWidget* shelf_widget = internal::RootWindowController::ForLauncher( + Shell::GetPrimaryRootWindow())->shelf(); + return shelf_widget ? shelf_widget->launcher() : NULL; } // static Launcher* Launcher::ForWindow(aura::Window* window) { - return internal::RootWindowController::ForLauncher(window)-> - shelf()->launcher(); + ShelfWidget* shelf_widget = + internal::RootWindowController::ForLauncher(window)->shelf(); + return shelf_widget ? shelf_widget->launcher() : NULL; } void Launcher::SetAlignment(ShelfAlignment alignment) { @@ -108,20 +110,6 @@ void Launcher::ActivateLauncherItem(int index) { ui::VKEY_UNKNOWN, // The actual key gets ignored. ui::EF_NONE, false); - // TODO(skuhne): Remove this temporary fix once M28 is out and CL 11596003 - // has landed. Note that all unit tests which use this function need to remove - // the "kChromeItemOffset" as well. - // Note: The index gets only decremented for the per app instance of the - // launcher. The old per browser launcher does not get impacted here. - if (delegate_->IsPerAppLauncher() && --index <= 0) { - LauncherItem item; - // Create a fake launcher item with an invalid ID so that - // our ChromeLauncherControllerPerApp can handle it accordingly. This is - // only a temporary fix until CL 11596003 has landed. - item.id = ash::kAppIdForBrowserSwitching; - delegate_->ItemSelected(item, event); - return; - } const ash::LauncherItems& items = launcher_view_->model()->items(); @@ -177,8 +165,7 @@ void Launcher::SwitchToWindow(int window_index) { // 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 && - items[i].type != TYPE_BROWSER_SHORTCUT) { + if (items[i].type != TYPE_APP_LIST) { found_index = i; indexes_left--; } diff --git a/ash/launcher/launcher_delegate.h b/ash/launcher/launcher_delegate.h index 0bd8ec6..a5c1842 100644 --- a/ash/launcher/launcher_delegate.h +++ b/ash/launcher/launcher_delegate.h @@ -21,11 +21,6 @@ class Event; namespace ash { class Launcher; -// When passed to LauncherDelegate::ItemSelected, the browser item will be -// addressed for switching. -// TODO(skuhne): Remove this constant once CL 11596003 has landed. -const LauncherID kAppIdForBrowserSwitching = -1; - // A special menu model which keeps track of an "active" menu item. class ASH_EXPORT LauncherMenuModel : public ui::SimpleMenuModel { public: @@ -46,11 +41,6 @@ class ASH_EXPORT LauncherDelegate { // Launcher owns the delegate. virtual ~LauncherDelegate() {} - // Invoked when the user clicks on button in the launcher to open last used - // window (or create a new one if there is no last used window). - // |event_flags| is the flags of the click event. - virtual void OnBrowserShortcutClicked(int event_flags) = 0; - // Invoked when the user clicks on a window entry in the launcher. // |event| is the click event. The |event| is dispatched by a view // and has an instance of |views::View| as the event target @@ -60,10 +50,6 @@ class ASH_EXPORT LauncherDelegate { virtual void ItemSelected(const LauncherItem& item, const ui::Event& event) = 0; - // Returns the resource id of the image to show on the browser shortcut - // button. - virtual int GetBrowserShortcutResourceId() = 0; - // Returns the title to display for the specified launcher item. virtual base::string16 GetTitle(const LauncherItem& item) = 0; diff --git a/ash/launcher/launcher_model.cc b/ash/launcher/launcher_model.cc index f3581fe..cac37c0 100644 --- a/ash/launcher/launcher_model.cc +++ b/ash/launcher/launcher_model.cc @@ -15,21 +15,20 @@ namespace { int LauncherItemTypeToWeight(LauncherItemType type) { switch (type) { case TYPE_BROWSER_SHORTCUT: - return 0; case TYPE_APP_SHORTCUT: case TYPE_WINDOWED_APP: - return 1; + return 0; case TYPE_TABBED: case TYPE_PLATFORM_APP: - return 2; + return 1; case TYPE_APP_LIST: - return 3; + return 2; case TYPE_APP_PANEL: - return 4; + return 3; } NOTREACHED() << "Invalid type " << type; - return 2; + return 1; } bool CompareByWeight(const LauncherItem& a, const LauncherItem& b) { @@ -42,13 +41,7 @@ LauncherModel::LauncherModel() : next_id_(1), status_(STATUS_NORMAL) { LauncherItem app_list; app_list.type = TYPE_APP_LIST; app_list.is_incognito = false; - - LauncherItem browser_shortcut; - browser_shortcut.type = TYPE_BROWSER_SHORTCUT; - browser_shortcut.is_incognito = false; - - AddAt(0, browser_shortcut); - AddAt(1, app_list); + AddAt(0, app_list); } LauncherModel::~LauncherModel() { diff --git a/ash/launcher/launcher_model_unittest.cc b/ash/launcher/launcher_model_unittest.cc index fb29267..0491ccd 100644 --- a/ash/launcher/launcher_model_unittest.cc +++ b/ash/launcher/launcher_model_unittest.cc @@ -77,17 +77,16 @@ TEST(LauncherModel, BasicAssertions) { TestLauncherModelObserver observer; LauncherModel model; - // Model is initially populated with two items. - EXPECT_EQ(2, model.item_count()); - // Two initial items should have different ids. - EXPECT_NE(model.items()[0].id, model.items()[1].id); + // Model is initially populated with item. + EXPECT_EQ(1, model.item_count()); // Add an item. model.AddObserver(&observer); LauncherItem item; int index = model.Add(item); - EXPECT_EQ(3, model.item_count()); + EXPECT_EQ(2, model.item_count()); EXPECT_EQ("added=1", observer.StateStringAndClear()); + // Verifies all the items get unique ids. std::set<LauncherID> ids; for (int i = 0; i < model.item_count(); ++i) @@ -103,7 +102,7 @@ TEST(LauncherModel, BasicAssertions) { // Remove the item. model.RemoveItemAt(index); - EXPECT_EQ(2, model.item_count()); + EXPECT_EQ(1, model.item_count()); EXPECT_EQ("removed=1", observer.StateStringAndClear()); // Add an app item. @@ -135,45 +134,49 @@ TEST(LauncherModel, AddIndices) { TestLauncherModelObserver observer; LauncherModel model; - // Model is initially populated with two items. - EXPECT_EQ(2, model.item_count()); - // Two initial items should have different ids. - EXPECT_NE(model.items()[0].id, model.items()[1].id); + // Model is initially populated with one item. + EXPECT_EQ(1, model.item_count()); - // Items come after the browser. - int browser = 0; - ASSERT_EQ(ash::TYPE_BROWSER_SHORTCUT, model.items()[browser].type); + // Insert browser short cut at index 0. + LauncherItem browser_shortcut; + browser_shortcut.type = TYPE_BROWSER_SHORTCUT; + int browser_shortcut_index = model.Add(browser_shortcut); + EXPECT_EQ(0, browser_shortcut_index); - // Tabbed items should be after shortcut. + // Tabbed items should be after browser shortcut. LauncherItem item; int tabbed_index1 = model.Add(item); - EXPECT_EQ(browser + 1, tabbed_index1); + EXPECT_EQ(1, tabbed_index1); // Add another tabbed item, it should follow first. int tabbed_index2 = model.Add(item); - EXPECT_EQ(browser + 2, tabbed_index2); + EXPECT_EQ(2, tabbed_index2); - // APP_SHORTCUT preceed browsers. + // APP_SHORTCUT's priority is higher than TABBED but same as + // BROWSER_SHORTCUT. So APP_SHORTCUT is located after BROWSER_SHORCUT. item.type = TYPE_APP_SHORTCUT; int app_shortcut_index1 = model.Add(item); - EXPECT_EQ(browser + 1, app_shortcut_index1); + EXPECT_EQ(1, app_shortcut_index1); item.type = TYPE_APP_SHORTCUT; int app_shortcut_index2 = model.Add(item); - EXPECT_EQ(browser + 2, app_shortcut_index2); + EXPECT_EQ(2, app_shortcut_index2); // Check that AddAt() figures out the correct indexes for app shortcuts. + // APP_SHORTCUT and BROWSER_SHORTCUT has the same weight. + // So APP_SHORTCUT is located at index 0. And, BROWSER_SHORTCUT is located at + // index 1. item.type = TYPE_APP_SHORTCUT; int app_shortcut_index3 = model.AddAt(0, item); - EXPECT_EQ(browser + 1, app_shortcut_index3); + EXPECT_EQ(0, app_shortcut_index3); item.type = TYPE_APP_SHORTCUT; int app_shortcut_index4 = model.AddAt(5, item); - EXPECT_EQ(browser + 4, app_shortcut_index4); + EXPECT_EQ(4, app_shortcut_index4); item.type = TYPE_APP_SHORTCUT; int app_shortcut_index5 = model.AddAt(2, item); - EXPECT_EQ(browser + 2, app_shortcut_index5); + EXPECT_EQ(2, app_shortcut_index5); // Before there are any panels, no icons should be right aligned. EXPECT_EQ(model.item_count(), model.FirstPanelIndex()); @@ -181,33 +184,32 @@ TEST(LauncherModel, AddIndices) { // Check that AddAt() figures out the correct indexes for tabs and panels. item.type = TYPE_TABBED; int tabbed_index3 = model.AddAt(2, item); - EXPECT_EQ(browser + 6, tabbed_index3); + EXPECT_EQ(6, tabbed_index3); item.type = TYPE_APP_PANEL; int app_panel_index1 = model.AddAt(2, item); - EXPECT_EQ(browser + 10, app_panel_index1); + EXPECT_EQ(10, app_panel_index1); item.type = TYPE_TABBED; int tabbed_index4 = model.AddAt(11, item); - EXPECT_EQ(browser + 9, tabbed_index4); + EXPECT_EQ(9, tabbed_index4); item.type = TYPE_APP_PANEL; int app_panel_index2 = model.AddAt(12, item); - EXPECT_EQ(browser + 12, app_panel_index2); + EXPECT_EQ(12, app_panel_index2); item.type = TYPE_TABBED; int tabbed_index5 = model.AddAt(7, item); - EXPECT_EQ(browser + 7, tabbed_index5); + EXPECT_EQ(7, tabbed_index5); item.type = TYPE_APP_PANEL; int app_panel_index3 = model.AddAt(13, item); - EXPECT_EQ(browser + 13, app_panel_index3); + EXPECT_EQ(13, app_panel_index3); // Right aligned index should be the first app panel index. - EXPECT_EQ(browser + 12, model.FirstPanelIndex()); + EXPECT_EQ(12, model.FirstPanelIndex()); - // Browser shortcut and app list should still be first and second. - EXPECT_EQ(TYPE_BROWSER_SHORTCUT, model.items()[0].type); + EXPECT_EQ(TYPE_BROWSER_SHORTCUT, model.items()[1].type); EXPECT_EQ(TYPE_APP_LIST, model.items()[model.FirstPanelIndex() - 1].type); } @@ -216,7 +218,7 @@ TEST(LauncherModel, LauncherIDTests) { TestLauncherModelObserver observer; LauncherModel model; - EXPECT_EQ(2, model.item_count()); + EXPECT_EQ(1, model.item_count()); // Get the next to use ID counter. LauncherID id = model.next_id(); @@ -228,7 +230,7 @@ TEST(LauncherModel, LauncherIDTests) { // but it will not change the item count and retrieving the next ID should // produce something new. EXPECT_EQ(model.reserve_external_id(), id); - EXPECT_EQ(2, model.item_count()); + EXPECT_EQ(1, model.item_count()); LauncherID id2 = model.next_id(); EXPECT_NE(id2, id); diff --git a/ash/launcher/launcher_navigator_unittest.cc b/ash/launcher/launcher_navigator_unittest.cc index 6c91717..6013390 100644 --- a/ash/launcher/launcher_navigator_unittest.cc +++ b/ash/launcher/launcher_navigator_unittest.cc @@ -21,6 +21,16 @@ class LauncherNavigatorTest : public testing::Test { protected: virtual void SetUp() OVERRIDE { model_.reset(new LauncherModel); + + // Initially, applist launcher item is only created. + int total_num = model_->item_count(); + EXPECT_EQ(1, total_num); + EXPECT_TRUE(model_->items()[0].type == TYPE_APP_LIST); + + // Add BROWSER_SHORTCUT for test. + LauncherItem browser_shortcut; + browser_shortcut.type = TYPE_BROWSER_SHORTCUT; + model_->Add(browser_shortcut); } void SetupMockLauncherModel(LauncherItemType* types, diff --git a/ash/launcher/launcher_view.cc b/ash/launcher/launcher_view.cc index 4c2dbf2..f24a1f5 100644 --- a/ash/launcher/launcher_view.cc +++ b/ash/launcher/launcher_view.cc @@ -878,6 +878,7 @@ views::View* LauncherView::CreateViewForItem(const LauncherItem& item) { break; } + case TYPE_BROWSER_SHORTCUT: case TYPE_APP_SHORTCUT: case TYPE_WINDOWED_APP: case TYPE_PLATFORM_APP: @@ -909,18 +910,6 @@ views::View* LauncherView::CreateViewForItem(const LauncherItem& item) { break; } - case TYPE_BROWSER_SHORTCUT: { - ResourceBundle& rb = ResourceBundle::GetSharedInstance(); - LauncherButton* button = LauncherButton::Create( - this, this, tooltip_->shelf_layout_manager()); - int image_id = delegate_ ? - delegate_->GetBrowserShortcutResourceId() : - IDR_AURA_LAUNCHER_BROWSER_SHORTCUT; - button->SetImage(*rb.GetImageNamed(image_id).ToImageSkia()); - view = button; - break; - } - default: break; } @@ -1030,10 +1019,11 @@ bool LauncherView::SameDragType(LauncherItemType typea, case TYPE_PLATFORM_APP: return (typeb == TYPE_TABBED || typeb == TYPE_PLATFORM_APP); case TYPE_APP_SHORTCUT: + case TYPE_BROWSER_SHORTCUT: + return (typeb == TYPE_APP_SHORTCUT || typeb == TYPE_BROWSER_SHORTCUT); case TYPE_WINDOWED_APP: case TYPE_APP_LIST: case TYPE_APP_PANEL: - case TYPE_BROWSER_SHORTCUT: return typeb == typea; } NOTREACHED(); @@ -1424,15 +1414,13 @@ base::string16 LauncherView::GetAccessibleName(const views::View* view) { case TYPE_APP_SHORTCUT: case TYPE_WINDOWED_APP: case TYPE_PLATFORM_APP: + case TYPE_BROWSER_SHORTCUT: return delegate_->GetTitle(model_->items()[view_index]); case TYPE_APP_LIST: return model_->status() == LauncherModel::STATUS_LOADING ? l10n_util::GetStringUTF16(IDS_AURA_APP_LIST_SYNCING_TITLE) : l10n_util::GetStringUTF16(IDS_AURA_APP_LIST_TITLE); - - case TYPE_BROWSER_SHORTCUT: - return Shell::GetInstance()->delegate()->GetProductName(); } return base::string16(); } @@ -1473,6 +1461,7 @@ void LauncherView::ButtonPressed(views::Button* sender, case TYPE_APP_SHORTCUT: case TYPE_WINDOWED_APP: case TYPE_PLATFORM_APP: + case TYPE_BROWSER_SHORTCUT: Shell::GetInstance()->delegate()->RecordUserMetricsAction( UMA_LAUNCHER_CLICK_ON_APP); // Fallthrough @@ -1491,13 +1480,6 @@ void LauncherView::ButtonPressed(views::Button* sender, ash::switches::kAshDragAndDropAppListToLauncher)) Shell::GetInstance()->SetDragAndDropHostOfCurrentAppList(this); break; - - case TYPE_BROWSER_SHORTCUT: - // Click on browser icon is counted in app clicks. - Shell::GetInstance()->delegate()->RecordUserMetricsAction( - UMA_LAUNCHER_CLICK_ON_APP); - delegate_->OnBrowserShortcutClicked(event.flags()); - break; } } diff --git a/ash/launcher/launcher_view_unittest.cc b/ash/launcher/launcher_view_unittest.cc index 8edcc51f..0470b88 100644 --- a/ash/launcher/launcher_view_unittest.cc +++ b/ash/launcher/launcher_view_unittest.cc @@ -34,10 +34,6 @@ #include "ui/views/widget/widget.h" #include "ui/views/widget/widget_delegate.h" -namespace { -const int kExpectedAppIndex = 1; -} - namespace ash { namespace test { @@ -208,7 +204,10 @@ class LauncherViewTest : public AshTestBase { test_api_.reset(new LauncherViewTestAPI(launcher_view_)); test_api_->SetAnimationDuration(1); // Speeds up animation for test. - } + + // Add browser shortcut launcher item at index 0 for test. + AddBrowserShortcut(); + } virtual void TearDown() OVERRIDE { test_api_.reset(); @@ -216,6 +215,17 @@ class LauncherViewTest : public AshTestBase { } protected: + LauncherID AddBrowserShortcut() { + LauncherItem browser_shortcut; + browser_shortcut.type = TYPE_BROWSER_SHORTCUT; + browser_shortcut.is_incognito = false; + + LauncherID id = model_->next_id(); + model_->AddAt(0, browser_shortcut); + test_api_->RunMessageLoopUntilAnimationsDone(); + return id; + } + LauncherID AddAppShortcut() { LauncherItem item; item.type = TYPE_APP_SHORTCUT; @@ -321,20 +331,17 @@ class LauncherViewTest : public AshTestBase { views::View* SimulateDrag(internal::LauncherButtonHost::Pointer pointer, int button_index, int destination_index) { - // Add kExpectedAppIndex to each button index to allow default icons. internal::LauncherButtonHost* button_host = launcher_view_; // Mouse down. - views::View* button = - test_api_->GetButton(kExpectedAppIndex + button_index); + views::View* button = test_api_->GetButton(button_index); ui::MouseEvent click_event(ui::ET_MOUSE_PRESSED, button->bounds().origin(), button->bounds().origin(), 0); button_host->PointerPressedOnButton(button, pointer, click_event); // Drag. - views::View* destination = - test_api_->GetButton(kExpectedAppIndex + destination_index); + views::View* destination = test_api_->GetButton(destination_index); ui::MouseEvent drag_event(ui::ET_MOUSE_DRAGGED, destination->bounds().origin(), destination->bounds().origin(), 0); @@ -354,7 +361,9 @@ class LauncherViewTest : public AshTestBase { // Add 5 app launcher buttons for testing. for (int i = 0; i < 5; ++i) { LauncherID id = AddAppShortcut(); - id_map->insert(id_map->begin() + (kExpectedAppIndex + i), + // browser shortcut is located at index 0. So we should start to add app + // shortcut at index 1. + id_map->insert(id_map->begin() + (i + 1), std::make_pair(id, GetButtonByID(id))); } ASSERT_NO_FATAL_FAILURE(CheckModelIDs(*id_map)); @@ -586,27 +595,40 @@ TEST_F(LauncherViewTest, ModelChangesWhileDragging) { std::vector<std::pair<LauncherID, views::View*> > id_map; SetupForDragTest(&id_map); - // Dragging changes model order. + // Dragging browser shortcut at index 0. + EXPECT_TRUE(model_->items()[0].type == TYPE_BROWSER_SHORTCUT); views::View* dragged_button = SimulateDrag( internal::LauncherButtonHost::MOUSE, 0, 2); - std::rotate(id_map.begin() + kExpectedAppIndex, - id_map.begin() + kExpectedAppIndex + 1, - id_map.begin() + kExpectedAppIndex + 3); + std::rotate(id_map.begin(), + id_map.begin() + 1, + id_map.begin() + 3); + ASSERT_NO_FATAL_FAILURE(CheckModelIDs(id_map)); + button_host->PointerReleasedOnButton(dragged_button, + internal::LauncherButtonHost::MOUSE, + false); + EXPECT_TRUE(model_->items()[2].type == TYPE_BROWSER_SHORTCUT); + + // Dragging changes model order. + dragged_button = SimulateDrag( + internal::LauncherButtonHost::MOUSE, 0, 2); + std::rotate(id_map.begin(), + id_map.begin() + 1, + id_map.begin() + 3); ASSERT_NO_FATAL_FAILURE(CheckModelIDs(id_map)); // Cancelling the drag operation restores previous order. button_host->PointerReleasedOnButton(dragged_button, internal::LauncherButtonHost::MOUSE, true); - std::rotate(id_map.begin() + kExpectedAppIndex, - id_map.begin() + kExpectedAppIndex + 2, - id_map.begin() + kExpectedAppIndex + 3); + std::rotate(id_map.begin(), + id_map.begin() + 2, + id_map.begin() + 3); ASSERT_NO_FATAL_FAILURE(CheckModelIDs(id_map)); // Deleting an item keeps the remaining intact. dragged_button = SimulateDrag(internal::LauncherButtonHost::MOUSE, 0, 2); - model_->RemoveItemAt(kExpectedAppIndex + 1); - id_map.erase(id_map.begin() + kExpectedAppIndex + 1); + model_->RemoveItemAt(1); + id_map.erase(id_map.begin() + 1); ASSERT_NO_FATAL_FAILURE(CheckModelIDs(id_map)); button_host->PointerReleasedOnButton(dragged_button, internal::LauncherButtonHost::MOUSE, @@ -615,7 +637,7 @@ TEST_F(LauncherViewTest, ModelChangesWhileDragging) { // Adding a launcher item cancels the drag and respects the order. dragged_button = SimulateDrag(internal::LauncherButtonHost::MOUSE, 0, 2); LauncherID new_id = AddAppShortcut(); - id_map.insert(id_map.begin() + kExpectedAppIndex + 4, + id_map.insert(id_map.begin() + 5, std::make_pair(new_id, GetButtonByID(new_id))); ASSERT_NO_FATAL_FAILURE(CheckModelIDs(id_map)); button_host->PointerReleasedOnButton(dragged_button, @@ -626,7 +648,7 @@ TEST_F(LauncherViewTest, ModelChangesWhileDragging) { // the order. dragged_button = SimulateDrag(internal::LauncherButtonHost::MOUSE, 0, 2); new_id = AddPanel(); - id_map.insert(id_map.begin() + kExpectedAppIndex + 6, + id_map.insert(id_map.begin() + 7, std::make_pair(new_id, GetButtonByID(new_id))); ASSERT_NO_FATAL_FAILURE(CheckModelIDs(id_map)); button_host->PointerReleasedOnButton(dragged_button, @@ -644,9 +666,9 @@ TEST_F(LauncherViewTest, SimultaneousDrag) { // Start a mouse drag. views::View* dragged_button_mouse = SimulateDrag( internal::LauncherButtonHost::MOUSE, 0, 2); - std::rotate(id_map.begin() + kExpectedAppIndex, - id_map.begin() + kExpectedAppIndex + 1, - id_map.begin() + kExpectedAppIndex + 3); + std::rotate(id_map.begin(), + id_map.begin() + 1, + id_map.begin() + 3); ASSERT_NO_FATAL_FAILURE(CheckModelIDs(id_map)); // Attempt a touch drag before the mouse drag finishes. views::View* dragged_button_touch = SimulateDrag( @@ -664,9 +686,9 @@ TEST_F(LauncherViewTest, SimultaneousDrag) { // Now start a touch drag. dragged_button_touch = SimulateDrag( internal::LauncherButtonHost::TOUCH, 3, 1); - std::rotate(id_map.begin() + kExpectedAppIndex + 2, - id_map.begin() + kExpectedAppIndex + 3, - id_map.begin() + kExpectedAppIndex + 4); + std::rotate(id_map.begin() + 2, + id_map.begin() + 3, + id_map.begin() + 4); ASSERT_NO_FATAL_FAILURE(CheckModelIDs(id_map)); // And attempt a mouse drag before the touch drag finishes. diff --git a/ash/shell/launcher_delegate_impl.cc b/ash/shell/launcher_delegate_impl.cc index 601adac..f933119 100644 --- a/ash/shell/launcher_delegate_impl.cc +++ b/ash/shell/launcher_delegate_impl.cc @@ -21,14 +21,6 @@ LauncherDelegateImpl::LauncherDelegateImpl(WindowWatcher* watcher) LauncherDelegateImpl::~LauncherDelegateImpl() { } -// In the shell we'll create a window all the time. -void LauncherDelegateImpl::OnBrowserShortcutClicked(int event_flags) { - ash::shell::ToplevelWindow::CreateParams create_params; - create_params.can_resize = true; - create_params.can_maximize = true; - ash::shell::ToplevelWindow::CreateToplevelWindow(create_params); -} - void LauncherDelegateImpl::ItemSelected(const ash::LauncherItem& item, const ui::Event& event) { aura::Window* window = watcher_->GetWindowByID(item.id); @@ -38,10 +30,6 @@ void LauncherDelegateImpl::ItemSelected(const ash::LauncherItem& item, ash::wm::ActivateWindow(window); } -int LauncherDelegateImpl::GetBrowserShortcutResourceId() { - return IDR_AURA_LAUNCHER_BROWSER_SHORTCUT; -} - base::string16 LauncherDelegateImpl::GetTitle(const ash::LauncherItem& item) { return watcher_->GetWindowByID(item.id)->title(); } diff --git a/ash/shell/launcher_delegate_impl.h b/ash/shell/launcher_delegate_impl.h index 26b1dfd..754ee69 100644 --- a/ash/shell/launcher_delegate_impl.h +++ b/ash/shell/launcher_delegate_impl.h @@ -25,10 +25,8 @@ class LauncherDelegateImpl : public ash::LauncherDelegate { void set_watcher(WindowWatcher* watcher) { watcher_ = watcher; } // LauncherDelegate overrides: - virtual void OnBrowserShortcutClicked(int event_flags) OVERRIDE; virtual void ItemSelected(const ash::LauncherItem& item, const ui::Event& event) OVERRIDE; - virtual int GetBrowserShortcutResourceId() OVERRIDE; virtual base::string16 GetTitle(const ash::LauncherItem& item) OVERRIDE; virtual ui::MenuModel* CreateContextMenu( const ash::LauncherItem& item, diff --git a/ash/test/test_launcher_delegate.cc b/ash/test/test_launcher_delegate.cc index 3a4fccd..e71fd344 100644 --- a/ash/test/test_launcher_delegate.cc +++ b/ash/test/test_launcher_delegate.cc @@ -63,9 +63,6 @@ void TestLauncherDelegate::OnWillRemoveWindow(aura::Window* window) { } } -void TestLauncherDelegate::OnBrowserShortcutClicked(int event_flags) { -} - void TestLauncherDelegate::ItemSelected(const ash::LauncherItem& item, const ui::Event& event) { aura::Window* window = GetWindowByID(item.id); @@ -75,10 +72,6 @@ void TestLauncherDelegate::ItemSelected(const ash::LauncherItem& item, ash::wm::ActivateWindow(window); } -int TestLauncherDelegate::GetBrowserShortcutResourceId() { - return IDR_AURA_LAUNCHER_BROWSER_SHORTCUT; -} - base::string16 TestLauncherDelegate::GetTitle(const ash::LauncherItem& item) { aura::Window* window = GetWindowByID(item.id); return window ? window->title() : base::string16(); diff --git a/ash/test/test_launcher_delegate.h b/ash/test/test_launcher_delegate.h index 65dc634..60d7411 100644 --- a/ash/test/test_launcher_delegate.h +++ b/ash/test/test_launcher_delegate.h @@ -35,10 +35,8 @@ class TestLauncherDelegate : public LauncherDelegate, virtual void OnWillRemoveWindow(aura::Window* window) OVERRIDE; // LauncherDelegate implementation. - virtual void OnBrowserShortcutClicked(int event_flags) OVERRIDE; virtual void ItemSelected(const LauncherItem& item, const ui::Event& event) OVERRIDE; - virtual int GetBrowserShortcutResourceId() OVERRIDE; virtual base::string16 GetTitle(const LauncherItem& item) OVERRIDE; virtual ui::MenuModel* CreateContextMenu(const LauncherItem& item, aura::RootWindow* root) OVERRIDE; diff --git a/chrome/browser/ui/ash/chrome_launcher_prefs.cc b/chrome/browser/ui/ash/chrome_launcher_prefs.cc index 9475632..e7f9f3b 100644 --- a/chrome/browser/ui/ash/chrome_launcher_prefs.cc +++ b/chrome/browser/ui/ash/chrome_launcher_prefs.cc @@ -58,6 +58,10 @@ void RegisterChromeLauncherUserPrefs( user_prefs::PrefRegistrySyncable* registry) { // TODO: If we want to support multiple profiles this will likely need to be // pushed to local state and we'll need to track profile per item. + registry->RegisterIntegerPref( + prefs::kShelfChromeIconIndex, + 0, + user_prefs::PrefRegistrySyncable::SYNCABLE_PREF); registry->RegisterListPref(prefs::kPinnedLauncherApps, CreateDefaultPinnedAppsList(), user_prefs::PrefRegistrySyncable::SYNCABLE_PREF); diff --git a/chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc b/chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc index f1f156e..95a3112 100644 --- a/chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc +++ b/chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc @@ -125,7 +125,7 @@ void AppShortcutLauncherItemController::LauncherItemChanged( } ChromeLauncherAppMenuItems -AppShortcutLauncherItemController::GetApplicationList() { +AppShortcutLauncherItemController::GetApplicationList(int event_flags) { ChromeLauncherAppMenuItems items; // Add the application name to the menu. items.push_back(new ChromeLauncherAppMenuItem(GetTitle(), NULL, false)); diff --git a/chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.h b/chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.h index e8ccb0e..ae6178f 100644 --- a/chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.h +++ b/chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.h @@ -43,7 +43,8 @@ class AppShortcutLauncherItemController : public LauncherItemController { virtual void LauncherItemChanged( int model_index, const ash::LauncherItem& old_item) OVERRIDE; - virtual ChromeLauncherAppMenuItems GetApplicationList() OVERRIDE; + virtual ChromeLauncherAppMenuItems GetApplicationList( + int event_flags) OVERRIDE; std::vector<content::WebContents*> GetRunningApplications(); // Get the refocus url pattern, which can be used to identify this application diff --git a/chrome/browser/ui/ash/launcher/browser_launcher_item_controller.cc b/chrome/browser/ui/ash/launcher/browser_launcher_item_controller.cc index 4319725..349bcf4 100644 --- a/chrome/browser/ui/ash/launcher/browser_launcher_item_controller.cc +++ b/chrome/browser/ui/ash/launcher/browser_launcher_item_controller.cc @@ -187,7 +187,7 @@ void BrowserLauncherItemController::LauncherItemChanged( } ChromeLauncherAppMenuItems -BrowserLauncherItemController::GetApplicationList() { +BrowserLauncherItemController::GetApplicationList(int event_flags) { // This will never be called and the entire class will go away. ChromeLauncherAppMenuItems items; return items.Pass(); diff --git a/chrome/browser/ui/ash/launcher/browser_launcher_item_controller.h b/chrome/browser/ui/ash/launcher/browser_launcher_item_controller.h index d3b37b8..2030cdc 100644 --- a/chrome/browser/ui/ash/launcher/browser_launcher_item_controller.h +++ b/chrome/browser/ui/ash/launcher/browser_launcher_item_controller.h @@ -78,7 +78,8 @@ class BrowserLauncherItemController : public LauncherItemController, virtual void OnRemoved() OVERRIDE; virtual void LauncherItemChanged(int index, const ash::LauncherItem& old_item) OVERRIDE; - virtual ChromeLauncherAppMenuItems GetApplicationList() OVERRIDE; + virtual ChromeLauncherAppMenuItems GetApplicationList( + int event_flags) OVERRIDE; // TabStripModel overrides: virtual void ActiveTabChanged(content::WebContents* old_contents, diff --git a/chrome/browser/ui/ash/launcher/browser_launcher_item_controller_unittest.cc b/chrome/browser/ui/ash/launcher/browser_launcher_item_controller_unittest.cc index 3adacc6..30f0869 100644 --- a/chrome/browser/ui/ash/launcher/browser_launcher_item_controller_unittest.cc +++ b/chrome/browser/ui/ash/launcher/browser_launcher_item_controller_unittest.cc @@ -37,8 +37,6 @@ namespace { -const int kExpectedAppIndex = 1; - // Test implementation of AppTabHelper. class AppTabHelperImpl : public ChromeLauncherController::AppTabHelper { public: @@ -128,17 +126,18 @@ class TabHelperTabStripModelDelegate : public TestTabStripModelDelegate { } // namespace -class BrowserLauncherItemControllerTest +// TODO(skuhne): Several of these unit tests need to be moved into a new home +// when the old launcher & the browser launcher item controller are removed +// (several of these tests are not testing the BrowserLauncherItemController - +// but the LauncherController framework). +class LauncherItemControllerPerAppTest : public ChromeRenderViewHostTestHarness { public: - BrowserLauncherItemControllerTest() + LauncherItemControllerPerAppTest() : browser_thread_(content::BrowserThread::UI, &message_loop_) { } virtual void SetUp() OVERRIDE { - CommandLine::ForCurrentProcess()->AppendSwitch( - ash::switches::kAshDisablePerAppLauncher); - ChromeRenderViewHostTestHarness::SetUp(); activation_client_.reset( @@ -164,7 +163,7 @@ class BrowserLauncherItemControllerTest struct State : public aura::client::ActivationDelegate, public aura::client::ActivationChangeObserver { public: - State(BrowserLauncherItemControllerTest* test, + State(LauncherItemControllerPerAppTest* test, const std::string& app_id, BrowserLauncherItemController::Type launcher_type) : launcher_test(test), @@ -202,7 +201,7 @@ class BrowserLauncherItemControllerTest updater.BrowserActivationStateChanged(); } - BrowserLauncherItemControllerTest* launcher_test; + LauncherItemControllerPerAppTest* launcher_test; aura::Window window; TabHelperTabStripModelDelegate tab_strip_delegate; TabStripModel tab_strip; @@ -246,6 +245,80 @@ class BrowserLauncherItemControllerTest private: content::TestBrowserThread browser_thread_; + DISALLOW_COPY_AND_ASSIGN(LauncherItemControllerPerAppTest); +}; + +// Verify that the launcher item positions are persisted and restored. +TEST_F(LauncherItemControllerPerAppTest, PersistLauncherItemPositions) { + EXPECT_EQ(ash::TYPE_BROWSER_SHORTCUT, + launcher_model_->items()[0].type); + EXPECT_EQ(ash::TYPE_APP_LIST, + launcher_model_->items()[1].type); + scoped_ptr<content::WebContents> tab1(CreateTestWebContents()); + scoped_ptr<content::WebContents> tab2(CreateTestWebContents()); + app_tab_helper_->SetAppID(tab1.get(), "1"); + app_tab_helper_->SetAppID(tab1.get(), "2"); + + launcher_delegate_->PinAppWithID("1"); + launcher_delegate_->PinAppWithID("2"); + + EXPECT_EQ(ash::TYPE_BROWSER_SHORTCUT, + launcher_model_->items()[0].type); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, + launcher_model_->items()[1].type); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, + launcher_model_->items()[2].type); + EXPECT_EQ(ash::TYPE_APP_LIST, + launcher_model_->items()[3].type); + + launcher_model_->Move(0, 2); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, + launcher_model_->items()[0].type); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, + launcher_model_->items()[1].type); + EXPECT_EQ(ash::TYPE_BROWSER_SHORTCUT, + launcher_model_->items()[2].type); + EXPECT_EQ(ash::TYPE_APP_LIST, + launcher_model_->items()[3].type); + + launcher_delegate_.reset(); + launcher_model_.reset(new ash::LauncherModel); + launcher_delegate_.reset( + ChromeLauncherController::CreateInstance(profile(), + launcher_model_.get())); + app_tab_helper_ = new AppTabHelperImpl; + app_tab_helper_->SetAppID(tab1.get(), "1"); + app_tab_helper_->SetAppID(tab2.get(), "2"); + ResetAppTabHelper(); + + launcher_delegate_->Init(); + + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, + launcher_model_->items()[0].type); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, + launcher_model_->items()[1].type); + EXPECT_EQ(ash::TYPE_BROWSER_SHORTCUT, + launcher_model_->items()[2].type); + EXPECT_EQ(ash::TYPE_APP_LIST, + launcher_model_->items()[3].type); +} + +class BrowserLauncherItemControllerTest + : public LauncherItemControllerPerAppTest { + public: + BrowserLauncherItemControllerTest() {} + + virtual void SetUp() OVERRIDE { + CommandLine::ForCurrentProcess()->AppendSwitch( + ash::switches::kAshDisablePerAppLauncher); + + LauncherItemControllerPerAppTest::SetUp(); + } + + virtual void TearDown() OVERRIDE { + LauncherItemControllerPerAppTest::TearDown(); + } + DISALLOW_COPY_AND_ASSIGN(BrowserLauncherItemControllerTest); }; @@ -294,19 +367,23 @@ TEST_F(BrowserLauncherItemControllerTest, TabbedSetup) { // Verifies pinned apps are persisted and restored. TEST_F(BrowserLauncherItemControllerTest, PersistPinned) { size_t initial_size = launcher_model_->items().size(); - scoped_ptr<content::WebContents> tab1(CreateTestWebContents()); + scoped_ptr<content::WebContents> tab1(CreateTestWebContents()); app_tab_helper_->SetAppID(tab1.get(), "1"); app_icon_loader_->GetAndClearFetchCount(); launcher_delegate_->PinAppWithID("1"); + ash::LauncherID id = launcher_delegate_->GetLauncherIDForAppID("1"); + int app_index = launcher_model_->ItemIndexByID(id); EXPECT_GT(app_icon_loader_->GetAndClearFetchCount(), 0); EXPECT_EQ(ash::TYPE_APP_SHORTCUT, - launcher_model_->items()[kExpectedAppIndex].type); + launcher_model_->items()[app_index].type); EXPECT_TRUE(launcher_delegate_->IsAppPinned("1")); EXPECT_FALSE(launcher_delegate_->IsAppPinned("0")); EXPECT_EQ(initial_size + 1, launcher_model_->items().size()); + launcher_delegate_.reset(); + launcher_model_.reset(new ash::LauncherModel); launcher_delegate_.reset( ChromeLauncherController::CreateInstance(profile(), launcher_model_.get())); @@ -321,12 +398,73 @@ TEST_F(BrowserLauncherItemControllerTest, PersistPinned) { EXPECT_TRUE(launcher_delegate_->IsAppPinned("1")); EXPECT_FALSE(launcher_delegate_->IsAppPinned("0")); EXPECT_EQ(ash::TYPE_APP_SHORTCUT, - launcher_model_->items()[kExpectedAppIndex].type); + launcher_model_->items()[app_index].type); UnpinAppsWithID("1"); ASSERT_EQ(initial_size, launcher_model_->items().size()); } +// Verify that launcher item positions are persisted and restored. +TEST_F(BrowserLauncherItemControllerTest, + PersistLauncherItemPositionsPerBrowser) { + int browser_shortcut_index = 0; + int app_list_index = 1; + + EXPECT_EQ(ash::TYPE_BROWSER_SHORTCUT, + launcher_model_->items()[browser_shortcut_index].type); + EXPECT_EQ(ash::TYPE_APP_LIST, + launcher_model_->items()[app_list_index].type); + + scoped_ptr<content::WebContents> tab1(CreateTestWebContents()); + scoped_ptr<content::WebContents> tab2(CreateTestWebContents()); + + app_tab_helper_->SetAppID(tab1.get(), "1"); + app_tab_helper_->SetAppID(tab2.get(), "2"); + + app_icon_loader_->GetAndClearFetchCount(); + launcher_delegate_->PinAppWithID("1"); + ash::LauncherID id = launcher_delegate_->GetLauncherIDForAppID("1"); + int app1_index = launcher_model_->ItemIndexByID(id); + + launcher_delegate_->PinAppWithID("2"); + id = launcher_delegate_->GetLauncherIDForAppID("2"); + int app2_index = launcher_model_->ItemIndexByID(id); + + launcher_model_->Move(browser_shortcut_index, app1_index); + + browser_shortcut_index = 1; + app1_index = 0; + + EXPECT_GT(app_icon_loader_->GetAndClearFetchCount(), 0); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, + launcher_model_->items()[app1_index].type); + EXPECT_EQ(ash::TYPE_BROWSER_SHORTCUT, + launcher_model_->items()[browser_shortcut_index].type); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, + launcher_model_->items()[app2_index].type); + + launcher_delegate_.reset(); + launcher_model_.reset(new ash::LauncherModel); + launcher_delegate_.reset( + ChromeLauncherController::CreateInstance(profile(), + launcher_model_.get())); + + app_tab_helper_ = new AppTabHelperImpl; + app_tab_helper_->SetAppID(tab1.get(), "1"); + app_tab_helper_->SetAppID(tab2.get(), "2"); + ResetAppTabHelper(); + app_icon_loader_ = new AppIconLoaderImpl; + ResetAppIconLoader(); + launcher_delegate_->Init(); + + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, + launcher_model_->items()[app1_index].type); + EXPECT_EQ(ash::TYPE_BROWSER_SHORTCUT, + launcher_model_->items()[browser_shortcut_index].type); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, + launcher_model_->items()[app2_index].type); +} + // Confirm that tabbed browsers handle activation correctly. TEST_F(BrowserLauncherItemControllerTest, ActivateBrowsers) { State state1(this, std::string(), BrowserLauncherItemController::TYPE_TABBED); diff --git a/chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.cc b/chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.cc new file mode 100644 index 0000000..f699b37 --- /dev/null +++ b/chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.cc @@ -0,0 +1,257 @@ +// Copyright (c) 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.h" + +#include <vector> + +#include "ash/launcher/launcher.h" +#include "ash/shell.h" +#include "ash/wm/window_util.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/browser/ui/ash/launcher/chrome_launcher_app_menu_item.h" +#include "chrome/browser/ui/ash/launcher/chrome_launcher_app_menu_item_browser.h" +#include "chrome/browser/ui/ash/launcher/chrome_launcher_app_menu_item_tab.h" +#include "chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/browser/ui/browser_finder.h" +#include "chrome/browser/ui/browser_list.h" +#include "chrome/browser/ui/browser_window.h" +#include "chrome/browser/ui/tabs/tab_strip_model.h" +#include "chrome/common/extensions/extension_constants.h" +#include "content/public/browser/web_contents.h" +#include "grit/ash_resources.h" +#include "grit/chromium_strings.h" +#include "grit/generated_resources.h" +#include "ui/aura/window.h" +#include "ui/base/events/event.h" +#include "ui/base/l10n/l10n_util.h" +#include "ui/base/resource/resource_bundle.h" +#include "ui/gfx/image/image.h" +#include "ui/views/corewm/window_animations.h" + +#if defined(OS_CHROMEOS) +#include "chrome/browser/chromeos/login/default_pinned_apps_field_trial.h" +#endif + +BrowserShortcutLauncherItemController::BrowserShortcutLauncherItemController( + ChromeLauncherControllerPerApp* launcher_controller, + Profile* profile) + : LauncherItemController(TYPE_SHORTCUT, + extension_misc::kChromeAppId, + launcher_controller), + app_controller_(launcher_controller), + profile_(profile) { +} + +string16 BrowserShortcutLauncherItemController::GetTitle() { + return l10n_util::GetStringUTF16(IDS_PRODUCT_NAME); +} + +bool BrowserShortcutLauncherItemController::HasWindow( + aura::Window* window) const { + const BrowserList* ash_browser_list = + BrowserList::GetInstance(chrome::HOST_DESKTOP_TYPE_ASH); + for (BrowserList::const_iterator it = ash_browser_list->begin(); + it != ash_browser_list->end(); ++it) { + if ((*it)->window()->GetNativeWindow() == window) + return true; + } + return false; +} + +bool BrowserShortcutLauncherItemController::IsOpen() const { + const BrowserList* ash_browser_list = + BrowserList::GetInstance(chrome::HOST_DESKTOP_TYPE_ASH); + return ash_browser_list->empty() ? false : true; +} + +bool BrowserShortcutLauncherItemController::IsVisible() const { + Browser* last_browser = chrome::FindTabbedBrowser( + profile_, + true, + chrome::HOST_DESKTOP_TYPE_ASH); + + if (!last_browser) { + return false; + } + + aura::Window* window = last_browser->window()->GetNativeWindow(); + return ash::wm::IsActiveWindow(window); +} + +void BrowserShortcutLauncherItemController::Launch(int event_flags) { +} + +void BrowserShortcutLauncherItemController::Activate() { + Browser* last_browser = chrome::FindTabbedBrowser( + profile_, + true, + chrome::HOST_DESKTOP_TYPE_ASH); + + if (!last_browser) { + launcher_controller()->CreateNewWindow(); + return; + } + + launcher_controller()->ActivateWindowOrMinimizeIfActive( + last_browser->window(), GetApplicationList(0).size() == 2); +} + +void BrowserShortcutLauncherItemController::Close() { +} + +void BrowserShortcutLauncherItemController::LauncherItemChanged( + int model_index, + const ash::LauncherItem& old_item) { +} + +void BrowserShortcutLauncherItemController::Clicked(const ui::Event& event) { + #if defined(OS_CHROMEOS) + chromeos::default_pinned_apps_field_trial::RecordShelfClick( + chromeos::default_pinned_apps_field_trial::CHROME); + #endif + + if (event.flags() & ui::EF_CONTROL_DOWN) { + launcher_controller()->CreateNewWindow(); + return; + } + + // In case of a keyboard event, we were called by a hotkey. In that case we + // activate the next item in line if an item of our list is already active. + if (event.type() & ui::ET_KEY_RELEASED) { + ActivateOrAdvanceToNextBrowser(); + return; + } + + Activate(); +} + +void BrowserShortcutLauncherItemController::OnRemoved() { + // BrowserShortcutLauncherItemController is owned by ChromeLauncherController. +} + +ChromeLauncherAppMenuItems +BrowserShortcutLauncherItemController::GetApplicationList(int event_flags) { + ChromeLauncherAppMenuItems items; + bool found_tabbed_browser = false; + // Add the application name to the menu. + items.push_back(new ChromeLauncherAppMenuItem(GetTitle(), NULL, false)); + const BrowserList* ash_browser_list = + BrowserList::GetInstance(chrome::HOST_DESKTOP_TYPE_ASH); + for (BrowserList::const_iterator it = ash_browser_list->begin(); + it != ash_browser_list->end(); ++it) { + Browser* browser = *it; + // Make sure that the browser was already shown and it has a proper window. + if (std::find(ash_browser_list->begin_last_active(), + ash_browser_list->end_last_active(), + browser) == ash_browser_list->end_last_active() || + !browser->window()) + continue; + if (browser->is_type_tabbed()) + found_tabbed_browser = true; + else if (!app_controller_->IsBrowserRepresentedInBrowserList(browser)) + continue; + TabStripModel* tab_strip = browser->tab_strip_model(); + if (tab_strip->active_index() == -1) + continue; + if (!(event_flags & ui::EF_SHIFT_DOWN)) { + content::WebContents* web_contents = + tab_strip->GetWebContentsAt(tab_strip->active_index()); + gfx::Image app_icon = GetBrowserListIcon(web_contents); + string16 title = GetBrowserListTitle(web_contents); + items.push_back(new ChromeLauncherAppMenuItemBrowser( + title, &app_icon, browser, items.size() == 1)); + } else { + for (int index = 0; index < tab_strip->count(); ++index) { + content::WebContents* web_contents = + tab_strip->GetWebContentsAt(index); + gfx::Image app_icon = app_controller_->GetAppListIcon(web_contents); + string16 title = app_controller_->GetAppListTitle(web_contents); + // Check if we need to insert a separator in front. + bool leading_separator = !index; + items.push_back(new ChromeLauncherAppMenuItemTab( + title, &app_icon, web_contents, leading_separator)); + } + } + } + // If only windowed applications are open, we return an empty list to + // enforce the creation of a new browser. + if (!found_tabbed_browser) + items.clear(); + return items.Pass(); +} + +gfx::Image BrowserShortcutLauncherItemController::GetBrowserListIcon( + content::WebContents* web_contents) const { + ResourceBundle& rb = ResourceBundle::GetSharedInstance(); + return rb.GetImageNamed(IsIncognito(web_contents) ? + IDR_AURA_LAUNCHER_LIST_INCOGNITO_BROWSER : + IDR_AURA_LAUNCHER_LIST_BROWSER); +} + +string16 BrowserShortcutLauncherItemController::GetBrowserListTitle( + content::WebContents* web_contents) const { + string16 title = web_contents->GetTitle(); + if (!title.empty()) + return title; + return l10n_util::GetStringUTF16(IDS_NEW_TAB_TITLE); +} + +bool BrowserShortcutLauncherItemController::IsIncognito( + content::WebContents* web_contents) const { + const Profile* profile = + Profile::FromBrowserContext(web_contents->GetBrowserContext()); + return profile->IsOffTheRecord() && !profile->IsGuestSession(); +} + +void BrowserShortcutLauncherItemController::ActivateOrAdvanceToNextBrowser() { + // Create a list of all suitable running browsers. + std::vector<Browser*> items; + // We use the list in the order of how the browsers got created - not the LRU + // order. + const BrowserList* ash_browser_list = + BrowserList::GetInstance(chrome::HOST_DESKTOP_TYPE_ASH); + for (BrowserList::const_iterator it = + ash_browser_list->begin(); + it != ash_browser_list->end(); ++it) { + if (app_controller_->IsBrowserRepresentedInBrowserList(*it)) + items.push_back(*it); + } + // If there are no suitable browsers we create a new one. + if (!items.size()) { + launcher_controller()->CreateNewWindow(); + return; + } + Browser* browser = chrome::FindBrowserWithWindow(ash::wm::GetActiveWindow()); + if (items.size() == 1) { + // If there is only one suitable browser, we can either activate it, or + // bounce it (if it is already active). + if (browser == items[0]) { + AnimateWindow(browser->window()->GetNativeWindow(), + views::corewm::WINDOW_ANIMATION_TYPE_BOUNCE); + return; + } + browser = items[0]; + } else { + // If there is more then one suitable browser, we advance to the next if + // |current_browser| is already active - or - check the last used browser + // if it can be used. + std::vector<Browser*>::iterator i = + std::find(items.begin(), items.end(), browser); + if (i != items.end()) { + browser = (++i == items.end()) ? items[0] : *i; + } else { + browser = chrome::FindTabbedBrowser(profile_, + true, + chrome::HOST_DESKTOP_TYPE_ASH); + if (!browser || + !app_controller_->IsBrowserRepresentedInBrowserList(browser)) + browser = items[0]; + } + } + DCHECK(browser); + browser->window()->Show(); + browser->window()->Activate(); +} diff --git a/chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.h b/chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.h new file mode 100644 index 0000000..7d30b8e --- /dev/null +++ b/chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.h @@ -0,0 +1,68 @@ +// Copyright (c) 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_UI_ASH_LAUNCHER_BROWSER_SHORTCUT_LAUNCHER_ITEM_CONTROLLER_H_ +#define CHROME_BROWSER_UI_ASH_LAUNCHER_BROWSER_SHORTCUT_LAUNCHER_ITEM_CONTROLLER_H_ + +#include "chrome/browser/ui/ash/launcher/launcher_item_controller.h" + +namespace content { +class WebContents; +} + +namespace gfx { +class Image; +} + +class Browser; +class ChromeLauncherControllerPerApp; +class Profile; + +// Item controller for an browser shortcut. +class BrowserShortcutLauncherItemController : public LauncherItemController { + public: + BrowserShortcutLauncherItemController( + ChromeLauncherControllerPerApp* controller, Profile* profile); + + virtual ~BrowserShortcutLauncherItemController() {} + + // LauncherItemController overrides: + virtual string16 GetTitle() OVERRIDE; + virtual bool HasWindow(aura::Window* window) const OVERRIDE; + virtual bool IsOpen() const OVERRIDE; + virtual bool IsVisible() const OVERRIDE; + virtual void Launch(int event_flags) OVERRIDE; + virtual void Activate() OVERRIDE; + virtual void Close() OVERRIDE; + virtual void LauncherItemChanged( + int model_index, + const ash::LauncherItem& old_item) OVERRIDE; + virtual void Clicked(const ui::Event& event) OVERRIDE; + virtual void OnRemoved() OVERRIDE; + virtual ChromeLauncherAppMenuItems GetApplicationList( + int event_flags) OVERRIDE; + + private: + // Get the favicon for the browser list entry for |web_contents|. + // Note that for incognito windows the incognito icon will be returned. + gfx::Image GetBrowserListIcon(content::WebContents* web_contents) const; + + // Get the title for the browser list entry for |web_contents|. + // If |web_contents| has not loaded, returns "Net Tab". + string16 GetBrowserListTitle(content::WebContents* web_contents) const; + + // Check if the given |web_contents| is in incognito mode. + bool IsIncognito(content::WebContents* web_contents) const; + + // Activate a browser - or advance to the next one on the list. + void ActivateOrAdvanceToNextBrowser(); + + ChromeLauncherControllerPerApp* app_controller_; + + Profile* profile_; + + DISALLOW_COPY_AND_ASSIGN(BrowserShortcutLauncherItemController); +}; + +#endif // CHROME_BROWSER_UI_ASH_LAUNCHER_BROWSER_SHORTCUT_LAUNCHER_ITEM_CONTROLLER_H_ diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller.h b/chrome/browser/ui/ash/launcher/chrome_launcher_controller.h index 59571f0..7083fbe 100644 --- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller.h +++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller.h @@ -15,7 +15,7 @@ #include "chrome/browser/extensions/extension_prefs.h" class BaseWindow; -class BrowserLauncherItemControllerTest; +class LauncherItemControllerPerAppTest; class LauncherItemController; class Profile; class ChromeLauncherAppMenuItem; @@ -262,10 +262,8 @@ class ChromeLauncherController virtual void ActivateWindowOrMinimizeIfActive(BaseWindow* window, bool allow_minimize) = 0; // ash::LauncherDelegate overrides: - virtual void OnBrowserShortcutClicked(int event_flags) OVERRIDE = 0; virtual void ItemSelected(const ash::LauncherItem& item, const ui::Event& event) OVERRIDE = 0; - virtual int GetBrowserShortcutResourceId() OVERRIDE = 0; virtual string16 GetTitle(const ash::LauncherItem& item) OVERRIDE = 0; virtual ui::MenuModel* CreateContextMenu( const ash::LauncherItem& item, aura::RootWindow* root) OVERRIDE = 0; @@ -282,7 +280,7 @@ class ChromeLauncherController const gfx::ImageSkia& image) OVERRIDE = 0; protected: - friend class BrowserLauncherItemControllerTest; + friend class LauncherItemControllerPerAppTest; friend class LauncherPlatformAppBrowserTest; friend class LauncherAppBrowserTest; // TODO(skuhne): Remove these when the old launcher get removed. diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc index 22e9294..ceecd5e 100644 --- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc +++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc @@ -33,6 +33,7 @@ #include "chrome/browser/ui/ash/app_sync_ui_state.h" #include "chrome/browser/ui/ash/chrome_launcher_prefs.h" #include "chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.h" +#include "chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.h" #include "chrome/browser/ui/ash/launcher/chrome_launcher_app_menu_item.h" #include "chrome/browser/ui/ash/launcher/chrome_launcher_app_menu_item_browser.h" #include "chrome/browser/ui/ash/launcher/chrome_launcher_app_menu_item_tab.h" @@ -261,7 +262,8 @@ ChromeLauncherControllerPerApp::~ChromeLauncherControllerPerApp() { int index = model_->ItemIndexByID(i->first); // A "browser proxy" is not known to the model and this removal does // therefore not need to be propagated to the model. - if (index != -1) + if (index != -1 && + model_->items()[index].type != ash::TYPE_BROWSER_SHORTCUT) model_->RemoveItemAt(index); } @@ -276,6 +278,7 @@ ChromeLauncherControllerPerApp::~ChromeLauncherControllerPerApp() { void ChromeLauncherControllerPerApp::Init() { UpdateAppLaunchersFromPref(); + CreateBrowserShortcutLauncherItem(); // TODO(sky): update unit test so that this test isn't necessary. if (ash::Shell::HasInstance()) { @@ -425,7 +428,7 @@ bool ChromeLauncherControllerPerApp::IsPinned(ash::LauncherID id) { if (index < 0) return false; ash::LauncherItemType type = model_->items()[index].type; - return type == ash::TYPE_APP_SHORTCUT; + return (type == ash::TYPE_APP_SHORTCUT || type == ash::TYPE_BROWSER_SHORTCUT); } void ChromeLauncherControllerPerApp::TogglePinned(ash::LauncherID id) { @@ -530,11 +533,6 @@ void ChromeLauncherControllerPerApp::LaunchApp(const std::string& app_id, void ChromeLauncherControllerPerApp::ActivateApp(const std::string& app_id, int event_flags) { - if (app_id == extension_misc::kChromeAppId) { - OnBrowserShortcutClicked(event_flags); - return; - } - // If there is an existing non-shortcut controller for this app, open it. ash::LauncherID id = GetLauncherIDForAppID(app_id); if (id) { @@ -723,6 +721,8 @@ void ChromeLauncherControllerPerApp::PersistPinnedState() { if (app_value) updater->Append(app_value); } + } else if (model_->items()[i].type == ash::TYPE_BROWSER_SHORTCUT) { + PersistChromeItemIndex(i); } } } @@ -879,7 +879,9 @@ void ChromeLauncherControllerPerApp::SetRefocusURLPatternForTest( const Extension* ChromeLauncherControllerPerApp::GetExtensionForAppID( const std::string& app_id) const { - return profile_->GetExtensionService()->GetInstalledExtension(app_id); + // Some unit tests do not have a real extension. + return (profile_->GetExtensionService()) ? + profile_->GetExtensionService()->GetInstalledExtension(app_id) : NULL; } void ChromeLauncherControllerPerApp::ActivateWindowOrMinimizeIfActive( @@ -899,37 +901,8 @@ void ChromeLauncherControllerPerApp::ActivateWindowOrMinimizeIfActive( } } -void ChromeLauncherControllerPerApp::OnBrowserShortcutClicked( - int event_flags) { -#if defined(OS_CHROMEOS) - chromeos::default_pinned_apps_field_trial::RecordShelfClick( - chromeos::default_pinned_apps_field_trial::CHROME); -#endif - if (event_flags & ui::EF_CONTROL_DOWN) { - CreateNewWindow(); - return; - } - - Browser* last_browser = chrome::FindTabbedBrowser( - GetProfileForNewWindows(), true, chrome::HOST_DESKTOP_TYPE_ASH); - - if (!last_browser) { - CreateNewWindow(); - return; - } - - ActivateWindowOrMinimizeIfActive(last_browser->window(), - GetBrowserApplicationList(0).size() == 2); -} - void ChromeLauncherControllerPerApp::ItemSelected(const ash::LauncherItem& item, const ui::Event& event) { - // TODO(skuhne): Remove this temporary fix once M28 is out and CL 11596003 - // has landed. - if (item.id == ash::kAppIdForBrowserSwitching) { - ActivateOrAdvanceToNextBrowser(); - return; - } DCHECK(HasItemController(item.id)); LauncherItemController* item_controller = id_to_item_controller_map_[item.id]; #if defined(OS_CHROMEOS) @@ -941,10 +914,6 @@ void ChromeLauncherControllerPerApp::ItemSelected(const ash::LauncherItem& item, item_controller->Clicked(event); } -int ChromeLauncherControllerPerApp::GetBrowserShortcutResourceId() { - return IDR_PRODUCT_LOGO_32; -} - string16 ChromeLauncherControllerPerApp::GetTitle( const ash::LauncherItem& item) { DCHECK(HasItemController(item.id)); @@ -1038,14 +1007,6 @@ void ChromeLauncherControllerPerApp::LauncherItemChanged( int index, const ash::LauncherItem& old_item) { ash::LauncherID id = model_->items()[index].id; - - // The browser item does not have a controller since it was created by the - // |LauncherModel|. - // TODO(skuhne): When removing the old launcher, this should get changed as - // well. - if (model_->items()[index].type == ash::TYPE_BROWSER_SHORTCUT) - return; - DCHECK(HasItemController(id)); id_to_item_controller_map_[id]->LauncherItemChanged(index, old_item); } @@ -1153,16 +1114,13 @@ void ChromeLauncherControllerPerApp::ExtensionEnableFlowAborted( ChromeLauncherAppMenuItems ChromeLauncherControllerPerApp::GetApplicationList( const ash::LauncherItem& item, int event_flags) { - if (item.type == ash::TYPE_BROWSER_SHORTCUT) - return GetBrowserApplicationList(event_flags); - // Make sure that there is a controller associated with the id and that the // extension itself is a valid application and not a panel. if (!HasItemController(item.id) || !GetLauncherIDForAppID(id_to_item_controller_map_[item.id]->app_id())) return ChromeLauncherAppMenuItems().Pass(); - return id_to_item_controller_map_[item.id]->GetApplicationList(); + return id_to_item_controller_map_[item.id]->GetApplicationList(event_flags); } std::vector<content::WebContents*> @@ -1249,22 +1207,6 @@ string16 ChromeLauncherControllerPerApp::GetAppListTitle( return l10n_util::GetStringUTF16(IDS_NEW_TAB_TITLE); } -gfx::Image ChromeLauncherControllerPerApp::GetBrowserListIcon( - content::WebContents* web_contents) const { - ResourceBundle& rb = ResourceBundle::GetSharedInstance(); - return rb.GetImageNamed(IsIncognito(web_contents) ? - IDR_AURA_LAUNCHER_LIST_INCOGNITO_BROWSER : - IDR_AURA_LAUNCHER_LIST_BROWSER); -} - -string16 ChromeLauncherControllerPerApp::GetBrowserListTitle( - content::WebContents* web_contents) const { - string16 title = web_contents->GetTitle(); - if (!title.empty()) - return title; - return l10n_util::GetStringUTF16(IDS_NEW_TAB_TITLE); -} - void ChromeLauncherControllerPerApp::OnBrowserRemoved(Browser* browser) { // When called by a unit test it is possible that there is no shell. // In that case, the following function should not get called. @@ -1596,59 +1538,6 @@ ChromeLauncherControllerPerApp::GetV1ApplicationsFromController( return app_controller->GetRunningApplications(); } -ChromeLauncherAppMenuItems -ChromeLauncherControllerPerApp::GetBrowserApplicationList( - int event_flags) { - ChromeLauncherAppMenuItems items; - bool found_tabbed_browser = false; - // Add the application name to the menu. - items.push_back(new ChromeLauncherAppMenuItem( - l10n_util::GetStringUTF16(IDS_PRODUCT_NAME), NULL, false)); - const BrowserList* ash_browser_list = - BrowserList::GetInstance(chrome::HOST_DESKTOP_TYPE_ASH); - for (BrowserList::const_iterator it = ash_browser_list->begin(); - it != ash_browser_list->end(); ++it) { - Browser* browser = *it; - // Make sure that the browser was already shown and it has a proper window. - if (std::find(ash_browser_list->begin_last_active(), - ash_browser_list->end_last_active(), - browser) == ash_browser_list->end_last_active() || - !browser->window()) - continue; - if (browser->is_type_tabbed()) - found_tabbed_browser = true; - else if (!IsBrowserRepresentedInBrowserList(browser)) - continue; - TabStripModel* tab_strip = browser->tab_strip_model(); - if (tab_strip->active_index() == -1) - continue; - if (!(event_flags & ui::EF_SHIFT_DOWN)) { - WebContents* web_contents = - tab_strip->GetWebContentsAt(tab_strip->active_index()); - gfx::Image app_icon = GetBrowserListIcon(web_contents); - string16 title = GetBrowserListTitle(web_contents); - items.push_back(new ChromeLauncherAppMenuItemBrowser( - title, &app_icon, browser, items.size() == 1)); - } else { - for (int index = 0; index < tab_strip->count(); ++index) { - content::WebContents* web_contents = - tab_strip->GetWebContentsAt(index); - gfx::Image app_icon = GetAppListIcon(web_contents); - string16 title = GetAppListTitle(web_contents); - // Check if we need to insert a separator in front. - bool leading_separator = !index; - items.push_back(new ChromeLauncherAppMenuItemTab( - title, &app_icon, web_contents, leading_separator)); - } - } - } - // If only windowed applications are open, we return an empty list to - // enforce the creation of a new browser. - if (!found_tabbed_browser) - items.clear(); - return items.Pass(); -} - bool ChromeLauncherControllerPerApp::IsBrowserRepresentedInBrowserList( Browser* browser) { return (browser && @@ -1659,57 +1548,54 @@ bool ChromeLauncherControllerPerApp::IsBrowserRepresentedInBrowserList( browser->app_name())) <= 0)); } +LauncherItemController* +ChromeLauncherControllerPerApp::GetBrowserShortcutLauncherItemController() { + for (IDToItemControllerMap::iterator i = id_to_item_controller_map_.begin(); + i != id_to_item_controller_map_.end(); ++i) { + int index = model_->ItemIndexByID(i->first); + const ash::LauncherItem& item = model_->items()[index]; + if (item.type == ash::TYPE_BROWSER_SHORTCUT) + return i->second; + } + // LauncerItemController For Browser Shortcut must be existed. If it does not + // existe create it. + ash::LauncherID id = CreateBrowserShortcutLauncherItem(); + DCHECK(id_to_item_controller_map_[id]); + return id_to_item_controller_map_[id]; +} + +ash::LauncherID +ChromeLauncherControllerPerApp::CreateBrowserShortcutLauncherItem() { + ash::LauncherItem browser_shortcut; + browser_shortcut.type = ash::TYPE_BROWSER_SHORTCUT; + browser_shortcut.is_incognito = false; + ResourceBundle& rb = ResourceBundle::GetSharedInstance(); + browser_shortcut.image = *rb.GetImageSkiaNamed(IDR_PRODUCT_LOGO_32); + ash::LauncherID id = model_->next_id(); + size_t index = GetChromeIconIndexFromPref(); + model_->AddAt(index, browser_shortcut); + browser_item_controller_.reset( + new BrowserShortcutLauncherItemController(this, profile_)); + id_to_item_controller_map_[id] = browser_item_controller_.get(); + id_to_item_controller_map_[id]->set_launcher_id(id); + return id; +} + +void ChromeLauncherControllerPerApp::PersistChromeItemIndex(int index) { + profile_->GetPrefs()->SetInteger(prefs::kShelfChromeIconIndex, index); +} + +int ChromeLauncherControllerPerApp::GetChromeIconIndexFromPref() const { + size_t index = profile_->GetPrefs()->GetInteger(prefs::kShelfChromeIconIndex); + const base::ListValue* pinned_apps_pref = + profile_->GetPrefs()->GetList(prefs::kPinnedLauncherApps); + return std::max(static_cast<size_t>(0), + std::min(pinned_apps_pref->GetSize(), index)); +} + bool ChromeLauncherControllerPerApp::IsIncognito( content::WebContents* web_contents) const { const Profile* profile = Profile::FromBrowserContext(web_contents->GetBrowserContext()); return profile->IsOffTheRecord() && !profile->IsGuestSession(); } - -void ChromeLauncherControllerPerApp::ActivateOrAdvanceToNextBrowser() { - // Create a list of all suitable running browsers. - std::vector<Browser*> items; - // We use the list in the order of how the browsers got created - not the LRU - // order. - const BrowserList* ash_browser_list = - BrowserList::GetInstance(chrome::HOST_DESKTOP_TYPE_ASH); - for (BrowserList::const_iterator it = - ash_browser_list->begin(); - it != ash_browser_list->end(); ++it) { - if (IsBrowserRepresentedInBrowserList(*it)) - items.push_back(*it); - } - // If there are no suitable browsers we create a new one. - if (!items.size()) { - CreateNewWindow(); - return; - } - Browser* browser = chrome::FindBrowserWithWindow(ash::wm::GetActiveWindow()); - if (items.size() == 1) { - // If there is only one suitable browser, we can either activate it, or - // bounce it (if it is already active). - if (browser == items[0]) { - AnimateWindow(browser->window()->GetNativeWindow(), - views::corewm::WINDOW_ANIMATION_TYPE_BOUNCE); - return; - } - browser = items[0]; - } else { - // If there is more then one suitable browser, we advance to the next if - // |current_browser| is already active - or - check the last used browser - // if it can be used. - std::vector<Browser*>::iterator i = - std::find(items.begin(), items.end(), browser); - if (i != items.end()) { - browser = (++i == items.end()) ? items[0] : *i; - } else { - browser = chrome::FindTabbedBrowser( - GetProfileForNewWindows(), true, chrome::HOST_DESKTOP_TYPE_ASH); - if (!browser || !IsBrowserRepresentedInBrowserList(browser)) - browser = items[0]; - } - } - DCHECK(browser); - browser->window()->Show(); - browser->window()->Activate(); -} diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h index 55979f8..95592c0 100644 --- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h +++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h @@ -38,6 +38,7 @@ class AppSyncUIState; class BaseWindow; class Browser; class BrowserLauncherItemControllerTest; +class BrowserShortcutLauncherItemController; class ExtensionEnableFlow; class LauncherItemController; class Profile; @@ -259,10 +260,8 @@ class ChromeLauncherControllerPerApp bool allow_minimize) OVERRIDE; // ash::LauncherDelegate overrides: - virtual void OnBrowserShortcutClicked(int event_flags) OVERRIDE; virtual void ItemSelected(const ash::LauncherItem& item, const ui::Event& event) OVERRIDE; - virtual int GetBrowserShortcutResourceId() OVERRIDE; virtual string16 GetTitle(const ash::LauncherItem& item) OVERRIDE; virtual ui::MenuModel* CreateContextMenu( const ash::LauncherItem& item, aura::RootWindow* root) OVERRIDE; @@ -344,17 +343,16 @@ class ChromeLauncherControllerPerApp // If |web_contents| has not loaded, returns "Net Tab". string16 GetAppListTitle(content::WebContents* web_contents) const; - // Get the favicon for the browser list entry for |web_contents|. - // Note that for incognito windows the incognito icon will be returned. - gfx::Image GetBrowserListIcon(content::WebContents* web_contents) const; - - // Get the title for the browser list entry for |web_contents|. - // If |web_contents| has not loaded, returns "Net Tab". - string16 GetBrowserListTitle(content::WebContents* web_contents) const; - // Overridden from chrome::BrowserListObserver. virtual void OnBrowserRemoved(Browser* browser) OVERRIDE; + // Returns true when the given |browser| is listed in the browser application + // list. + bool IsBrowserRepresentedInBrowserList(Browser* browser); + + // Returns the LauncherItemController of BrowserShortcut. + LauncherItemController* GetBrowserShortcutLauncherItemController(); + protected: // ChromeLauncherController overrides: @@ -439,22 +437,17 @@ class ChromeLauncherControllerPerApp std::vector<content::WebContents*> GetV1ApplicationsFromController( LauncherItemController* controller); - // Returns the list of all browsers runing. - // |event_flags| specifies the flags which were set by the event which - // triggered this menu generation. It can be used to generate different lists. - // TODO(skuhne): Move to wherever the BrowserLauncherItemController - // functionality moves to. - ChromeLauncherAppMenuItems GetBrowserApplicationList(int event_flags); - - // Returns true when the given |browser| is listed in the browser application - // list. - bool IsBrowserRepresentedInBrowserList(Browser* browser); + // Create LauncherItem for Browser Shortcut. + ash::LauncherID CreateBrowserShortcutLauncherItem(); // Check if the given |web_contents| is in incognito mode. bool IsIncognito(content::WebContents* web_contents) const; - // Activate a browser - or the next one in the list. - void ActivateOrAdvanceToNextBrowser(); + // Update browser shortcut's index. + void PersistChromeItemIndex(int index); + + // Get browser shortcut's index from pref. + int GetChromeIconIndexFromPref() const; ash::LauncherModel* model_; @@ -490,6 +483,9 @@ class ChromeLauncherControllerPerApp // Launchers that are currently being observed. std::set<ash::Launcher*> launchers_; + // The owned browser shortcut item. + scoped_ptr<BrowserShortcutLauncherItemController> browser_item_controller_; + DISALLOW_COPY_AND_ASSIGN(ChromeLauncherControllerPerApp); }; diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_browsertest.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_browsertest.cc index ce640fd..75f1163 100644 --- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_browsertest.cc +++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_browsertest.cc @@ -26,6 +26,7 @@ #include "chrome/browser/extensions/shell_window_registry.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/app_list/app_list_service.h" +#include "chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.h" #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h" #include "chrome/browser/ui/ash/launcher/launcher_item_controller.h" #include "chrome/browser/ui/browser.h" @@ -55,10 +56,6 @@ using content::WebContents; namespace { -// TODO(skuhne): Remove this temporary fix once M28 is out and CL 11596003 -// has landed. See also launcher.cc for the offset introduction. -const int kChromeItemOffset = 1; - class TestEvent : public ui::Event { public: explicit TestEvent(ui::EventType type) @@ -163,7 +160,7 @@ class LauncherPlatformPerAppAppBrowserTest // Activate the launcher item with the given |id|. void ActivateLauncherItem(int id) { - launcher_->ActivateLauncherItem(id + kChromeItemOffset); + launcher_->ActivateLauncherItem(id); } ash::Launcher* launcher_; @@ -202,7 +199,9 @@ class LauncherPerAppAppBrowserTest : public ExtensionBrowserTest { size_t NumberOfDetectedLauncherBrowsers(bool show_all_tabs) { ChromeLauncherControllerPerApp* controller = static_cast<ChromeLauncherControllerPerApp*>(launcher_->delegate()); - int items = controller->GetBrowserApplicationList( + LauncherItemController* item_controller = + controller->GetBrowserShortcutLauncherItemController(); + int items = item_controller->GetApplicationList( show_all_tabs ? ui::EF_SHIFT_DOWN : 0).size(); // If we have at least one item, we have also a title which we remove here. return items ? (items - 1) : 0; @@ -253,7 +252,7 @@ class LauncherPerAppAppBrowserTest : public ExtensionBrowserTest { // Activate the launcher item with the given |id|. void ActivateLauncherItem(int id) { - launcher_->ActivateLauncherItem(id + kChromeItemOffset); + launcher_->ActivateLauncherItem(id); } ash::Launcher* launcher_; @@ -1301,10 +1300,10 @@ IN_PROC_BROWSER_TEST_F(LauncherPerAppAppBrowserTestNoDefaultBrowser, // Get the number of items in the browser menu. EXPECT_EQ(0u, chrome::GetTotalBrowserCount()); // The first activation should create a browser. - launcher_->ActivateLauncherItem(1); + launcher_->ActivateLauncherItem(0); EXPECT_EQ(1u, chrome::GetTotalBrowserCount()); // A second activation should not create a new instance. - launcher_->ActivateLauncherItem(1); + launcher_->ActivateLauncherItem(0); Browser* browser1 = chrome::FindBrowserWithWindow(ash::wm::GetActiveWindow()); EXPECT_TRUE(browser1); aura::Window* window1 = browser1->window()->GetNativeWindow(); @@ -1316,9 +1315,9 @@ IN_PROC_BROWSER_TEST_F(LauncherPerAppAppBrowserTestNoDefaultBrowser, EXPECT_EQ(window2, ash::wm::GetActiveWindow()); // Activate multiple times the switcher to see that the windows get activated. - launcher_->ActivateLauncherItem(1); + launcher_->ActivateLauncherItem(0); EXPECT_EQ(window1, ash::wm::GetActiveWindow()); - launcher_->ActivateLauncherItem(1); + launcher_->ActivateLauncherItem(0); EXPECT_EQ(window2, ash::wm::GetActiveWindow()); // Create a third browser - make sure that we do not toggle simply between @@ -1331,13 +1330,13 @@ IN_PROC_BROWSER_TEST_F(LauncherPerAppAppBrowserTestNoDefaultBrowser, EXPECT_NE(window2, window3); EXPECT_EQ(window3, ash::wm::GetActiveWindow()); - launcher_->ActivateLauncherItem(1); + launcher_->ActivateLauncherItem(0); EXPECT_EQ(window1, ash::wm::GetActiveWindow()); - launcher_->ActivateLauncherItem(1); + launcher_->ActivateLauncherItem(0); EXPECT_EQ(window2, ash::wm::GetActiveWindow()); - launcher_->ActivateLauncherItem(1); + launcher_->ActivateLauncherItem(0); EXPECT_EQ(window3, ash::wm::GetActiveWindow()); - launcher_->ActivateLauncherItem(1); + launcher_->ActivateLauncherItem(0); EXPECT_EQ(window1, ash::wm::GetActiveWindow()); // Create anther app and make sure that none of our browsers is active. @@ -1346,7 +1345,7 @@ IN_PROC_BROWSER_TEST_F(LauncherPerAppAppBrowserTestNoDefaultBrowser, EXPECT_NE(window2, ash::wm::GetActiveWindow()); // After activation our browser should be active again. - launcher_->ActivateLauncherItem(1); + launcher_->ActivateLauncherItem(0); EXPECT_EQ(window1, ash::wm::GetActiveWindow()); } @@ -1408,19 +1407,21 @@ IN_PROC_BROWSER_TEST_F(LauncherPerAppAppBrowserTest, DragAndDrop) { EXPECT_EQ(3, model_->item_count()); EXPECT_TRUE(grid_view->forward_events_to_drag_and_drop_host_for_test()); - // Move it somewhere else and check that it disappears again. - generator.MoveMouseTo(0, 0); + // Move it where the item originally was and check that it disappears again. + generator.MoveMouseTo(bounds_grid_1.CenterPoint().x(), + bounds_grid_1.CenterPoint().y()); MessageLoop::current()->RunUntilIdle(); EXPECT_EQ(2, model_->item_count()); EXPECT_FALSE(grid_view->forward_events_to_drag_and_drop_host_for_test()); - // Dropping it outside of the launcher should keep the launcher as it - // originally was. + // Dropping it should keep the launcher as it originally was. generator.ReleaseLeftButton(); MessageLoop::current()->RunUntilIdle(); EXPECT_EQ(2, model_->item_count()); // There are a few animations which need finishing before we can continue. test.RunMessageLoopUntilAnimationsDone(); + // Move the mouse outside of the launcher. + generator.MoveMouseTo(0, 0); // Test #2: Check that the unknown item dropped into the launcher will // create a new item. @@ -1453,3 +1454,37 @@ IN_PROC_BROWSER_TEST_F(LauncherPerAppAppBrowserTest, DragAndDrop) { EXPECT_FALSE(grid_view->forward_events_to_drag_and_drop_host_for_test()); EXPECT_EQ(3, model_->item_count()); // And it remains that way. } + +// Check LauncherItemController of Browser Shortcut functionality. +IN_PROC_BROWSER_TEST_F(LauncherPerAppAppBrowserTestNoDefaultBrowser, + BrowserShortcutLauncherItemController) { + ChromeLauncherControllerPerApp* controller = + static_cast<ChromeLauncherControllerPerApp*>(launcher_->delegate()); + LauncherItemController* item_controller = + controller->GetBrowserShortcutLauncherItemController(); + + // Get the number of browsers. + size_t running_browser = chrome::GetTotalBrowserCount(); + EXPECT_EQ(0u, running_browser); + EXPECT_FALSE(item_controller->IsOpen()); + + // Activate. This creates new browser + item_controller->Activate(); + // New Window is created. + running_browser = chrome::GetTotalBrowserCount(); + EXPECT_EQ(1u, running_browser); + EXPECT_TRUE(item_controller->IsOpen()); + + // Minimize Window. + aura::Window* window = ash::wm::GetActiveWindow(); + ash::wm::MinimizeWindow(window); + EXPECT_TRUE(ash::wm::IsWindowMinimized(window)); + + // Activate again. This doesn't create new browser. + // It activates window. + item_controller->Activate(); + running_browser = chrome::GetTotalBrowserCount(); + EXPECT_EQ(1u, running_browser); + EXPECT_TRUE(item_controller->IsOpen()); + EXPECT_FALSE(ash::wm::IsWindowMinimized(window)); +} diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_unittest.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_unittest.cc index d97c0df..7ba89b3 100644 --- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_unittest.cc +++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_unittest.cc @@ -29,6 +29,7 @@ #include "chrome/browser/ui/host_desktop.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/common/extensions/extension.h" +#include "chrome/common/extensions/extension_constants.h" #include "chrome/common/extensions/extension_manifest_constants.h" #include "chrome/common/pref_names.h" #include "chrome/test/base/browser_with_test_window_test.h" @@ -42,7 +43,6 @@ using extensions::Extension; using extensions::Manifest; namespace { -const int kExpectedAppIndex = 1; const char* offline_gmail_url = "https://mail.google.com/mail/mu/u"; const char* gmail_url = "https://mail.google.com/mail/u"; } @@ -195,10 +195,6 @@ class ChromeLauncherControllerPerAppTest : public BrowserWithTestWindowTest { Extension::NO_FLAGS, extension_misc::kGoogleSearchAppId, &error); - - // Create a default launcher controller; some tests will call - // InitLauncherController* to create a new one with a different setup. - InitLauncherController(); } virtual void TearDown() OVERRIDE { @@ -264,6 +260,7 @@ class ChromeLauncherControllerPerAppTest : public BrowserWithTestWindowTest { }; TEST_F(ChromeLauncherControllerPerAppTest, DefaultApps) { + InitLauncherController(); // Model should only contain the browser shortcut and app list items. EXPECT_EQ(2, model_->item_count()); EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id())); @@ -273,7 +270,7 @@ TEST_F(ChromeLauncherControllerPerAppTest, DefaultApps) { // Installing |extension3_| should add it to the launcher. extension_service_->AddExtension(extension3_.get()); EXPECT_EQ(3, model_->item_count()); - EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[kExpectedAppIndex].type); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[0].type); EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id())); EXPECT_FALSE(launcher_controller_->IsAppPinned(extension2_->id())); EXPECT_TRUE(launcher_controller_->IsAppPinned(extension3_->id())); @@ -281,6 +278,7 @@ TEST_F(ChromeLauncherControllerPerAppTest, DefaultApps) { // Check that simple locking of an application will 'create' a launcher item. TEST_F(ChromeLauncherControllerPerAppTest, CheckLockApps) { + InitLauncherController(); // Model should only contain the browser shortcut and app list items. EXPECT_EQ(2, model_->item_count()); EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id())); @@ -293,7 +291,7 @@ TEST_F(ChromeLauncherControllerPerAppTest, CheckLockApps) { launcher_controller_->LockV1AppWithID(extension1_->id()); EXPECT_EQ(3, model_->item_count()); - EXPECT_EQ(ash::TYPE_WINDOWED_APP, model_->items()[kExpectedAppIndex].type); + EXPECT_EQ(ash::TYPE_WINDOWED_APP, model_->items()[1].type); EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id())); EXPECT_TRUE(launcher_controller_->IsWindowedAppInLauncher(extension1_->id())); EXPECT_FALSE(launcher_controller_->IsAppPinned(extension2_->id())); @@ -313,6 +311,7 @@ TEST_F(ChromeLauncherControllerPerAppTest, CheckLockApps) { // Check that multiple locks of an application will be properly handled. TEST_F(ChromeLauncherControllerPerAppTest, CheckMukltiLockApps) { + InitLauncherController(); // Model should only contain the browser shortcut and app list items. EXPECT_EQ(2, model_->item_count()); EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id())); @@ -323,7 +322,7 @@ TEST_F(ChromeLauncherControllerPerAppTest, CheckMukltiLockApps) { launcher_controller_->LockV1AppWithID(extension1_->id()); EXPECT_EQ(3, model_->item_count()); - EXPECT_EQ(ash::TYPE_WINDOWED_APP, model_->items()[kExpectedAppIndex].type); + EXPECT_EQ(ash::TYPE_WINDOWED_APP, model_->items()[1].type); EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id())); EXPECT_TRUE(launcher_controller_->IsWindowedAppInLauncher( extension1_->id())); @@ -332,7 +331,7 @@ TEST_F(ChromeLauncherControllerPerAppTest, CheckMukltiLockApps) { launcher_controller_->UnlockV1AppWithID(extension1_->id()); EXPECT_EQ(3, model_->item_count()); - EXPECT_EQ(ash::TYPE_WINDOWED_APP, model_->items()[kExpectedAppIndex].type); + EXPECT_EQ(ash::TYPE_WINDOWED_APP, model_->items()[1].type); EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id())); EXPECT_TRUE(launcher_controller_->IsWindowedAppInLauncher(extension1_->id())); @@ -349,6 +348,7 @@ TEST_F(ChromeLauncherControllerPerAppTest, CheckMukltiLockApps) { // Check that already pinned items are not effected by locks. TEST_F(ChromeLauncherControllerPerAppTest, CheckAlreadyPinnedLockApps) { + InitLauncherController(); // Model should only contain the browser shortcut and app list items. EXPECT_EQ(2, model_->item_count()); EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id())); @@ -358,7 +358,7 @@ TEST_F(ChromeLauncherControllerPerAppTest, CheckAlreadyPinnedLockApps) { launcher_controller_->PinAppWithID(extension1_->id()); EXPECT_EQ(3, model_->item_count()); - EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[kExpectedAppIndex].type); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[1].type); EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id())); EXPECT_FALSE( launcher_controller_->IsWindowedAppInLauncher(extension1_->id())); @@ -366,7 +366,7 @@ TEST_F(ChromeLauncherControllerPerAppTest, CheckAlreadyPinnedLockApps) { launcher_controller_->LockV1AppWithID(extension1_->id()); EXPECT_EQ(3, model_->item_count()); - EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[kExpectedAppIndex].type); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[1].type); EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id())); EXPECT_FALSE( launcher_controller_->IsWindowedAppInLauncher(extension1_->id())); @@ -374,7 +374,7 @@ TEST_F(ChromeLauncherControllerPerAppTest, CheckAlreadyPinnedLockApps) { launcher_controller_->UnlockV1AppWithID(extension1_->id()); EXPECT_EQ(3, model_->item_count()); - EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[kExpectedAppIndex].type); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[1].type); EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id())); EXPECT_FALSE( launcher_controller_->IsWindowedAppInLauncher(extension1_->id())); @@ -386,6 +386,7 @@ TEST_F(ChromeLauncherControllerPerAppTest, CheckAlreadyPinnedLockApps) { // Check that already pinned items which get locked stay after unpinning. TEST_F(ChromeLauncherControllerPerAppTest, CheckPinnedAppsStayAfterUnlock) { + InitLauncherController(); // Model should only contain the browser shortcut and app list items. EXPECT_EQ(2, model_->item_count()); EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id())); @@ -395,7 +396,7 @@ TEST_F(ChromeLauncherControllerPerAppTest, CheckPinnedAppsStayAfterUnlock) { launcher_controller_->PinAppWithID(extension1_->id()); EXPECT_EQ(3, model_->item_count()); - EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[kExpectedAppIndex].type); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[1].type); EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id())); EXPECT_FALSE( launcher_controller_->IsWindowedAppInLauncher(extension1_->id())); @@ -403,7 +404,7 @@ TEST_F(ChromeLauncherControllerPerAppTest, CheckPinnedAppsStayAfterUnlock) { launcher_controller_->LockV1AppWithID(extension1_->id()); EXPECT_EQ(3, model_->item_count()); - EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[kExpectedAppIndex].type); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[1].type); EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id())); EXPECT_FALSE( launcher_controller_->IsWindowedAppInLauncher(extension1_->id())); @@ -411,7 +412,7 @@ TEST_F(ChromeLauncherControllerPerAppTest, CheckPinnedAppsStayAfterUnlock) { launcher_controller_->UnpinAppsWithID(extension1_->id()); EXPECT_EQ(3, model_->item_count()); - EXPECT_EQ(ash::TYPE_WINDOWED_APP, model_->items()[kExpectedAppIndex].type); + EXPECT_EQ(ash::TYPE_WINDOWED_APP, model_->items()[1].type); EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id())); EXPECT_TRUE(launcher_controller_->IsWindowedAppInLauncher(extension1_->id())); @@ -422,6 +423,7 @@ TEST_F(ChromeLauncherControllerPerAppTest, CheckPinnedAppsStayAfterUnlock) { // Check that lock -> pin -> unlock -> unpin does properly transition. TEST_F(ChromeLauncherControllerPerAppTest, CheckLockPinUnlockUnpin) { + InitLauncherController(); // Model should only contain the browser shortcut and app list items. EXPECT_EQ(2, model_->item_count()); EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id())); @@ -431,14 +433,14 @@ TEST_F(ChromeLauncherControllerPerAppTest, CheckLockPinUnlockUnpin) { launcher_controller_->LockV1AppWithID(extension1_->id()); EXPECT_EQ(3, model_->item_count()); - EXPECT_EQ(ash::TYPE_WINDOWED_APP, model_->items()[kExpectedAppIndex].type); + EXPECT_EQ(ash::TYPE_WINDOWED_APP, model_->items()[1].type); EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id())); EXPECT_TRUE(launcher_controller_->IsWindowedAppInLauncher(extension1_->id())); launcher_controller_->PinAppWithID(extension1_->id()); EXPECT_EQ(3, model_->item_count()); - EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[kExpectedAppIndex].type); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[1].type); EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id())); EXPECT_FALSE( launcher_controller_->IsWindowedAppInLauncher(extension1_->id())); @@ -446,7 +448,7 @@ TEST_F(ChromeLauncherControllerPerAppTest, CheckLockPinUnlockUnpin) { launcher_controller_->UnlockV1AppWithID(extension1_->id()); EXPECT_EQ(3, model_->item_count()); - EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[kExpectedAppIndex].type); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[1].type); EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id())); EXPECT_FALSE( launcher_controller_->IsWindowedAppInLauncher(extension1_->id())); @@ -471,7 +473,7 @@ TEST_F(ChromeLauncherControllerPerAppTest, Policy) { // take effect when the policy override is in place. InitLauncherController(); EXPECT_EQ(3, model_->item_count()); - EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[kExpectedAppIndex].type); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[1].type); EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id())); EXPECT_FALSE(launcher_controller_->IsAppPinned(extension2_->id())); EXPECT_FALSE(launcher_controller_->IsAppPinned(extension3_->id())); @@ -479,7 +481,7 @@ TEST_F(ChromeLauncherControllerPerAppTest, Policy) { // Installing |extension2_| should add it to the launcher. extension_service_->AddExtension(extension2_.get()); EXPECT_EQ(4, model_->item_count()); - EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[kExpectedAppIndex].type); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[1].type); EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[2].type); EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id())); EXPECT_TRUE(launcher_controller_->IsAppPinned(extension2_->id())); @@ -490,7 +492,7 @@ TEST_F(ChromeLauncherControllerPerAppTest, Policy) { profile()->GetTestingPrefService()->SetManagedPref(prefs::kPinnedLauncherApps, policy_value.DeepCopy()); EXPECT_EQ(3, model_->item_count()); - EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[kExpectedAppIndex].type); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_->items()[1].type); EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id())); EXPECT_TRUE(launcher_controller_->IsAppPinned(extension2_->id())); EXPECT_FALSE(launcher_controller_->IsAppPinned(extension3_->id())); @@ -652,6 +654,8 @@ TEST_F(ChromeLauncherControllerPerAppTest, BrowserMenuGeneration) { // Check that the browser list is empty at this time. ash::LauncherItem item_browser; item_browser.type = ash::TYPE_BROWSER_SHORTCUT; + item_browser.id = + launcher_controller_->GetLauncherIDForAppID(extension_misc::kChromeAppId); EXPECT_TRUE(CheckMenuCreation( launcher_controller_.get(), item_browser, 0, NULL, true)); @@ -661,6 +665,7 @@ TEST_F(ChromeLauncherControllerPerAppTest, BrowserMenuGeneration) { string16 title1 = ASCIIToUTF16("Test1"); NavigateAndCommitActiveTabWithTitle(browser(), GURL("http://test1"), title1); string16 one_menu_item[] = {title1}; + EXPECT_TRUE(CheckMenuCreation( launcher_controller_.get(), item_browser, 1, one_menu_item, true)); @@ -710,6 +715,8 @@ TEST_F(ChromeLauncherControllerPerAppTest, V1AppMenuGeneration) { // Check the menu content. ash::LauncherItem item_browser; item_browser.type = ash::TYPE_BROWSER_SHORTCUT; + item_browser.id = + launcher_controller_->GetLauncherIDForAppID(extension_misc::kChromeAppId); ash::LauncherItem item_gmail; item_gmail.type = ash::TYPE_APP_SHORTCUT; @@ -851,6 +858,7 @@ TEST_F(ChromeLauncherControllerPerAppTest, V1AppMenuDeletionExecution) { // Tests that panels create launcher items correctly TEST_F(ChromeLauncherControllerPerAppTest, AppPanels) { InitLauncherControllerWithBrowser(); + EXPECT_EQ(1, model_observer_->added()); TestAppIconLoaderImpl* app_icon_loader = new TestAppIconLoaderImpl(); SetAppIconLoader(app_icon_loader); @@ -862,7 +870,7 @@ TEST_F(ChromeLauncherControllerPerAppTest, AppPanels) { launcher_controller_.get()); ash::LauncherID launcher_id1 = launcher_controller_->CreateAppLauncherItem( &app_panel_controller, app_id, ash::STATUS_RUNNING); - EXPECT_EQ(1, model_observer_->added()); + EXPECT_EQ(2, model_observer_->added()); EXPECT_EQ(0, model_observer_->changed()); EXPECT_EQ(1, app_icon_loader->fetch_count()); model_observer_->clear_counts(); diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_browser.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_browser.cc index 9eebfcd0..d319d73 100644 --- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_browser.cc +++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_browser.cc @@ -52,9 +52,11 @@ #include "content/public/browser/notification_service.h" #include "content/public/browser/web_contents.h" #include "extensions/common/url_pattern.h" +#include "grit/chromium_strings.h" #include "grit/theme_resources.h" #include "ui/aura/root_window.h" #include "ui/aura/window.h" +#include "ui/base/l10n/l10n_util.h" #if defined(OS_CHROMEOS) #include "chrome/browser/chromeos/login/default_pinned_apps_field_trial.h" @@ -128,7 +130,8 @@ class AppShortcutLauncherItemController : public LauncherItemController { const ash::LauncherItem& old_item) OVERRIDE { } - virtual ChromeLauncherAppMenuItems GetApplicationList() OVERRIDE { + virtual ChromeLauncherAppMenuItems GetApplicationList( + int event_flags) OVERRIDE { ChromeLauncherAppMenuItems items; return items.Pass(); } @@ -331,6 +334,7 @@ ChromeLauncherControllerPerBrowser::~ChromeLauncherControllerPerBrowser() { void ChromeLauncherControllerPerBrowser::Init() { UpdateAppLaunchersFromPref(); + CreateBrowserShortcutLauncherItem(); // TODO(sky): update unit test so that this test isn't necessary. if (ash::Shell::HasInstance()) { @@ -451,7 +455,7 @@ void ChromeLauncherControllerPerBrowser::Pin(ash::LauncherID id) { bool ChromeLauncherControllerPerBrowser::IsPinned(ash::LauncherID id) { int index = model_->ItemIndexByID(id); ash::LauncherItemType type = model_->items()[index].type; - return type == ash::TYPE_APP_SHORTCUT; + return (type == ash::TYPE_APP_SHORTCUT || type == ash::TYPE_BROWSER_SHORTCUT); } void ChromeLauncherControllerPerBrowser::TogglePinned(ash::LauncherID id) { @@ -539,7 +543,7 @@ void ChromeLauncherControllerPerBrowser::LaunchApp(const std::string& app_id, void ChromeLauncherControllerPerBrowser::ActivateApp(const std::string& app_id, int event_flags) { if (app_id == extension_misc::kChromeAppId) { - OnBrowserShortcutClicked(event_flags); + BrowserShortcutClicked(event_flags); return; } @@ -667,6 +671,14 @@ void ChromeLauncherControllerPerBrowser::SetLauncherItemImage( bool ChromeLauncherControllerPerBrowser::IsAppPinned( const std::string& app_id) { + // Check the LauncherModel since there is no controller for the browser item. + if (app_id == extension_misc::kChromeAppId) { + for (size_t index = 0; index < model_->items().size(); index++) { + if (model_->items()[index].type == ash::TYPE_BROWSER_SHORTCUT) + return true; + } + return false; + } for (IDToItemControllerMap::const_iterator i = id_to_item_controller_map_.begin(); i != id_to_item_controller_map_.end(); ++i) { @@ -861,7 +873,7 @@ void ChromeLauncherControllerPerBrowser::ActivateWindowOrMinimizeIfActive( window->Activate(); } -void ChromeLauncherControllerPerBrowser::OnBrowserShortcutClicked( +void ChromeLauncherControllerPerBrowser::BrowserShortcutClicked( int event_flags) { #if defined(OS_CHROMEOS) chromeos::default_pinned_apps_field_trial::RecordShelfClick( @@ -888,6 +900,11 @@ void ChromeLauncherControllerPerBrowser::OnBrowserShortcutClicked( void ChromeLauncherControllerPerBrowser::ItemSelected( const ash::LauncherItem& item, const ui::Event& event) { + if (item.type == ash::TYPE_BROWSER_SHORTCUT) { + BrowserShortcutClicked(event.flags()); + return; + } + DCHECK(HasItemController(item.id)); LauncherItemController* item_controller = id_to_item_controller_map_[item.id]; #if defined(OS_CHROMEOS) @@ -899,12 +916,11 @@ void ChromeLauncherControllerPerBrowser::ItemSelected( item_controller->Clicked(event); } -int ChromeLauncherControllerPerBrowser::GetBrowserShortcutResourceId() { - return IDR_PRODUCT_LOGO_32; -} - string16 ChromeLauncherControllerPerBrowser::GetTitle( const ash::LauncherItem& item) { + if (item.type == ash::TYPE_BROWSER_SHORTCUT) + return l10n_util::GetStringUTF16(IDS_PRODUCT_NAME); + DCHECK(HasItemController(item.id)); return id_to_item_controller_map_[item.id]->GetTitle(); } @@ -974,6 +990,9 @@ void ChromeLauncherControllerPerBrowser::LauncherItemMoved( ash::LauncherID id = model_->items()[target_index].id; if (HasItemController(id) && IsPinned(id)) PersistPinnedState(); + else if (!HasItemController(id) && + model_->items()[target_index].type == ash::TYPE_BROWSER_SHORTCUT) + PersistPinnedState(); } void ChromeLauncherControllerPerBrowser::LauncherItemChanged( @@ -1109,6 +1128,8 @@ void ChromeLauncherControllerPerBrowser::PersistPinnedState() { if (app_value) updater->Append(app_value); } + } else if (model_->items()[i].type == ash::TYPE_BROWSER_SHORTCUT) { + SetChromeIconIndexToPref(i); } } } @@ -1359,6 +1380,31 @@ bool ChromeLauncherControllerPerBrowser::HasItemController( } ash::LauncherID +ChromeLauncherControllerPerBrowser::CreateBrowserShortcutLauncherItem() { + ash::LauncherItem browser_shortcut; + browser_shortcut.type = ash::TYPE_BROWSER_SHORTCUT; + browser_shortcut.is_incognito = false; + ResourceBundle& rb = ResourceBundle::GetSharedInstance(); + browser_shortcut.image = *rb.GetImageSkiaNamed(IDR_PRODUCT_LOGO_32); + ash::LauncherID id = model_->next_id(); + size_t index = GetChromeIconIndexFromPref(); + model_->AddAt(index, browser_shortcut); + return id; +} + +void ChromeLauncherControllerPerBrowser::SetChromeIconIndexToPref(int index) { + profile_->GetPrefs()->SetInteger(prefs::kShelfChromeIconIndex, index); +} + +int ChromeLauncherControllerPerBrowser::GetChromeIconIndexFromPref() const { + size_t index = profile_->GetPrefs()->GetInteger(prefs::kShelfChromeIconIndex); + const base::ListValue* pinned_apps_pref = + profile_->GetPrefs()->GetList(prefs::kPinnedLauncherApps); + return std::max(static_cast<size_t>(0), + std::min(pinned_apps_pref->GetSize(), index)); +} + +ash::LauncherID ChromeLauncherControllerPerBrowser::CreateAppShortcutLauncherItem( const std::string& app_id, int index) { diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_browser.h b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_browser.h index 7ddeee1..419e89e 100644 --- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_browser.h +++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_browser.h @@ -248,10 +248,8 @@ class ChromeLauncherControllerPerBrowser bool allow_minimize) OVERRIDE; // ash::LauncherDelegate overrides: - virtual void OnBrowserShortcutClicked(int event_flags) OVERRIDE; virtual void ItemSelected(const ash::LauncherItem& item, const ui::Event& event) OVERRIDE; - virtual int GetBrowserShortcutResourceId() OVERRIDE; virtual string16 GetTitle(const ash::LauncherItem& item) OVERRIDE; virtual ui::MenuModel* CreateContextMenu( const ash::LauncherItem& item, aura::RootWindow* root) OVERRIDE; @@ -368,6 +366,18 @@ class ChromeLauncherControllerPerBrowser bool HasItemController(ash::LauncherID id) const; + // Create LauncherItem for Browser Shortcut. + ash::LauncherID CreateBrowserShortcutLauncherItem(); + + // Update browser shortcut's index. + void SetChromeIconIndexToPref(int index); + + // Get browser shortcut's index from pref. + int GetChromeIconIndexFromPref() const; + + // Invoked when browser shortcut is clicked. + void BrowserShortcutClicked(int event_flags); + ash::LauncherModel* model_; // Profile used for prefs and loading extensions. This is NOT necessarily the diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_browser_unittest.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_browser_unittest.cc index 248078d..44ce905 100644 --- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_browser_unittest.cc +++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_browser_unittest.cc @@ -35,10 +35,6 @@ using extensions::Extension; using extensions::Manifest; -namespace { -const int kExpectedAppIndex = 1; -} - class ChromeLauncherControllerPerBrowserTest : public testing::Test { protected: ChromeLauncherControllerPerBrowserTest() @@ -148,7 +144,9 @@ TEST_F(ChromeLauncherControllerPerBrowserTest, DefaultApps) { // Installing |extension3_| should add it to the launcher. extension_service_->AddExtension(extension3_.get()); EXPECT_EQ(3, model_.item_count()); - EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_.items()[kExpectedAppIndex].type); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_.items()[0].type); + EXPECT_EQ(ash::TYPE_BROWSER_SHORTCUT, model_.items()[1].type); + EXPECT_EQ(ash::TYPE_APP_LIST, model_.items()[2].type); EXPECT_FALSE(launcher_controller.IsAppPinned(extension1_->id())); EXPECT_FALSE(launcher_controller.IsAppPinned(extension2_->id())); EXPECT_TRUE(launcher_controller.IsAppPinned(extension3_->id())); @@ -171,16 +169,22 @@ TEST_F(ChromeLauncherControllerPerBrowserTest, Policy) { &model_); launcher_controller.Init(); EXPECT_EQ(3, model_.item_count()); - EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_.items()[kExpectedAppIndex].type); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_.items()[1].type); EXPECT_TRUE(launcher_controller.IsAppPinned(extension1_->id())); EXPECT_FALSE(launcher_controller.IsAppPinned(extension2_->id())); EXPECT_FALSE(launcher_controller.IsAppPinned(extension3_->id())); + EXPECT_EQ(ash::TYPE_BROWSER_SHORTCUT, model_.items()[0].type); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_.items()[1].type); + EXPECT_EQ(ash::TYPE_APP_LIST, model_.items()[2].type); + // Installing |extension2_| should add it to the launcher. extension_service_->AddExtension(extension2_.get()); EXPECT_EQ(4, model_.item_count()); - EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_.items()[kExpectedAppIndex].type); + EXPECT_EQ(ash::TYPE_BROWSER_SHORTCUT, model_.items()[0].type); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_.items()[1].type); EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_.items()[2].type); + EXPECT_EQ(ash::TYPE_APP_LIST, model_.items()[3].type); EXPECT_TRUE(launcher_controller.IsAppPinned(extension1_->id())); EXPECT_TRUE(launcher_controller.IsAppPinned(extension2_->id())); EXPECT_FALSE(launcher_controller.IsAppPinned(extension3_->id())); @@ -190,7 +194,7 @@ TEST_F(ChromeLauncherControllerPerBrowserTest, Policy) { profile_->GetTestingPrefService()->SetManagedPref(prefs::kPinnedLauncherApps, policy_value.DeepCopy()); EXPECT_EQ(3, model_.item_count()); - EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_.items()[kExpectedAppIndex].type); + EXPECT_EQ(ash::TYPE_APP_SHORTCUT, model_.items()[1].type); EXPECT_FALSE(launcher_controller.IsAppPinned(extension1_->id())); EXPECT_TRUE(launcher_controller.IsAppPinned(extension2_->id())); EXPECT_FALSE(launcher_controller.IsAppPinned(extension3_->id())); diff --git a/chrome/browser/ui/ash/launcher/launcher_item_controller.h b/chrome/browser/ui/ash/launcher/launcher_item_controller.h index c2a62f1..4b07a5f 100644 --- a/chrome/browser/ui/ash/launcher/launcher_item_controller.h +++ b/chrome/browser/ui/ash/launcher/launcher_item_controller.h @@ -100,7 +100,7 @@ class LauncherItemController { virtual void OnRemoved() = 0; // Called to retrieve the list of running applications. - virtual ChromeLauncherAppMenuItems GetApplicationList() = 0; + virtual ChromeLauncherAppMenuItems GetApplicationList(int event_flags) = 0; // Helper function to get the ash::LauncherItemType for the item type. ash::LauncherItemType GetLauncherItemType() const; diff --git a/chrome/browser/ui/ash/launcher/shell_window_launcher_item_controller.cc b/chrome/browser/ui/ash/launcher/shell_window_launcher_item_controller.cc index 852b4a9..bde22ab 100644 --- a/chrome/browser/ui/ash/launcher/shell_window_launcher_item_controller.cc +++ b/chrome/browser/ui/ash/launcher/shell_window_launcher_item_controller.cc @@ -192,7 +192,7 @@ void ShellWindowLauncherItemController::ActivateIndexedApp(size_t index) { } ChromeLauncherAppMenuItems -ShellWindowLauncherItemController::GetApplicationList() { +ShellWindowLauncherItemController::GetApplicationList(int event_flags) { ChromeLauncherAppMenuItems items; items.push_back(new ChromeLauncherAppMenuItem(GetTitle(), NULL, false)); int index = 0; @@ -234,7 +234,7 @@ void ShellWindowLauncherItemController::ShowAndActivateOrMinimize( // Either show or minimize windows when shown from the launcher. launcher_controller()->ActivateWindowOrMinimizeIfActive( shell_window->GetBaseWindow(), - GetApplicationList().size() == 2); + GetApplicationList(0).size() == 2); } void ShellWindowLauncherItemController::ActivateOrAdvanceToNextShellWindow( diff --git a/chrome/browser/ui/ash/launcher/shell_window_launcher_item_controller.h b/chrome/browser/ui/ash/launcher/shell_window_launcher_item_controller.h index ea22375..182ca98 100644 --- a/chrome/browser/ui/ash/launcher/shell_window_launcher_item_controller.h +++ b/chrome/browser/ui/ash/launcher/shell_window_launcher_item_controller.h @@ -64,7 +64,8 @@ class ShellWindowLauncherItemController : public LauncherItemController, virtual void LauncherItemChanged( int model_index, const ash::LauncherItem& old_item) OVERRIDE {} - virtual ChromeLauncherAppMenuItems GetApplicationList() OVERRIDE; + virtual ChromeLauncherAppMenuItems GetApplicationList( + int event_flags) OVERRIDE; // aura::WindowObserver virtual void OnWindowPropertyChanged(aura::Window* window, diff --git a/chrome/chrome_browser_ui.gypi b/chrome/chrome_browser_ui.gypi index 52fb8bf..aa73c2a 100644 --- a/chrome/chrome_browser_ui.gypi +++ b/chrome/chrome_browser_ui.gypi @@ -192,6 +192,8 @@ 'browser/ui/ash/launcher/app_shortcut_launcher_item_controller.h', 'browser/ui/ash/launcher/browser_launcher_item_controller.cc', 'browser/ui/ash/launcher/browser_launcher_item_controller.h', + 'browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.cc', + 'browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.h', 'browser/ui/ash/launcher/chrome_launcher_app_menu_item.cc', 'browser/ui/ash/launcher/chrome_launcher_app_menu_item.h', 'browser/ui/ash/launcher/chrome_launcher_app_menu_item_browser.cc', diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc index 1f447b4..42c4232 100644 --- a/chrome/common/pref_names.cc +++ b/chrome/common/pref_names.cc @@ -2290,6 +2290,13 @@ const char kShelfAlignmentLocal[] = "shelf_alignment_local"; // String value corresponding to ash::Shell::ShelfAutoHideBehavior. const char kShelfAutoHideBehavior[] = "auto_hide_behavior"; const char kShelfAutoHideBehaviorLocal[] = "auto_hide_behavior_local"; +// This value stores chrome icon's index in the launcher. This should be handled +// separately with app shortcut's index because of LauncherModel's backward +// compatability. If we add chrome icon index to |kPinnedLauncherApps|, its +// index is also stored in the |kPinnedLauncherApp| pref. It may causes +// creating two chrome icons. +const char kShelfChromeIconIndex[] = "shelf_chrome_icon_index"; + const char kPinnedLauncherApps[] = "pinned_launcher_apps"; // Boolean value indicating whether to show a logout button in the ash tray. const char kShowLogoutButtonInTray[] = "show_logout_button_in_tray"; diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h index ce4c4a8..44dcf92 100644 --- a/chrome/common/pref_names.h +++ b/chrome/common/pref_names.h @@ -829,6 +829,7 @@ extern const char kShelfAlignment[]; extern const char kShelfAlignmentLocal[]; extern const char kShelfAutoHideBehavior[]; extern const char kShelfAutoHideBehaviorLocal[]; +extern const char kShelfChromeIconIndex[]; extern const char kPinnedLauncherApps[]; extern const char kShowLogoutButtonInTray[]; extern const char kShelfPreferences[]; |