diff options
19 files changed, 703 insertions, 323 deletions
diff --git a/chrome/browser/bookmarks/bookmark_context_menu_controller.cc b/chrome/browser/bookmarks/bookmark_context_menu_controller.cc index 1c6e8b4..325f341 100644 --- a/chrome/browser/bookmarks/bookmark_context_menu_controller.cc +++ b/chrome/browser/bookmarks/bookmark_context_menu_controller.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -210,7 +210,10 @@ BookmarkContextMenuController::BookmarkContextMenuController( model_(profile->GetBookmarkModel()) { DCHECK(profile_); DCHECK(model_->IsLoaded()); + menu_model_.reset(new menus::SimpleMenuModel(this)); model_->AddObserver(this); + + BuildMenu(); } BookmarkContextMenuController::~BookmarkContextMenuController() { @@ -221,60 +224,82 @@ BookmarkContextMenuController::~BookmarkContextMenuController() { void BookmarkContextMenuController::BuildMenu() { if (configuration_ != BOOKMARK_MANAGER_ORGANIZE_MENU) { if (selection_.size() == 1 && selection_[0]->is_url()) { - delegate_->AddItemWithStringId(IDS_BOOMARK_BAR_OPEN_ALL, - IDS_BOOMARK_BAR_OPEN_IN_NEW_TAB); - delegate_->AddItemWithStringId(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW, - IDS_BOOMARK_BAR_OPEN_IN_NEW_WINDOW); - delegate_->AddItemWithStringId(IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO, - IDS_BOOMARK_BAR_OPEN_INCOGNITO); + AddItem(IDS_BOOMARK_BAR_OPEN_ALL, + IDS_BOOMARK_BAR_OPEN_IN_NEW_TAB); + AddItem(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW, + IDS_BOOMARK_BAR_OPEN_IN_NEW_WINDOW); + AddItem(IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO, + IDS_BOOMARK_BAR_OPEN_INCOGNITO); } else { - delegate_->AddItem(IDS_BOOMARK_BAR_OPEN_ALL); - delegate_->AddItem(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW); - delegate_->AddItem(IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO); + AddItem(IDS_BOOMARK_BAR_OPEN_ALL); + AddItem(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW); + AddItem(IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO); } - delegate_->AddSeparator(); + AddSeparator(); } if (selection_.size() == 1 && selection_[0]->is_folder()) { - delegate_->AddItem(IDS_BOOKMARK_BAR_RENAME_FOLDER); + AddItem(IDS_BOOKMARK_BAR_RENAME_FOLDER); } else { - delegate_->AddItem(IDS_BOOKMARK_BAR_EDIT); + AddItem(IDS_BOOKMARK_BAR_EDIT); } + AddItem(IDS_BOOKMARK_BAR_REMOVE); if (configuration_ == BOOKMARK_MANAGER_TABLE || configuration_ == BOOKMARK_MANAGER_TABLE_OTHER || configuration_ == BOOKMARK_MANAGER_ORGANIZE_MENU || configuration_ == BOOKMARK_MANAGER_ORGANIZE_MENU_OTHER) { - delegate_->AddItem(IDS_BOOKMARK_MANAGER_SHOW_IN_FOLDER); + AddItem(IDS_BOOKMARK_MANAGER_SHOW_IN_FOLDER); } - delegate_->AddSeparator(); - delegate_->AddItem(IDS_CUT); - delegate_->AddItem(IDS_COPY); - delegate_->AddItem(IDS_PASTE); - - delegate_->AddSeparator(); - delegate_->AddItem(IDS_BOOKMARK_BAR_REMOVE); + if (configuration_ == BOOKMARK_MANAGER_TABLE || + configuration_ == BOOKMARK_MANAGER_TABLE_OTHER || + configuration_ == BOOKMARK_MANAGER_TREE || + configuration_ == BOOKMARK_MANAGER_ORGANIZE_MENU || + configuration_ == BOOKMARK_MANAGER_ORGANIZE_MENU_OTHER) { + AddSeparator(); + AddItem(IDS_CUT); + AddItem(IDS_COPY); + AddItem(IDS_PASTE); + } if (configuration_ == BOOKMARK_MANAGER_ORGANIZE_MENU) { - delegate_->AddSeparator(); - delegate_->AddItem(IDS_BOOKMARK_MANAGER_SORT); + AddSeparator(); + AddItem(IDS_BOOKMARK_MANAGER_SORT); } - delegate_->AddSeparator(); + AddSeparator(); - delegate_->AddItem(IDS_BOOMARK_BAR_ADD_NEW_BOOKMARK); - delegate_->AddItem(IDS_BOOMARK_BAR_NEW_FOLDER); + AddItem(IDS_BOOMARK_BAR_ADD_NEW_BOOKMARK); + AddItem(IDS_BOOMARK_BAR_NEW_FOLDER); if (configuration_ == BOOKMARK_BAR) { - delegate_->AddSeparator(); - delegate_->AddItem(IDS_BOOKMARK_MANAGER); - delegate_->AddCheckboxItem(IDS_BOOMARK_BAR_ALWAYS_SHOW); + AddSeparator(); + AddItem(IDS_BOOKMARK_MANAGER); + AddCheckboxItem(IDS_BOOMARK_BAR_ALWAYS_SHOW); } } +void BookmarkContextMenuController::AddItem(int id) { + menu_model_->AddItem(id, l10n_util::GetStringUTF16(id)); +} + +void BookmarkContextMenuController::AddItem(int id, int localization_id) { + menu_model_->AddItemWithStringId(id, localization_id); +} + +void BookmarkContextMenuController::AddSeparator() { + menu_model_->AddSeparator(); +} + +void BookmarkContextMenuController::AddCheckboxItem(int id) { + menu_model_->AddCheckItemWithStringId(id, id); +} + void BookmarkContextMenuController::ExecuteCommand(int id) { BookmarkModel* model = RemoveModelObserver(); + if (delegate_) + delegate_->WillExecuteCommand(); switch (id) { case IDS_BOOMARK_BAR_OPEN_ALL: @@ -305,7 +330,7 @@ void BookmarkContextMenuController::ExecuteCommand(int id) { if (selection_.size() != 1) { NOTREACHED(); - return; + break; } if (selection_[0]->is_url()) { @@ -326,12 +351,10 @@ void BookmarkContextMenuController::ExecuteCommand(int id) { case IDS_BOOKMARK_BAR_REMOVE: { UserMetrics::RecordAction("BookmarkBar_ContextMenu_Remove", profile_); - delegate_->WillRemoveBookmarks(selection_); for (size_t i = 0; i < selection_.size(); ++i) { model->Remove(selection_[i]->GetParent(), selection_[i]->GetParent()->IndexOfChild(selection_[i])); } - delegate_->DidRemoveBookmarks(); selection_.clear(); break; } @@ -373,7 +396,7 @@ void BookmarkContextMenuController::ExecuteCommand(int id) { if (selection_.size() != 1) { NOTREACHED(); - return; + break; } BookmarkManager::SelectInTree(profile_, selection_[0]); @@ -390,9 +413,7 @@ void BookmarkContextMenuController::ExecuteCommand(int id) { break; case IDS_CUT: - delegate_->WillRemoveBookmarks(selection_); bookmark_utils::CopyToClipboard(model, selection_, true); - delegate_->DidRemoveBookmarks(); break; case IDS_COPY: @@ -415,18 +436,21 @@ void BookmarkContextMenuController::ExecuteCommand(int id) { default: NOTREACHED(); } + + if (delegate_) + delegate_->DidExecuteCommand(); } -bool BookmarkContextMenuController::IsItemChecked(int id) const { - DCHECK(id == IDS_BOOMARK_BAR_ALWAYS_SHOW); +bool BookmarkContextMenuController::IsCommandIdChecked(int command_id) const { + DCHECK(command_id == IDS_BOOMARK_BAR_ALWAYS_SHOW); return profile_->GetPrefs()->GetBoolean(prefs::kShowBookmarkBar); } -bool BookmarkContextMenuController::IsCommandEnabled(int id) const { +bool BookmarkContextMenuController::IsCommandIdEnabled(int command_id) const { bool is_root_node = (selection_.size() == 1 && selection_[0]->GetParent() == model_->root_node()); - switch (id) { + switch (command_id) { case IDS_BOOMARK_BAR_OPEN_INCOGNITO: return !profile_->IsOffTheRecord(); @@ -463,8 +487,8 @@ bool BookmarkContextMenuController::IsCommandEnabled(int id) const { case IDS_PASTE: // Paste to selection from the Bookmark Bar, to parent_ everywhere else return (configuration_ == BOOKMARK_BAR && !selection_.empty() && - bookmark_utils::CanPasteFromClipboard(selection_[0])) || - bookmark_utils::CanPasteFromClipboard(parent_); + bookmark_utils::CanPasteFromClipboard(selection_[0])) || + bookmark_utils::CanPasteFromClipboard(parent_); } return true; } @@ -511,7 +535,8 @@ void BookmarkContextMenuController::BookmarkNodeChildrenReordered( } void BookmarkContextMenuController::ModelChanged() { - delegate_->CloseMenu(); + if (delegate_) + delegate_->CloseMenu(); } BookmarkModel* BookmarkContextMenuController::RemoveModelObserver() { diff --git a/chrome/browser/bookmarks/bookmark_context_menu_controller.h b/chrome/browser/bookmarks/bookmark_context_menu_controller.h index 3812e28..e2e2194 100644 --- a/chrome/browser/bookmarks/bookmark_context_menu_controller.h +++ b/chrome/browser/bookmarks/bookmark_context_menu_controller.h @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -8,6 +8,7 @@ #include <vector> #include "app/gfx/native_widget_types.h" +#include "app/menus/simple_menu_model.h" #include "base/basictypes.h" #include "chrome/browser/bookmarks/bookmark_model_observer.h" @@ -24,23 +25,17 @@ class BookmarkContextMenuControllerDelegate { // Closes the bookmark context menu. virtual void CloseMenu() = 0; - // Methods that add items to the underlying menu. - virtual void AddItem(int command_id) = 0; - virtual void AddItemWithStringId(int command_id, int string_id) = 0; - virtual void AddSeparator() = 0; - virtual void AddCheckboxItem(int command_id) = 0; + // Sent before any command from the menu is executed. + virtual void WillExecuteCommand() {} - // Sent before bookmarks are removed. - virtual void WillRemoveBookmarks( - const std::vector<const BookmarkNode*>& bookmarks) {} - - // Sent after bookmarks have been removed. - virtual void DidRemoveBookmarks() {} + // Sent after any command from the menu is executed. + virtual void DidExecuteCommand() {} }; // BookmarkContextMenuController creates and manages state for the context menu // shown for any bookmark item. -class BookmarkContextMenuController : public BookmarkModelObserver { +class BookmarkContextMenuController : public BookmarkModelObserver, + public menus::SimpleMenuModel::Delegate { public: // Used to configure what the context menu shows. enum ConfigurationType { @@ -75,15 +70,35 @@ class BookmarkContextMenuController : public BookmarkModelObserver { void BuildMenu(); - void ExecuteCommand(int id); - bool IsItemChecked(int id) const; - bool IsCommandEnabled(int id) const; + menus::SimpleMenuModel* menu_model() { + return menu_model_.get(); + } + + + // menus::SimpleMenuModel::Delegate implementation: + virtual bool IsCommandIdChecked(int command_id) const; + virtual bool IsCommandIdEnabled(int command_id) const; + virtual bool GetAcceleratorForCommandId( + int command_id, + menus::Accelerator* accelerator) { + return false; + } + virtual void ExecuteCommand(int command_id); // Accessors: Profile* profile() const { return profile_; } PageNavigator* navigator() const { return navigator_; } private: + // Adds a IDS_* style command to the menu. + void AddItem(int id); + // Adds a IDS_* style command to the menu with a different localized string. + void AddItem(int id, int localization_id); + // Adds a separator to the menu. + void AddSeparator(); + // Adds a checkable item to the menu. + void AddCheckboxItem(int id); + // BookmarkModelObserver methods. Any change to the model results in closing // the menu. virtual void Loaded(BookmarkModel* model) {} @@ -129,6 +144,7 @@ class BookmarkContextMenuController : public BookmarkModelObserver { std::vector<const BookmarkNode*> selection_; ConfigurationType configuration_; BookmarkModel* model_; + scoped_ptr<menus::SimpleMenuModel> menu_model_; DISALLOW_COPY_AND_ASSIGN(BookmarkContextMenuController); }; diff --git a/chrome/browser/bookmarks/bookmark_context_menu_controller_unittest.cc b/chrome/browser/bookmarks/bookmark_context_menu_controller_unittest.cc new file mode 100755 index 0000000..96907f8 --- /dev/null +++ b/chrome/browser/bookmarks/bookmark_context_menu_controller_unittest.cc @@ -0,0 +1,346 @@ +// Copyright (c) 2010 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 "base/scoped_ptr.h" +#include "chrome/browser/bookmarks/bookmark_context_menu_controller.h" +#include "chrome/browser/bookmarks/bookmark_model.h" +#include "chrome/browser/bookmarks/bookmark_utils.h" +#include "chrome/browser/chrome_thread.h" +#include "chrome/browser/profile.h" +#include "chrome/common/pref_names.h" +#include "chrome/common/pref_service.h" +#include "chrome/browser/tab_contents/page_navigator.h" +#include "chrome/test/testing_profile.h" +#include "grit/generated_resources.h" +#include "testing/gtest/include/gtest/gtest.h" + +#if defined(OS_WIN) +#include "chrome/browser/views/bookmark_bar_view.h" +#endif + +namespace { + +// PageNavigator implementation that records the URL. +class TestingPageNavigator : public PageNavigator { + public: + virtual void OpenURL(const GURL& url, + const GURL& referrer, + WindowOpenDisposition disposition, + PageTransition::Type transition) { + urls_.push_back(url); + } + + std::vector<GURL> urls_; +}; + +} // namespace + +class BookmarkContextMenuTest : public testing::Test { + public: + BookmarkContextMenuTest() + : ui_thread_(ChromeThread::UI, &message_loop_), + file_thread_(ChromeThread::FILE, &message_loop_), + model_(NULL) { + } + + virtual void SetUp() { +#if defined(OS_WIN) + BookmarkBarView::testing_ = true; +#endif + + profile_.reset(new TestingProfile()); + profile_->set_has_history_service(true); + profile_->CreateBookmarkModel(true); + profile_->BlockUntilBookmarkModelLoaded(); + + model_ = profile_->GetBookmarkModel(); + + AddTestData(); + } + + virtual void TearDown() { +#if defined(OS_WIN) + BookmarkBarView::testing_ = false; +#endif + + // Flush the message loop to make Purify happy. + message_loop_.RunAllPending(); + } + + protected: + MessageLoopForUI message_loop_; + ChromeThread ui_thread_; + ChromeThread file_thread_; + scoped_ptr<TestingProfile> profile_; + BookmarkModel* model_; + TestingPageNavigator navigator_; + + private: + // Creates the following structure: + // a + // F1 + // f1a + // F11 + // f11a + // F2 + // F3 + // F4 + // f4a + void AddTestData() { + std::string test_base = "file:///c:/tmp/"; + + model_->AddURL(model_->GetBookmarkBarNode(), 0, L"a", + GURL(test_base + "a")); + const BookmarkNode* f1 = + model_->AddGroup(model_->GetBookmarkBarNode(), 1, L"F1"); + model_->AddURL(f1, 0, L"f1a", GURL(test_base + "f1a")); + const BookmarkNode* f11 = model_->AddGroup(f1, 1, L"F11"); + model_->AddURL(f11, 0, L"f11a", GURL(test_base + "f11a")); + model_->AddGroup(model_->GetBookmarkBarNode(), 2, L"F2"); + model_->AddGroup(model_->GetBookmarkBarNode(), 3, L"F3"); + const BookmarkNode* f4 = + model_->AddGroup(model_->GetBookmarkBarNode(), 4, L"F4"); + model_->AddURL(f4, 0, L"f4a", GURL(test_base + "f4a")); + } +}; + +// Tests Deleting from the menu. +TEST_F(BookmarkContextMenuTest, DeleteURL) { + std::vector<const BookmarkNode*> nodes; + nodes.push_back(model_->GetBookmarkBarNode()->GetChild(0)); + BookmarkContextMenuController controller( + NULL, NULL, profile_.get(), NULL, nodes[0]->GetParent(), nodes, + BookmarkContextMenuController::BOOKMARK_BAR); + GURL url = model_->GetBookmarkBarNode()->GetChild(0)->GetURL(); + ASSERT_TRUE(controller.IsCommandIdEnabled(IDS_BOOKMARK_BAR_REMOVE)); + // Delete the URL. + controller.ExecuteCommand(IDS_BOOKMARK_BAR_REMOVE); + // Model shouldn't have URL anymore. + ASSERT_FALSE(model_->IsBookmarked(url)); +} + +// Tests open all on a folder with a couple of bookmarks. +TEST_F(BookmarkContextMenuTest, OpenAll) { + const BookmarkNode* folder = model_->GetBookmarkBarNode()->GetChild(1); + bookmark_utils::OpenAll( + NULL, profile_.get(), &navigator_, folder, NEW_FOREGROUND_TAB); + + // Should have navigated to F1's children. + ASSERT_EQ(static_cast<size_t>(2), navigator_.urls_.size()); + ASSERT_TRUE(folder->GetChild(0)->GetURL() == navigator_.urls_[0]); + ASSERT_TRUE(folder->GetChild(1)->GetChild(0)->GetURL() == + navigator_.urls_[1]); +} + +// Tests the enabled state of the menus when supplied an empty vector. +TEST_F(BookmarkContextMenuTest, EmptyNodes) { + BookmarkContextMenuController controller( + NULL, NULL, profile_.get(), NULL, model_->other_node(), + std::vector<const BookmarkNode*>(), + BookmarkContextMenuController::BOOKMARK_BAR); + EXPECT_FALSE(controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_ALL)); + EXPECT_FALSE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW)); + EXPECT_FALSE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO)); + EXPECT_FALSE(controller.IsCommandIdEnabled(IDS_BOOKMARK_BAR_REMOVE)); + EXPECT_FALSE( + controller.IsCommandIdEnabled(IDS_BOOKMARK_MANAGER_SHOW_IN_FOLDER)); + EXPECT_TRUE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_ADD_NEW_BOOKMARK)); + EXPECT_TRUE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_NEW_FOLDER)); +} + +// Tests the enabled state of the menus when supplied a vector with a single +// url. +TEST_F(BookmarkContextMenuTest, SingleURL) { + std::vector<const BookmarkNode*> nodes; + nodes.push_back(model_->GetBookmarkBarNode()->GetChild(0)); + BookmarkContextMenuController controller( + NULL, NULL, profile_.get(), NULL, nodes[0]->GetParent(), + nodes, BookmarkContextMenuController::BOOKMARK_BAR); + EXPECT_TRUE(controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_ALL)); + EXPECT_TRUE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW)); + EXPECT_TRUE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO)); + EXPECT_TRUE(controller.IsCommandIdEnabled(IDS_BOOKMARK_BAR_REMOVE)); + EXPECT_FALSE( + controller.IsCommandIdEnabled(IDS_BOOKMARK_MANAGER_SHOW_IN_FOLDER)); + EXPECT_TRUE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_ADD_NEW_BOOKMARK)); + EXPECT_TRUE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_NEW_FOLDER)); +} + +// Tests the enabled state of the menus when supplied a vector with multiple +// urls. +TEST_F(BookmarkContextMenuTest, MultipleURLs) { + std::vector<const BookmarkNode*> nodes; + nodes.push_back(model_->GetBookmarkBarNode()->GetChild(0)); + nodes.push_back(model_->GetBookmarkBarNode()->GetChild(1)->GetChild(0)); + BookmarkContextMenuController controller( + NULL, NULL, profile_.get(), NULL, nodes[0]->GetParent(), + nodes, BookmarkContextMenuController::BOOKMARK_BAR); + EXPECT_TRUE(controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_ALL)); + EXPECT_TRUE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW)); + EXPECT_TRUE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO)); + EXPECT_TRUE(controller.IsCommandIdEnabled(IDS_BOOKMARK_BAR_REMOVE)); + EXPECT_FALSE( + controller.IsCommandIdEnabled(IDS_BOOKMARK_MANAGER_SHOW_IN_FOLDER)); + EXPECT_TRUE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_ADD_NEW_BOOKMARK)); + EXPECT_TRUE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_NEW_FOLDER)); +} + +// Tests the enabled state of the menus when supplied an vector with a single +// folder. +TEST_F(BookmarkContextMenuTest, SingleFolder) { + std::vector<const BookmarkNode*> nodes; + nodes.push_back(model_->GetBookmarkBarNode()->GetChild(2)); + BookmarkContextMenuController controller( + NULL, NULL, profile_.get(), NULL, nodes[0]->GetParent(), + nodes, BookmarkContextMenuController::BOOKMARK_BAR); + EXPECT_FALSE(controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_ALL)); + EXPECT_FALSE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW)); + EXPECT_FALSE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO)); + EXPECT_TRUE(controller.IsCommandIdEnabled(IDS_BOOKMARK_BAR_REMOVE)); + EXPECT_FALSE( + controller.IsCommandIdEnabled(IDS_BOOKMARK_MANAGER_SHOW_IN_FOLDER)); + EXPECT_TRUE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_ADD_NEW_BOOKMARK)); + EXPECT_TRUE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_NEW_FOLDER)); +} + +// Tests the enabled state of the menus when supplied a vector with multiple +// folders, all of which are empty. +TEST_F(BookmarkContextMenuTest, MultipleEmptyFolders) { + std::vector<const BookmarkNode*> nodes; + nodes.push_back(model_->GetBookmarkBarNode()->GetChild(2)); + nodes.push_back(model_->GetBookmarkBarNode()->GetChild(3)); + BookmarkContextMenuController controller( + NULL, NULL, profile_.get(), NULL, nodes[0]->GetParent(), + nodes, BookmarkContextMenuController::BOOKMARK_BAR); + EXPECT_FALSE(controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_ALL)); + EXPECT_FALSE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW)); + EXPECT_FALSE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO)); + EXPECT_TRUE(controller.IsCommandIdEnabled(IDS_BOOKMARK_BAR_REMOVE)); + EXPECT_FALSE( + controller.IsCommandIdEnabled(IDS_BOOKMARK_MANAGER_SHOW_IN_FOLDER)); + EXPECT_TRUE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_ADD_NEW_BOOKMARK)); + EXPECT_TRUE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_NEW_FOLDER)); +} + +// Tests the enabled state of the menus when supplied a vector with multiple +// folders, some of which contain URLs. +TEST_F(BookmarkContextMenuTest, MultipleFoldersWithURLs) { + std::vector<const BookmarkNode*> nodes; + nodes.push_back(model_->GetBookmarkBarNode()->GetChild(3)); + nodes.push_back(model_->GetBookmarkBarNode()->GetChild(4)); + BookmarkContextMenuController controller( + NULL, NULL, profile_.get(), NULL, nodes[0]->GetParent(), + nodes, BookmarkContextMenuController::BOOKMARK_BAR); + EXPECT_TRUE(controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_ALL)); + EXPECT_TRUE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW)); + EXPECT_TRUE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO)); + EXPECT_TRUE(controller.IsCommandIdEnabled(IDS_BOOKMARK_BAR_REMOVE)); + EXPECT_FALSE( + controller.IsCommandIdEnabled(IDS_BOOKMARK_MANAGER_SHOW_IN_FOLDER)); + EXPECT_TRUE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_ADD_NEW_BOOKMARK)); + EXPECT_TRUE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_NEW_FOLDER)); +} + +// Tests the enabled state of open incognito. +TEST_F(BookmarkContextMenuTest, DisableIncognito) { + std::vector<const BookmarkNode*> nodes; + nodes.push_back(model_->GetBookmarkBarNode()->GetChild(0)); + BookmarkContextMenuController controller( + NULL, NULL, profile_.get(), NULL, nodes[0]->GetParent(), + nodes, BookmarkContextMenuController::BOOKMARK_BAR); + profile_->set_off_the_record(true); + EXPECT_FALSE(controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_INCOGNITO)); + EXPECT_FALSE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO)); +} + +// Tests that you can't remove/edit when showing the other node. +TEST_F(BookmarkContextMenuTest, DisabledItemsWithOtherNode) { + std::vector<const BookmarkNode*> nodes; + nodes.push_back(model_->other_node()); + BookmarkContextMenuController controller( + NULL, NULL, profile_.get(), NULL, nodes[0], nodes, + BookmarkContextMenuController::BOOKMARK_BAR); + EXPECT_FALSE(controller.IsCommandIdEnabled(IDS_BOOKMARK_BAR_EDIT)); + EXPECT_FALSE(controller.IsCommandIdEnabled(IDS_BOOKMARK_BAR_REMOVE)); +} + +// Tests the enabled state of the menus when supplied an empty vector and null +// parent. +TEST_F(BookmarkContextMenuTest, EmptyNodesNullParent) { + BookmarkContextMenuController controller( + NULL, NULL, profile_.get(), NULL, NULL, + std::vector<const BookmarkNode*>(), + BookmarkContextMenuController::BOOKMARK_MANAGER_ORGANIZE_MENU); + EXPECT_FALSE(controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_ALL)); + EXPECT_FALSE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW)); + EXPECT_FALSE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO)); + EXPECT_FALSE(controller.IsCommandIdEnabled(IDS_BOOKMARK_BAR_REMOVE)); + EXPECT_FALSE( + controller.IsCommandIdEnabled(IDS_BOOKMARK_MANAGER_SHOW_IN_FOLDER)); + EXPECT_FALSE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_ADD_NEW_BOOKMARK)); + EXPECT_FALSE( + controller.IsCommandIdEnabled(IDS_BOOMARK_BAR_NEW_FOLDER)); +} + +TEST_F(BookmarkContextMenuTest, CutCopyPasteNode) { + std::vector<const BookmarkNode*> nodes; + nodes.push_back(model_->GetBookmarkBarNode()->GetChild(0)); + scoped_ptr<BookmarkContextMenuController> controller( + new BookmarkContextMenuController( + NULL, NULL, profile_.get(), NULL, nodes[0]->GetParent(), nodes, + BookmarkContextMenuController::BOOKMARK_BAR)); + EXPECT_TRUE(controller->IsCommandIdEnabled(IDS_COPY)); + EXPECT_TRUE(controller->IsCommandIdEnabled(IDS_CUT)); + + // Copy the URL. + controller->ExecuteCommand(IDS_COPY); + + controller.reset(new BookmarkContextMenuController( + NULL, NULL, profile_.get(), NULL, nodes[0]->GetParent(), nodes, + BookmarkContextMenuController::BOOKMARK_BAR)); + int old_count = model_->GetBookmarkBarNode()->GetChildCount(); + controller->ExecuteCommand(IDS_PASTE); + + ASSERT_TRUE(model_->GetBookmarkBarNode()->GetChild(1)->is_url()); + ASSERT_EQ(old_count + 1, model_->GetBookmarkBarNode()->GetChildCount()); + ASSERT_EQ(model_->GetBookmarkBarNode()->GetChild(0)->GetURL(), + model_->GetBookmarkBarNode()->GetChild(1)->GetURL()); + + controller.reset(new BookmarkContextMenuController( + NULL, NULL, profile_.get(), NULL, nodes[0]->GetParent(), nodes, + BookmarkContextMenuController::BOOKMARK_BAR)); + // Cut the URL. + controller->ExecuteCommand(IDS_CUT); + ASSERT_TRUE(model_->GetBookmarkBarNode()->GetChild(0)->is_url()); + ASSERT_TRUE(model_->GetBookmarkBarNode()->GetChild(1)->is_folder()); + ASSERT_EQ(old_count, model_->GetBookmarkBarNode()->GetChildCount()); +} diff --git a/chrome/browser/gtk/bookmark_bar_gtk.cc b/chrome/browser/gtk/bookmark_bar_gtk.cc index 608abe7..de8e8a2 100644 --- a/chrome/browser/gtk/bookmark_bar_gtk.cc +++ b/chrome/browser/gtk/bookmark_bar_gtk.cc @@ -17,7 +17,6 @@ #include "chrome/browser/bookmarks/bookmark_model.h" #include "chrome/browser/bookmarks/bookmark_utils.h" #include "chrome/browser/browser.h" -#include "chrome/browser/gtk/bookmark_context_menu_gtk.h" #include "chrome/browser/gtk/bookmark_menu_controller_gtk.h" #include "chrome/browser/gtk/bookmark_tree_model.h" #include "chrome/browser/gtk/bookmark_utils_gtk.h" @@ -932,9 +931,9 @@ void BookmarkBarGtk::PopupMenuForNode(GtkWidget* sender, GtkWindow* window = GTK_WINDOW(gtk_widget_get_toplevel(sender)); current_context_menu_controller_.reset( - new BookmarkContextMenuGtk( - window, profile_, browser_, page_navigator_, parent, nodes, - BookmarkContextMenuGtk::BOOKMARK_BAR, NULL)); + new BookmarkContextMenuController( + window, this, profile_, page_navigator_, parent, nodes, + BookmarkContextMenuController::BOOKMARK_BAR)); current_context_menu_.reset( new MenuGtk(NULL, current_context_menu_controller_->menu_model())); current_context_menu_->PopupAsContext(event->time); @@ -1382,3 +1381,7 @@ void BookmarkBarGtk::PopupForButtonNextTo(GtkWidget* button, button_idx = (button_idx + shift + folder_list.size()) % folder_list.size(); PopupForButton(folder_list[button_idx]); } + +void BookmarkBarGtk::CloseMenu() { + current_context_menu_->Cancel(); +} diff --git a/chrome/browser/gtk/bookmark_bar_gtk.h b/chrome/browser/gtk/bookmark_bar_gtk.h index 2770fc5..de0b8c1 100644 --- a/chrome/browser/gtk/bookmark_bar_gtk.h +++ b/chrome/browser/gtk/bookmark_bar_gtk.h @@ -13,6 +13,7 @@ #include "app/slide_animation.h" #include "base/gfx/size.h" #include "base/scoped_ptr.h" +#include "chrome/browser/bookmarks/bookmark_context_menu_controller.h" #include "chrome/browser/bookmarks/bookmark_model_observer.h" #include "chrome/browser/gtk/bookmark_bar_instructions_gtk.h" #include "chrome/browser/gtk/menu_bar_helper.h" @@ -23,7 +24,6 @@ #include "chrome/common/owned_widget_gtk.h" #include "testing/gtest/include/gtest/gtest_prod.h" -class BookmarkContextMenuGtk; class BookmarkMenuController; class Browser; class BrowserWindowGtk; @@ -39,7 +39,8 @@ class BookmarkBarGtk : public AnimationDelegate, public BookmarkModelObserver, public MenuBarHelper::Delegate, public NotificationObserver, - public BookmarkBarInstructionsGtk::Delegate { + public BookmarkBarInstructionsGtk::Delegate, + public BookmarkContextMenuControllerDelegate { FRIEND_TEST(BookmarkBarGtkUnittest, DisplaysHelpMessageOnEmpty); FRIEND_TEST(BookmarkBarGtkUnittest, HidesHelpMessageWithBookmark); FRIEND_TEST(BookmarkBarGtkUnittest, BuildsButtons); @@ -106,6 +107,9 @@ class BookmarkBarGtk : public AnimationDelegate, // The NTP needs to have access to this. static const int kBookmarkBarNTPHeight; + // BookmarkContextMenuController::Delegate implementation -------------------- + virtual void CloseMenu(); + private: // Helper function which generates GtkToolItems for |bookmark_toolbar_|. void CreateAllBookmarkButtons(); @@ -352,7 +356,7 @@ class BookmarkBarGtk : public AnimationDelegate, // The last displayed right click menu, or NULL if no menus have been // displayed yet. // The controller. - scoped_ptr<BookmarkContextMenuGtk> current_context_menu_controller_; + scoped_ptr<BookmarkContextMenuController> current_context_menu_controller_; // The view. scoped_ptr<MenuGtk> current_context_menu_; diff --git a/chrome/browser/gtk/bookmark_manager_gtk.cc b/chrome/browser/gtk/bookmark_manager_gtk.cc index e751301..927436f 100644 --- a/chrome/browser/gtk/bookmark_manager_gtk.cc +++ b/chrome/browser/gtk/bookmark_manager_gtk.cc @@ -632,10 +632,15 @@ void BookmarkManagerGtk::ResetOrganizeMenu(bool left) { if (old_menu) MessageLoop::current()->DeleteSoon(FROM_HERE, old_menu); + BookmarkContextMenuController* old_controller = + organize_menu_controller_.release(); + if (old_controller) + MessageLoop::current()->DeleteSoon(FROM_HERE, old_controller); + organize_menu_controller_.reset( - new BookmarkContextMenuGtk(GTK_WINDOW(window_), profile_, - NULL, NULL, parent, nodes, - BookmarkContextMenuGtk::BOOKMARK_MANAGER_ORGANIZE_MENU, NULL)); + new BookmarkContextMenuController(GTK_WINDOW(window_), this, profile_, + NULL, parent, nodes, + BookmarkContextMenuController::BOOKMARK_MANAGER_ORGANIZE_MENU)); organize_menu_.reset( new MenuGtk(NULL, organize_menu_controller_->menu_model())); gtk_menu_item_set_submenu(GTK_MENU_ITEM(organize_), @@ -1483,6 +1488,10 @@ void BookmarkManagerGtk::OnStateChanged() { UpdateSyncStatus(); } +void BookmarkManagerGtk::CloseMenu() { + organize_menu_->Cancel(); +} + // static gboolean BookmarkManagerGtk::OnGtkAccelerator(GtkAccelGroup* accel_group, GObject* acceleratable, diff --git a/chrome/browser/gtk/bookmark_manager_gtk.h b/chrome/browser/gtk/bookmark_manager_gtk.h index 24db624..ec27be1 100644 --- a/chrome/browser/gtk/bookmark_manager_gtk.h +++ b/chrome/browser/gtk/bookmark_manager_gtk.h @@ -12,8 +12,8 @@ #include "base/gfx/rect.h" #include "base/ref_counted.h" #include "base/task.h" +#include "chrome/browser/bookmarks/bookmark_context_menu_controller.h" #include "chrome/browser/bookmarks/bookmark_model_observer.h" -#include "chrome/browser/gtk/bookmark_context_menu_gtk.h" #include "chrome/browser/sync/profile_sync_service.h" #include "chrome/browser/shell_dialogs.h" #include "chrome/common/gtk_tree.h" @@ -27,7 +27,8 @@ class Profile; class BookmarkManagerGtk : public BookmarkModelObserver, public ProfileSyncServiceObserver, public gtk_tree::TableAdapter::Delegate, - public SelectFileDialog::Listener { + public SelectFileDialog::Listener, + public BookmarkContextMenuControllerDelegate { public: virtual ~BookmarkManagerGtk(); @@ -74,6 +75,9 @@ class BookmarkManagerGtk : public BookmarkModelObserver, // ProfileSyncServiceObserver implementation. virtual void OnStateChanged(); + // BookmarkContextMenuController::Delegate implementation. + virtual void CloseMenu(); + private: friend class BookmarkManagerTest; FRIEND_TEST(BookmarkManagerTest, Crash); @@ -340,7 +344,7 @@ class BookmarkManagerGtk : public BookmarkModelObserver, GtkWidget* organize_; // The submenu the item pops up. // The controller. - scoped_ptr<BookmarkContextMenuGtk> organize_menu_controller_; + scoped_ptr<BookmarkContextMenuController> organize_menu_controller_; // The view. scoped_ptr<MenuGtk> organize_menu_; // Whether the menu refers to the left selection. diff --git a/chrome/browser/gtk/bookmark_menu_controller_gtk.cc b/chrome/browser/gtk/bookmark_menu_controller_gtk.cc index e3bdcb3..a6232ac 100644 --- a/chrome/browser/gtk/bookmark_menu_controller_gtk.cc +++ b/chrome/browser/gtk/bookmark_menu_controller_gtk.cc @@ -12,7 +12,6 @@ #include "app/resource_bundle.h" #include "base/string_util.h" #include "chrome/browser/bookmarks/bookmark_model.h" -#include "chrome/browser/gtk/bookmark_context_menu_gtk.h" #include "chrome/browser/gtk/bookmark_utils_gtk.h" #include "chrome/browser/gtk/gtk_chrome_button.h" #include "chrome/browser/gtk/gtk_theme_provider.h" @@ -106,8 +105,6 @@ BookmarkMenuController::BookmarkMenuController(Browser* browser, } BookmarkMenuController::~BookmarkMenuController() { - if (context_menu_controller_.get()) - context_menu_controller_->DelegateDestroyed(); profile_->GetBookmarkModel()->RemoveObserver(this); gtk_menu_popdown(GTK_MENU(menu_)); } @@ -140,6 +137,10 @@ void BookmarkMenuController::WillExecuteCommand() { gtk_menu_popdown(GTK_MENU(menu_)); } +void BookmarkMenuController::CloseMenu() { + context_menu_->Cancel(); +} + void BookmarkMenuController::NavigateToMenuItem( GtkWidget* menu_item, WindowOpenDisposition disposition) { @@ -251,10 +252,10 @@ gboolean BookmarkMenuController::OnButtonPressed( if (node) nodes.push_back(node); controller->context_menu_controller_.reset( - new BookmarkContextMenuGtk( - controller->parent_window_, controller->profile_, - controller->browser_, controller->page_navigator_, parent, nodes, - BookmarkContextMenuGtk::BOOKMARK_BAR, controller)); + new BookmarkContextMenuController( + controller->parent_window_, controller, controller->profile_, + controller->page_navigator_, parent, nodes, + BookmarkContextMenuController::BOOKMARK_BAR)); controller->context_menu_.reset( new MenuGtk(NULL, controller->context_menu_controller_->menu_model())); diff --git a/chrome/browser/gtk/bookmark_menu_controller_gtk.h b/chrome/browser/gtk/bookmark_menu_controller_gtk.h index fdf098a..b07b658 100644 --- a/chrome/browser/gtk/bookmark_menu_controller_gtk.h +++ b/chrome/browser/gtk/bookmark_menu_controller_gtk.h @@ -11,7 +11,7 @@ #include "base/scoped_ptr.h" #include "chrome/browser/bookmarks/base_bookmark_model_observer.h" -#include "chrome/browser/gtk/bookmark_context_menu_gtk.h" +#include "chrome/browser/bookmarks/bookmark_context_menu_controller.h" #include "chrome/common/owned_widget_gtk.h" #include "webkit/glue/window_open_disposition.h" @@ -24,7 +24,7 @@ class BookmarkNode; class MenuGtk; class BookmarkMenuController : public BaseBookmarkModelObserver, - public BookmarkContextMenuGtk::Delegate { + public BookmarkContextMenuControllerDelegate { public: // Creates a BookmarkMenuController showing the children of |node| starting // at index |start_child_index|. @@ -47,8 +47,9 @@ class BookmarkMenuController : public BaseBookmarkModelObserver, virtual void BookmarkNodeFavIconLoaded(BookmarkModel* model, const BookmarkNode* node); - // Overridden from BookmarkContextMenuGtk::Delegate: + // Overridden from BookmarkContextMenuController::Delegate: virtual void WillExecuteCommand(); + virtual void CloseMenu(); private: // Recursively change the bookmark hierarchy rooted in |parent| into a set of @@ -125,7 +126,7 @@ class BookmarkMenuController : public BaseBookmarkModelObserver, std::map<const BookmarkNode*, GtkWidget*> node_to_menu_widget_map_; // The controller and view for the right click context menu. - scoped_ptr<BookmarkContextMenuGtk> context_menu_controller_; + scoped_ptr<BookmarkContextMenuController> context_menu_controller_; scoped_ptr<MenuGtk> context_menu_; DISALLOW_COPY_AND_ASSIGN(BookmarkMenuController); diff --git a/chrome/browser/views/bookmark_bar_view.cc b/chrome/browser/views/bookmark_bar_view.cc index fce6a7f..644b403 100644 --- a/chrome/browser/views/bookmark_bar_view.cc +++ b/chrome/browser/views/bookmark_bar_view.cc @@ -1255,7 +1255,7 @@ void BookmarkBarView::ShowContextMenu(View* source, browser() ? browser()->GetSelectedTabContents() : NULL; BookmarkContextMenu controller(GetWindow()->GetNativeWindow(), GetProfile(), navigator, parent, nodes, - BookmarkContextMenuController::BOOKMARK_BAR); + BookmarkContextMenuControllerViews::BOOKMARK_BAR); controller.RunMenuAt(gfx::Point(x, y)); } diff --git a/chrome/browser/views/bookmark_context_menu.cc b/chrome/browser/views/bookmark_context_menu.cc index 21eb200..fee7865 100644 --- a/chrome/browser/views/bookmark_context_menu.cc +++ b/chrome/browser/views/bookmark_context_menu.cc @@ -18,9 +18,9 @@ BookmarkContextMenu::BookmarkContextMenu( PageNavigator* page_navigator, const BookmarkNode* parent, const std::vector<const BookmarkNode*>& selection, - BookmarkContextMenuController::ConfigurationType configuration) + BookmarkContextMenuControllerViews::ConfigurationType configuration) : ALLOW_THIS_IN_INITIALIZER_LIST( - controller_(new BookmarkContextMenuController(parent_window, this, + controller_(new BookmarkContextMenuControllerViews(parent_window, this, profile, page_navigator, parent, selection, configuration))), @@ -62,7 +62,8 @@ bool BookmarkContextMenu::ShouldCloseAllMenusOnExecute(int id) { } //////////////////////////////////////////////////////////////////////////////// -// BookmarkContextMenu, BookmarkContextMenuControllerDelegate implementation: +// BookmarkContextMenu, BookmarkContextMenuControllerViewsDelegate +// implementation: void BookmarkContextMenu::CloseMenu() { menu_->Cancel(); diff --git a/chrome/browser/views/bookmark_context_menu.h b/chrome/browser/views/bookmark_context_menu.h index 47b8ade..fe4da5b 100644 --- a/chrome/browser/views/bookmark_context_menu.h +++ b/chrome/browser/views/bookmark_context_menu.h @@ -1,11 +1,11 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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_VIEWS_BOOKMARK_CONTEXT_MENU_H_ #define CHROME_BROWSER_VIEWS_BOOKMARK_CONTEXT_MENU_H_ -#include "chrome/browser/bookmarks/bookmark_context_menu_controller.h" +#include "chrome/browser/views/bookmark_context_menu_controller_views.h" #include "views/controls/menu/menu_delegate.h" // Observer for the BookmarkContextMenu. @@ -19,7 +19,7 @@ class BookmarkContextMenuObserver { virtual void DidRemoveBookmarks() = 0; }; -class BookmarkContextMenu : public BookmarkContextMenuControllerDelegate, +class BookmarkContextMenu : public BookmarkContextMenuControllerViewsDelegate, public views::MenuDelegate { public: BookmarkContextMenu( @@ -28,7 +28,7 @@ class BookmarkContextMenu : public BookmarkContextMenuControllerDelegate, PageNavigator* page_navigator, const BookmarkNode* parent, const std::vector<const BookmarkNode*>& selection, - BookmarkContextMenuController::ConfigurationType configuration); + BookmarkContextMenuControllerViews::ConfigurationType configuration); virtual ~BookmarkContextMenu(); // Shows the context menu at the specified point. @@ -46,7 +46,7 @@ class BookmarkContextMenu : public BookmarkContextMenuControllerDelegate, virtual bool IsCommandEnabled(int command_id) const; virtual bool ShouldCloseAllMenusOnExecute(int id); - // Overridden from BookmarkContextMenuControllerDelegate: + // Overridden from BookmarkContextMenuControllerViewsDelegate: virtual void CloseMenu(); virtual void AddItem(int command_id); virtual void AddItemWithStringId(int command_id, int string_id); @@ -57,7 +57,7 @@ class BookmarkContextMenu : public BookmarkContextMenuControllerDelegate, virtual void DidRemoveBookmarks(); private: - scoped_ptr<BookmarkContextMenuController> controller_; + scoped_ptr<BookmarkContextMenuControllerViews> controller_; // The parent of dialog boxes opened from the context menu. gfx::NativeWindow parent_window_; diff --git a/chrome/browser/gtk/bookmark_context_menu_gtk.cc b/chrome/browser/views/bookmark_context_menu_controller_views.cc index 4c20ff3..343973d 100644 --- a/chrome/browser/gtk/bookmark_context_menu_gtk.cc +++ b/chrome/browser/views/bookmark_context_menu_controller_views.cc @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "chrome/browser/gtk/bookmark_context_menu_gtk.h" +#include "chrome/browser/views/bookmark_context_menu_controller_views.h" #include "app/l10n_util.h" #include "base/compiler_specific.h" @@ -10,13 +10,10 @@ #include "chrome/browser/bookmarks/bookmark_manager.h" #include "chrome/browser/bookmarks/bookmark_model.h" #include "chrome/browser/bookmarks/bookmark_utils.h" -#include "chrome/browser/browser.h" -#include "chrome/browser/browser_list.h" #include "chrome/browser/input_window_dialog.h" #include "chrome/browser/metrics/user_metrics.h" #include "chrome/browser/profile.h" #include "chrome/browser/tab_contents/page_navigator.h" -#include "chrome/browser/tab_contents/tab_contents.h" #include "chrome/common/pref_names.h" #include "chrome/common/pref_service.h" #include "grit/generated_resources.h" @@ -81,7 +78,7 @@ class EditFolderController : public InputWindowDialog::Delegate, l10n_util::GetString(IDS_BOOMARK_BAR_EDIT_FOLDER_LABEL); std::wstring contents = is_new_ ? l10n_util::GetString(IDS_BOOMARK_EDITOR_NEW_FOLDER_NAME) : - node_->GetTitle(); + UTF16ToWide(node_->GetTitleAsString16()); dialog_ = InputWindowDialog::Create(wnd, title, label, contents, this); model_->AddObserver(this); @@ -195,110 +192,94 @@ class SelectOnCreationHandler : public BookmarkEditor::Handler { } // namespace -// BookmarkContextMenuGtk ------------------------------------------- - -BookmarkContextMenuGtk::BookmarkContextMenuGtk( - gfx::NativeWindow wnd, +BookmarkContextMenuControllerViews::BookmarkContextMenuControllerViews( + gfx::NativeWindow parent_window, + BookmarkContextMenuControllerViewsDelegate* delegate, Profile* profile, - Browser* browser, PageNavigator* navigator, const BookmarkNode* parent, const std::vector<const BookmarkNode*>& selection, - ConfigurationType configuration, - Delegate* delegate) - : wnd_(wnd), + ConfigurationType configuration) + : parent_window_(parent_window), + delegate_(delegate), profile_(profile), - browser_(browser), navigator_(navigator), parent_(parent), selection_(selection), - model_(profile->GetBookmarkModel()), configuration_(configuration), - delegate_(delegate), - model_changed_(false) { + model_(profile->GetBookmarkModel()) { DCHECK(profile_); DCHECK(model_->IsLoaded()); - menu_model_.reset(new menus::SimpleMenuModel(this)); - - if (configuration != BOOKMARK_MANAGER_ORGANIZE_MENU) { - if (selection.size() == 1 && selection[0]->is_url()) { - AppendItem(IDS_BOOMARK_BAR_OPEN_ALL, IDS_BOOMARK_BAR_OPEN_IN_NEW_TAB); - AppendItem(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW, - IDS_BOOMARK_BAR_OPEN_IN_NEW_WINDOW); - AppendItem(IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO, - IDS_BOOMARK_BAR_OPEN_INCOGNITO); + model_->AddObserver(this); +} + +BookmarkContextMenuControllerViews::~BookmarkContextMenuControllerViews() { + if (model_) + model_->RemoveObserver(this); +} + +void BookmarkContextMenuControllerViews::BuildMenu() { + if (configuration_ != BOOKMARK_MANAGER_ORGANIZE_MENU) { + if (selection_.size() == 1 && selection_[0]->is_url()) { + delegate_->AddItemWithStringId(IDS_BOOMARK_BAR_OPEN_ALL, + IDS_BOOMARK_BAR_OPEN_IN_NEW_TAB); + delegate_->AddItemWithStringId(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW, + IDS_BOOMARK_BAR_OPEN_IN_NEW_WINDOW); + delegate_->AddItemWithStringId(IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO, + IDS_BOOMARK_BAR_OPEN_INCOGNITO); } else { - AppendItem(IDS_BOOMARK_BAR_OPEN_ALL, IDS_BOOMARK_BAR_OPEN_ALL); - AppendItem(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW, - IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW); - AppendItem(IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO, - IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO); + delegate_->AddItem(IDS_BOOMARK_BAR_OPEN_ALL); + delegate_->AddItem(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW); + delegate_->AddItem(IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO); } - AppendSeparator(); + delegate_->AddSeparator(); } - if (selection.size() == 1 && selection[0]->is_folder()) { - AppendItem(IDS_BOOKMARK_BAR_RENAME_FOLDER); + if (selection_.size() == 1 && selection_[0]->is_folder()) { + delegate_->AddItem(IDS_BOOKMARK_BAR_RENAME_FOLDER); } else { - AppendItem(IDS_BOOKMARK_BAR_EDIT); + delegate_->AddItem(IDS_BOOKMARK_BAR_EDIT); } - if (configuration == BOOKMARK_MANAGER_TABLE || - configuration == BOOKMARK_MANAGER_TABLE_OTHER || - configuration == BOOKMARK_MANAGER_ORGANIZE_MENU || - configuration == BOOKMARK_MANAGER_ORGANIZE_MENU_OTHER) { - AppendItem(IDS_BOOKMARK_MANAGER_SHOW_IN_FOLDER); + if (configuration_ == BOOKMARK_MANAGER_TABLE || + configuration_ == BOOKMARK_MANAGER_TABLE_OTHER || + configuration_ == BOOKMARK_MANAGER_ORGANIZE_MENU || + configuration_ == BOOKMARK_MANAGER_ORGANIZE_MENU_OTHER) { + delegate_->AddItem(IDS_BOOKMARK_MANAGER_SHOW_IN_FOLDER); } - AppendSeparator(); - AppendItem(IDS_CUT); - AppendItem(IDS_COPY); - AppendItem(IDS_PASTE); + delegate_->AddSeparator(); + delegate_->AddItem(IDS_CUT); + delegate_->AddItem(IDS_COPY); + delegate_->AddItem(IDS_PASTE); - AppendSeparator(); - AppendItem(IDS_BOOKMARK_BAR_REMOVE); + delegate_->AddSeparator(); + delegate_->AddItem(IDS_BOOKMARK_BAR_REMOVE); - if (configuration == BOOKMARK_MANAGER_ORGANIZE_MENU) { - AppendSeparator(); - AppendItem(IDS_BOOKMARK_MANAGER_SORT); + if (configuration_ == BOOKMARK_MANAGER_ORGANIZE_MENU) { + delegate_->AddSeparator(); + delegate_->AddItem(IDS_BOOKMARK_MANAGER_SORT); } - AppendSeparator(); + delegate_->AddSeparator(); - AppendItem(IDS_BOOMARK_BAR_ADD_NEW_BOOKMARK); - AppendItem(IDS_BOOMARK_BAR_NEW_FOLDER); + delegate_->AddItem(IDS_BOOMARK_BAR_ADD_NEW_BOOKMARK); + delegate_->AddItem(IDS_BOOMARK_BAR_NEW_FOLDER); - if (configuration == BOOKMARK_BAR) { - AppendSeparator(); - AppendItem(IDS_BOOKMARK_MANAGER); - AppendCheckboxItem(IDS_BOOMARK_BAR_ALWAYS_SHOW); + if (configuration_ == BOOKMARK_BAR) { + delegate_->AddSeparator(); + delegate_->AddItem(IDS_BOOKMARK_MANAGER); + delegate_->AddCheckboxItem(IDS_BOOMARK_BAR_ALWAYS_SHOW); } - - model_->AddObserver(this); } -BookmarkContextMenuGtk::~BookmarkContextMenuGtk() { - if (model_) - model_->RemoveObserver(this); -} - -void BookmarkContextMenuGtk::DelegateDestroyed() { - delegate_ = NULL; -} - -void BookmarkContextMenuGtk::ExecuteCommand(int id) { - if (model_changed_) - return; - - if (delegate_) - delegate_->WillExecuteCommand(); +void BookmarkContextMenuControllerViews::ExecuteCommand(int id) { + BookmarkModel* model = RemoveModelObserver(); switch (id) { case IDS_BOOMARK_BAR_OPEN_ALL: case IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO: case IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW: { - PageNavigator* navigator = browser_ ? - browser_->GetSelectedTabContents() : navigator_; WindowOpenDisposition initial_disposition; if (id == IDS_BOOMARK_BAR_OPEN_ALL) { initial_disposition = NEW_FOREGROUND_TAB; @@ -313,8 +294,7 @@ void BookmarkContextMenuGtk::ExecuteCommand(int id) { UserMetrics::RecordAction("BookmarkBar_ContextMenu_OpenAllIncognito", profile_); } - - bookmark_utils::OpenAll(wnd_, profile_, navigator, selection_, + bookmark_utils::OpenAll(parent_window_, profile_, navigator_, selection_, initial_disposition); break; } @@ -334,23 +314,24 @@ void BookmarkContextMenuGtk::ExecuteCommand(int id) { editor_config = BookmarkEditor::SHOW_TREE; else editor_config = BookmarkEditor::NO_TREE; - BookmarkEditor::Show(wnd_, profile_, parent_, + BookmarkEditor::Show(parent_window_, profile_, parent_, BookmarkEditor::EditDetails(selection_[0]), editor_config, NULL); } else { - EditFolderController::Show(profile_, wnd_, selection_[0], false, - false); + EditFolderController::Show(profile_, parent_window_, selection_[0], + false, false); } break; case IDS_BOOKMARK_BAR_REMOVE: { UserMetrics::RecordAction("BookmarkBar_ContextMenu_Remove", profile_); - BookmarkModel* model = RemoveModelObserver(); + delegate_->WillRemoveBookmarks(selection_); for (size_t i = 0; i < selection_.size(); ++i) { model->Remove(selection_[i]->GetParent(), selection_[i]->GetParent()->IndexOfChild(selection_[i])); } + delegate_->DidRemoveBookmarks(); selection_.clear(); break; } @@ -367,7 +348,7 @@ void BookmarkContextMenuGtk::ExecuteCommand(int id) { // This is owned by the BookmarkEditorView. handler = new SelectOnCreationHandler(profile_); } - BookmarkEditor::Show(wnd_, profile_, GetParentForNewNodes(), + BookmarkEditor::Show(parent_window_, profile_, GetParentForNewNodes(), BookmarkEditor::EditDetails(), editor_config, handler); break; @@ -376,8 +357,9 @@ void BookmarkContextMenuGtk::ExecuteCommand(int id) { case IDS_BOOMARK_BAR_NEW_FOLDER: { UserMetrics::RecordAction("BookmarkBar_ContextMenu_NewFolder", profile_); - EditFolderController::Show(profile_, wnd_, GetParentForNewNodes(), - true, (configuration_ != BOOKMARK_BAR)); + EditFolderController::Show(profile_, parent_window_, + GetParentForNewNodes(), true, + configuration_ != BOOKMARK_BAR); break; } @@ -404,13 +386,17 @@ void BookmarkContextMenuGtk::ExecuteCommand(int id) { case IDS_BOOKMARK_MANAGER_SORT: UserMetrics::RecordAction("BookmarkManager_Sort", profile_); - model_->SortChildren(parent_); + model->SortChildren(parent_); break; - case IDS_COPY: case IDS_CUT: - bookmark_utils::CopyToClipboard(profile_->GetBookmarkModel(), - selection_, id == IDS_CUT); + delegate_->WillRemoveBookmarks(selection_); + bookmark_utils::CopyToClipboard(model, selection_, true); + delegate_->DidRemoveBookmarks(); + break; + + case IDS_COPY: + bookmark_utils::CopyToClipboard(model, selection_, false); break; case IDS_PASTE: { @@ -419,12 +405,10 @@ void BookmarkContextMenuGtk::ExecuteCommand(int id) { return; int index = -1; - if (selection_.size() == 1 && selection_[0]->is_url()) { + if (selection_.size() == 1 && selection_[0]->is_url()) index = paste_target->IndexOfChild(selection_[0]) + 1; - } - bookmark_utils::PasteFromClipboard(profile_->GetBookmarkModel(), - paste_target, index); + bookmark_utils::PasteFromClipboard(model, parent_, index); break; } @@ -433,12 +417,12 @@ void BookmarkContextMenuGtk::ExecuteCommand(int id) { } } -bool BookmarkContextMenuGtk::IsCommandIdChecked(int id) const { +bool BookmarkContextMenuControllerViews::IsItemChecked(int id) const { DCHECK(id == IDS_BOOMARK_BAR_ALWAYS_SHOW); return profile_->GetPrefs()->GetBoolean(prefs::kShowBookmarkBar); } -bool BookmarkContextMenuGtk::IsCommandIdEnabled(int id) const { +bool BookmarkContextMenuControllerViews::IsCommandEnabled(int id) const { bool is_root_node = (selection_.size() == 1 && selection_[0]->GetParent() == model_->root_node()); @@ -478,83 +462,66 @@ bool BookmarkContextMenuGtk::IsCommandIdEnabled(int id) const { case IDS_PASTE: // Paste to selection from the Bookmark Bar, to parent_ everywhere else - return (configuration_ == BOOKMARK_BAR && - !selection_.empty() && + return (configuration_ == BOOKMARK_BAR && !selection_.empty() && bookmark_utils::CanPasteFromClipboard(selection_[0])) || - bookmark_utils::CanPasteFromClipboard(parent_); + bookmark_utils::CanPasteFromClipboard(parent_); } return true; } -bool BookmarkContextMenuGtk::GetAcceleratorForCommandId( - int command_id, - menus::Accelerator* accelerator) { - return false; -} - -void BookmarkContextMenuGtk::BookmarkModelBeingDeleted(BookmarkModel* model) { +void BookmarkContextMenuControllerViews::BookmarkModelBeingDeleted( + BookmarkModel* model) { ModelChanged(); } -void BookmarkContextMenuGtk::BookmarkNodeMoved(BookmarkModel* model, - const BookmarkNode* old_parent, - int old_index, - const BookmarkNode* new_parent, - int new_index) { +void BookmarkContextMenuControllerViews::BookmarkNodeMoved( + BookmarkModel* model, + const BookmarkNode* old_parent, + int old_index, + const BookmarkNode* new_parent, + int new_index) { ModelChanged(); } -void BookmarkContextMenuGtk::BookmarkNodeAdded(BookmarkModel* model, - const BookmarkNode* parent, - int index) { +void BookmarkContextMenuControllerViews::BookmarkNodeAdded( + BookmarkModel* model, + const BookmarkNode* parent, + int index) { ModelChanged(); } -void BookmarkContextMenuGtk::BookmarkNodeRemoved(BookmarkModel* model, - const BookmarkNode* parent, - int index, - const BookmarkNode* node) { +void BookmarkContextMenuControllerViews::BookmarkNodeRemoved( + BookmarkModel* model, + const BookmarkNode* parent, + int index, + const BookmarkNode* node) { ModelChanged(); } -void BookmarkContextMenuGtk::BookmarkNodeChanged(BookmarkModel* model, - const BookmarkNode* node) { +void BookmarkContextMenuControllerViews::BookmarkNodeChanged( + BookmarkModel* model, + const BookmarkNode* node) { ModelChanged(); } -void BookmarkContextMenuGtk::BookmarkNodeChildrenReordered( - BookmarkModel* model, const BookmarkNode* node) { +void BookmarkContextMenuControllerViews::BookmarkNodeChildrenReordered( + BookmarkModel* model, + const BookmarkNode* node) { ModelChanged(); } -void BookmarkContextMenuGtk::ModelChanged() { - model_changed_ = true; -} - -void BookmarkContextMenuGtk::AppendItem(int id) { - menu_model_->AddItem(id, l10n_util::GetStringUTF16(id)); -} - -void BookmarkContextMenuGtk::AppendItem(int id, int localization_id) { - menu_model_->AddItemWithStringId(id, localization_id); -} - -void BookmarkContextMenuGtk::AppendSeparator() { - menu_model_->AddSeparator(); -} - -void BookmarkContextMenuGtk::AppendCheckboxItem(int id) { - menu_model_->AddCheckItemWithStringId(id, id); +void BookmarkContextMenuControllerViews::ModelChanged() { + delegate_->CloseMenu(); } -BookmarkModel* BookmarkContextMenuGtk::RemoveModelObserver() { +BookmarkModel* BookmarkContextMenuControllerViews::RemoveModelObserver() { BookmarkModel* model = model_; model_->RemoveObserver(this); model_ = NULL; return model; } -bool BookmarkContextMenuGtk::HasURLs() const { +bool BookmarkContextMenuControllerViews::HasURLs() const { for (size_t i = 0; i < selection_.size(); ++i) { if (NodeHasURLs(selection_[i])) return true; @@ -562,7 +529,8 @@ bool BookmarkContextMenuGtk::HasURLs() const { return false; } -const BookmarkNode* BookmarkContextMenuGtk::GetParentForNewNodes() const { +const BookmarkNode* + BookmarkContextMenuControllerViews::GetParentForNewNodes() const { return (selection_.size() == 1 && selection_[0]->is_folder()) ? selection_[0] : parent_; } diff --git a/chrome/browser/gtk/bookmark_context_menu_gtk.h b/chrome/browser/views/bookmark_context_menu_controller_views.h index a625611..441b666 100644 --- a/chrome/browser/gtk/bookmark_context_menu_gtk.h +++ b/chrome/browser/views/bookmark_context_menu_controller_views.h @@ -2,26 +2,45 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef CHROME_BROWSER_GTK_BOOKMARK_CONTEXT_MENU_GTK_H_ -#define CHROME_BROWSER_GTK_BOOKMARK_CONTEXT_MENU_GTK_H_ +#ifndef CHROME_BROWSER_VIEWS_BOOKMARK_CONTEXT_MENU_CONTROLLER_VIEWS_H_ +#define CHROME_BROWSER_VIEWS_BOOKMARK_CONTEXT_MENU_CONTROLLER_VIEWS_H_ #include <vector> #include "app/gfx/native_widget_types.h" -#include "app/menus/simple_menu_model.h" #include "base/basictypes.h" -#include "base/scoped_ptr.h" #include "chrome/browser/bookmarks/bookmark_model_observer.h" class Browser; class PageNavigator; class Profile; -// BookmarkContextMenu manages the context menu shown for the -// bookmark bar, items on the bookmark bar, submenus of the bookmark bar and -// the bookmark manager. -class BookmarkContextMenuGtk : public BookmarkModelObserver, - public menus::SimpleMenuModel::Delegate { +// An interface implemented by an object that performs actions on the actual +// menu for the controller. +class BookmarkContextMenuControllerViewsDelegate { + public: + virtual ~BookmarkContextMenuControllerViewsDelegate() {} + + // Closes the bookmark context menu. + virtual void CloseMenu() = 0; + + // Methods that add items to the underlying menu. + virtual void AddItem(int command_id) = 0; + virtual void AddItemWithStringId(int command_id, int string_id) = 0; + virtual void AddSeparator() = 0; + virtual void AddCheckboxItem(int command_id) = 0; + + // Sent before bookmarks are removed. + virtual void WillRemoveBookmarks( + const std::vector<const BookmarkNode*>& bookmarks) {} + + // Sent after bookmarks have been removed. + virtual void DidRemoveBookmarks() {} +}; + +// BookmarkContextMenuControllerViews creates and manages state for the context +// menu shown for any bookmark item. +class BookmarkContextMenuControllerViews : public BookmarkModelObserver { public: // Used to configure what the context menu shows. enum ConfigurationType { @@ -37,12 +56,6 @@ class BookmarkContextMenuGtk : public BookmarkModelObserver, BOOKMARK_MANAGER_ORGANIZE_MENU_OTHER }; - class Delegate { - public: - // Called when one of the menu items is selected and executed. - virtual void WillExecuteCommand() = 0; - }; - // Creates the bookmark context menu. // |profile| is used for opening urls as well as enabling 'open incognito'. // |browser| is used to determine the PageNavigator and may be null. @@ -50,31 +63,28 @@ class BookmarkContextMenuGtk : public BookmarkModelObserver, // |parent| is the parent for newly created nodes if |selection| is empty. // |selection| is the nodes the context menu operates on and may be empty. // |configuration| determines which items to show. - BookmarkContextMenuGtk(gfx::NativeWindow hwnd, - Profile* profile, - Browser* browser, - PageNavigator* navigator, - const BookmarkNode* parent, - const std::vector<const BookmarkNode*>& selection, - ConfigurationType configuration, - Delegate* delegate); - virtual ~BookmarkContextMenuGtk(); - - menus::MenuModel* menu_model() const { return menu_model_.get(); } - - // Should be called by the delegate when it is no longer valid. - void DelegateDestroyed(); - - // Menu::Delegate methods. - virtual bool IsCommandIdChecked(int command_id) const; - virtual bool IsCommandIdEnabled(int command_id) const; - virtual bool GetAcceleratorForCommandId( - int command_id, - menus::Accelerator* accelerator); - virtual void ExecuteCommand(int command_id); + BookmarkContextMenuControllerViews( + gfx::NativeWindow parent_window, + BookmarkContextMenuControllerViewsDelegate* delegate, + Profile* profile, + PageNavigator* navigator, + const BookmarkNode* parent, + const std::vector<const BookmarkNode*>& selection, + ConfigurationType configuration); + virtual ~BookmarkContextMenuControllerViews(); + + void BuildMenu(); + + void ExecuteCommand(int id); + bool IsItemChecked(int id) const; + bool IsCommandEnabled(int id) const; + + // Accessors: + Profile* profile() const { return profile_; } + PageNavigator* navigator() const { return navigator_; } private: - // BookmarkModelObserver method. Any change to the model results in closing + // BookmarkModelObserver methods. Any change to the model results in closing // the menu. virtual void Loaded(BookmarkModel* model) {} virtual void BookmarkModelBeingDeleted(BookmarkModel* model); @@ -100,15 +110,6 @@ class BookmarkContextMenuGtk : public BookmarkModelObserver, // Invoked from the various bookmark model observer methods. Closes the menu. void ModelChanged(); - // Adds a IDS_* style command to the menu. - void AppendItem(int id); - // Adds a IDS_* style command to the menu with a different localized string. - void AppendItem(int id, int localization_id); - // Adds a separator to the menu. - void AppendSeparator(); - // Adds a checkable item to the menu. - void AppendCheckboxItem(int id); - // Removes the observer from the model and NULLs out model_. BookmarkModel* RemoveModelObserver(); @@ -120,23 +121,16 @@ class BookmarkContextMenuGtk : public BookmarkModelObserver, // parent_ is returned. const BookmarkNode* GetParentForNewNodes() const; - gfx::NativeWindow wnd_; + gfx::NativeWindow parent_window_; + BookmarkContextMenuControllerViewsDelegate* delegate_; Profile* profile_; - Browser* browser_; PageNavigator* navigator_; const BookmarkNode* parent_; std::vector<const BookmarkNode*> selection_; - BookmarkModel* model_; ConfigurationType configuration_; - Delegate* delegate_; - scoped_ptr<menus::SimpleMenuModel> menu_model_; - - // Tracks whether the model has changed. For the most part the model won't - // change while a context menu is showing, but if it does, we'd better not - // try to execute any commands. - bool model_changed_; + BookmarkModel* model_; - DISALLOW_COPY_AND_ASSIGN(BookmarkContextMenuGtk); + DISALLOW_COPY_AND_ASSIGN(BookmarkContextMenuControllerViews); }; -#endif // CHROME_BROWSER_GTK_BOOKMARK_CONTEXT_MENU_GTK_H_ +#endif // CHROME_BROWSER_VIEWS_BOOKMARK_CONTEXT_MENU_CONTROLLER_VIEWS_H_ diff --git a/chrome/browser/views/bookmark_context_menu_test.cc b/chrome/browser/views/bookmark_context_menu_test.cc index e0a3936..9082324 100644 --- a/chrome/browser/views/bookmark_context_menu_test.cc +++ b/chrome/browser/views/bookmark_context_menu_test.cc @@ -111,7 +111,7 @@ TEST_F(BookmarkContextMenuTest, DeleteURL) { nodes.push_back(model_->GetBookmarkBarNode()->GetChild(0)); BookmarkContextMenu controller( NULL, profile_.get(), NULL, nodes[0]->GetParent(), nodes, - BookmarkContextMenuController::BOOKMARK_BAR); + BookmarkContextMenuControllerViews::BOOKMARK_BAR); GURL url = model_->GetBookmarkBarNode()->GetChild(0)->GetURL(); ASSERT_TRUE(controller.IsCommandEnabled(IDS_BOOKMARK_BAR_REMOVE)); // Delete the URL. @@ -138,7 +138,7 @@ TEST_F(BookmarkContextMenuTest, EmptyNodes) { BookmarkContextMenu controller( NULL, profile_.get(), NULL, model_->other_node(), std::vector<const BookmarkNode*>(), - BookmarkContextMenuController::BOOKMARK_BAR); + BookmarkContextMenuControllerViews::BOOKMARK_BAR); EXPECT_FALSE(controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL)); EXPECT_FALSE( controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW)); @@ -159,7 +159,7 @@ TEST_F(BookmarkContextMenuTest, SingleURL) { nodes.push_back(model_->GetBookmarkBarNode()->GetChild(0)); BookmarkContextMenu controller( NULL, profile_.get(), NULL, nodes[0]->GetParent(), - nodes, BookmarkContextMenuController::BOOKMARK_BAR); + nodes, BookmarkContextMenuControllerViews::BOOKMARK_BAR); EXPECT_TRUE(controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL)); EXPECT_TRUE( controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW)); @@ -181,7 +181,7 @@ TEST_F(BookmarkContextMenuTest, MultipleURLs) { nodes.push_back(model_->GetBookmarkBarNode()->GetChild(1)->GetChild(0)); BookmarkContextMenu controller( NULL, profile_.get(), NULL, nodes[0]->GetParent(), - nodes, BookmarkContextMenuController::BOOKMARK_BAR); + nodes, BookmarkContextMenuControllerViews::BOOKMARK_BAR); EXPECT_TRUE(controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL)); EXPECT_TRUE( controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW)); @@ -202,7 +202,7 @@ TEST_F(BookmarkContextMenuTest, SingleFolder) { nodes.push_back(model_->GetBookmarkBarNode()->GetChild(2)); BookmarkContextMenu controller( NULL, profile_.get(), NULL, nodes[0]->GetParent(), - nodes, BookmarkContextMenuController::BOOKMARK_BAR); + nodes, BookmarkContextMenuControllerViews::BOOKMARK_BAR); EXPECT_FALSE(controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL)); EXPECT_FALSE( controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW)); @@ -224,7 +224,7 @@ TEST_F(BookmarkContextMenuTest, MultipleEmptyFolders) { nodes.push_back(model_->GetBookmarkBarNode()->GetChild(3)); BookmarkContextMenu controller( NULL, profile_.get(), NULL, nodes[0]->GetParent(), - nodes, BookmarkContextMenuController::BOOKMARK_BAR); + nodes, BookmarkContextMenuControllerViews::BOOKMARK_BAR); EXPECT_FALSE(controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL)); EXPECT_FALSE( controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW)); @@ -246,7 +246,7 @@ TEST_F(BookmarkContextMenuTest, MultipleFoldersWithURLs) { nodes.push_back(model_->GetBookmarkBarNode()->GetChild(4)); BookmarkContextMenu controller( NULL, profile_.get(), NULL, nodes[0]->GetParent(), - nodes, BookmarkContextMenuController::BOOKMARK_BAR); + nodes, BookmarkContextMenuControllerViews::BOOKMARK_BAR); EXPECT_TRUE(controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL)); EXPECT_TRUE( controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW)); @@ -266,7 +266,7 @@ TEST_F(BookmarkContextMenuTest, DisableIncognito) { nodes.push_back(model_->GetBookmarkBarNode()->GetChild(0)); BookmarkContextMenu controller( NULL, profile_.get(), NULL, nodes[0]->GetParent(), - nodes, BookmarkContextMenuController::BOOKMARK_BAR); + nodes, BookmarkContextMenuControllerViews::BOOKMARK_BAR); profile_->set_off_the_record(true); EXPECT_FALSE(controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_INCOGNITO)); EXPECT_FALSE(controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO)); @@ -278,7 +278,7 @@ TEST_F(BookmarkContextMenuTest, DisabledItemsWithOtherNode) { nodes.push_back(model_->other_node()); BookmarkContextMenu controller( NULL, profile_.get(), NULL, nodes[0], nodes, - BookmarkContextMenuController::BOOKMARK_BAR); + BookmarkContextMenuControllerViews::BOOKMARK_BAR); EXPECT_FALSE(controller.IsCommandEnabled(IDS_BOOKMARK_BAR_EDIT)); EXPECT_FALSE(controller.IsCommandEnabled(IDS_BOOKMARK_BAR_REMOVE)); } @@ -288,7 +288,7 @@ TEST_F(BookmarkContextMenuTest, DisabledItemsWithOtherNode) { TEST_F(BookmarkContextMenuTest, EmptyNodesNullParent) { BookmarkContextMenu controller( NULL, profile_.get(), NULL, NULL, std::vector<const BookmarkNode*>(), - BookmarkContextMenuController::BOOKMARK_MANAGER_ORGANIZE_MENU); + BookmarkContextMenuControllerViews::BOOKMARK_MANAGER_ORGANIZE_MENU); EXPECT_FALSE(controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL)); EXPECT_FALSE( controller.IsCommandEnabled(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW)); @@ -307,7 +307,7 @@ TEST_F(BookmarkContextMenuTest, CutCopyPasteNode) { nodes.push_back(model_->GetBookmarkBarNode()->GetChild(0)); scoped_ptr<BookmarkContextMenu> controller(new BookmarkContextMenu( NULL, profile_.get(), NULL, nodes[0]->GetParent(), nodes, - BookmarkContextMenuController::BOOKMARK_BAR)); + BookmarkContextMenuControllerViews::BOOKMARK_BAR)); EXPECT_TRUE(controller->IsCommandEnabled(IDS_COPY)); EXPECT_TRUE(controller->IsCommandEnabled(IDS_CUT)); @@ -316,7 +316,7 @@ TEST_F(BookmarkContextMenuTest, CutCopyPasteNode) { controller.reset(new BookmarkContextMenu( NULL, profile_.get(), NULL, nodes[0]->GetParent(), nodes, - BookmarkContextMenuController::BOOKMARK_BAR)); + BookmarkContextMenuControllerViews::BOOKMARK_BAR)); int old_count = model_->GetBookmarkBarNode()->GetChildCount(); controller->ExecuteCommand(IDS_PASTE); @@ -327,7 +327,7 @@ TEST_F(BookmarkContextMenuTest, CutCopyPasteNode) { controller.reset(new BookmarkContextMenu( NULL, profile_.get(), NULL, nodes[0]->GetParent(), nodes, - BookmarkContextMenuController::BOOKMARK_BAR)); + BookmarkContextMenuControllerViews::BOOKMARK_BAR)); // Cut the URL. controller->ExecuteCommand(IDS_CUT); ASSERT_TRUE(model_->GetBookmarkBarNode()->GetChild(0)->is_url()); diff --git a/chrome/browser/views/bookmark_manager_view.cc b/chrome/browser/views/bookmark_manager_view.cc index 4e17e96..18e9fb2 100644 --- a/chrome/browser/views/bookmark_manager_view.cc +++ b/chrome/browser/views/bookmark_manager_view.cc @@ -556,8 +556,9 @@ void BookmarkManagerView::ShowContextMenu(views::View* source, DCHECK(source == table_view_ || source == tree_view_); bool is_table = (source == table_view_); ShowMenu(x, y, - is_table ? BookmarkContextMenuController::BOOKMARK_MANAGER_TABLE : - BookmarkContextMenuController::BOOKMARK_MANAGER_TREE); + is_table ? + BookmarkContextMenuControllerViews::BOOKMARK_MANAGER_TABLE : + BookmarkContextMenuControllerViews::BOOKMARK_MANAGER_TREE); } void BookmarkManagerView::RunMenu(views::View* source, const gfx::Point& pt) { @@ -572,7 +573,7 @@ void BookmarkManagerView::RunMenu(views::View* source, const gfx::Point& pt) { (-source->width() + 5); if (source->GetID() == kOrganizeMenuButtonID) { ShowMenu(menu_x, pt.y() + 2, - BookmarkContextMenuController::BOOKMARK_MANAGER_ORGANIZE_MENU); + BookmarkContextMenuControllerViews::BOOKMARK_MANAGER_ORGANIZE_MENU); } else if (source->GetID() == kToolsMenuButtonID) { ShowToolsMenu(menu_x, pt.y() + 2); } else { @@ -717,22 +718,26 @@ BookmarkModel* BookmarkManagerView::GetBookmarkModel() const { } void BookmarkManagerView::ShowMenu( - int x, int y, BookmarkContextMenuController::ConfigurationType config) { + int x, int y, + BookmarkContextMenuControllerViews::ConfigurationType config) { if (!GetBookmarkModel()->IsLoaded()) return; - if (config == BookmarkContextMenuController::BOOKMARK_MANAGER_TABLE || + if (config == BookmarkContextMenuControllerViews::BOOKMARK_MANAGER_TABLE || (config == - BookmarkContextMenuController::BOOKMARK_MANAGER_ORGANIZE_MENU && + BookmarkContextMenuControllerViews::BOOKMARK_MANAGER_ORGANIZE_MENU && table_view_->HasFocus())) { std::vector<const BookmarkNode*> nodes = GetSelectedTableNodes(); const BookmarkNode* parent = GetSelectedFolder(); if (!parent) { - if (config == BookmarkContextMenuController::BOOKMARK_MANAGER_TABLE) { - config = BookmarkContextMenuController::BOOKMARK_MANAGER_TABLE_OTHER; + if (config == + BookmarkContextMenuControllerViews::BOOKMARK_MANAGER_TABLE) { + config = + BookmarkContextMenuControllerViews::BOOKMARK_MANAGER_TABLE_OTHER; } else { config = - BookmarkContextMenuController::BOOKMARK_MANAGER_ORGANIZE_MENU_OTHER; + BookmarkContextMenuControllerViews:: + BOOKMARK_MANAGER_ORGANIZE_MENU_OTHER; } } BookmarkContextMenu menu(GetWindow()->GetNativeWindow(), profile_, NULL, diff --git a/chrome/browser/views/bookmark_manager_view.h b/chrome/browser/views/bookmark_manager_view.h index 870d89b..74aee84 100644 --- a/chrome/browser/views/bookmark_manager_view.h +++ b/chrome/browser/views/bookmark_manager_view.h @@ -194,7 +194,7 @@ class BookmarkManagerView : public views::View, // Shows the menu. This is invoked to show the context menu for table/tree // as well as to show the menu from the organize button. void ShowMenu(int x, int y, - BookmarkContextMenuController::ConfigurationType config); + BookmarkContextMenuControllerViews::ConfigurationType config); // Invoked to handle cut/copy/paste from the table or tree. If |from_table| // is true the source is the table. diff --git a/chrome/browser/views/bookmark_menu_controller_views.cc b/chrome/browser/views/bookmark_menu_controller_views.cc index cb334ed..529eeee 100644 --- a/chrome/browser/views/bookmark_menu_controller_views.cc +++ b/chrome/browser/views/bookmark_menu_controller_views.cc @@ -200,12 +200,13 @@ bool BookmarkMenuController::ShowContextMenu(MenuItemView* source, std::vector<const BookmarkNode*> nodes; nodes.push_back(menu_id_to_node_map_[id]); context_menu_.reset( - new BookmarkContextMenu(parent_, - profile_, - page_navigator_, - nodes[0]->GetParent(), - nodes, - BookmarkContextMenuController::BOOKMARK_BAR)); + new BookmarkContextMenu( + parent_, + profile_, + page_navigator_, + nodes[0]->GetParent(), + nodes, + BookmarkContextMenuControllerViews::BOOKMARK_BAR)); context_menu_->set_observer(this); context_menu_->RunMenuAt(gfx::Point(x, y)); context_menu_.reset(NULL); diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 9a4a831..ca123dd 100755 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -922,8 +922,6 @@ 'browser/gtk/bookmark_bar_instructions_gtk.h', 'browser/gtk/bookmark_bubble_gtk.cc', 'browser/gtk/bookmark_bubble_gtk.h', - 'browser/gtk/bookmark_context_menu_gtk.cc', - 'browser/gtk/bookmark_context_menu_gtk.h', 'browser/gtk/bookmark_editor_gtk.cc', 'browser/gtk/bookmark_editor_gtk.h', 'browser/gtk/bookmark_manager_gtk.cc', @@ -1755,6 +1753,8 @@ 'browser/views/bookmark_bubble_view.h', 'browser/views/bookmark_context_menu.cc', 'browser/views/bookmark_context_menu.h', + 'browser/views/bookmark_context_menu_controller_views.cc', + 'browser/views/bookmark_context_menu_controller_views.h', 'browser/views/bookmark_editor_view.cc', 'browser/views/bookmark_editor_view.h', 'browser/views/bookmark_folder_tree_view.cc', @@ -2364,6 +2364,8 @@ ['include', '^browser/views/bookmark_bubble_view.h'], ['include', '^browser/views/bookmark_context_menu.cc'], ['include', '^browser/views/bookmark_context_menu.h'], + ['include', '^browser/views/bookmark_context_menu_controller_views.cc'], + ['include', '^browser/views/bookmark_context_menu_controller_views.h'], ['include', '^browser/views/bookmark_menu_button.cc'], ['include', '^browser/views/bookmark_menu_button.h'], ['include', '^browser/views/bookmark_menu_controller_views.cc'], |