summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--chrome/browser/bookmarks/bookmark_utils.cc8
-rw-r--r--chrome/browser/bookmarks/bookmark_utils.h7
-rw-r--r--chrome/browser/views/bookmark_bar_view_test.cc67
-rw-r--r--chrome/views/chrome_menu.cc56
-rw-r--r--chrome/views/chrome_menu.h29
5 files changed, 124 insertions, 43 deletions
diff --git a/chrome/browser/bookmarks/bookmark_utils.cc b/chrome/browser/bookmarks/bookmark_utils.cc
index 22f9e47..9e0b7c8 100644
--- a/chrome/browser/bookmarks/bookmark_utils.cc
+++ b/chrome/browser/bookmarks/bookmark_utils.cc
@@ -20,10 +20,6 @@
namespace {
-// Number of bookmarks we'll open before prompting the user to see if they
-// really want to open all.
-const int kNumURLsBeforePrompting = 15;
-
// A PageNavigator implementation that creates a new Browser. This is used when
// opening a url and there is no Browser open. The Browser is created the first
// time the PageNavigator method is invoked.
@@ -131,7 +127,7 @@ bool ShouldOpenAll(HWND parent, const std::vector<BookmarkNode*>& nodes) {
int descendant_count = 0;
for (size_t i = 0; i < nodes.size(); ++i)
descendant_count += DescendantURLCount(nodes[i]);
- if (descendant_count < kNumURLsBeforePrompting)
+ if (descendant_count < bookmark_utils::num_urls_before_prompting)
return true;
std::wstring message =
@@ -146,6 +142,8 @@ bool ShouldOpenAll(HWND parent, const std::vector<BookmarkNode*>& nodes) {
namespace bookmark_utils {
+int num_urls_before_prompting = 15;
+
int PreferredDropOperation(int source_operations, int operations) {
int common_ops = (source_operations & operations);
if (!common_ops)
diff --git a/chrome/browser/bookmarks/bookmark_utils.h b/chrome/browser/bookmarks/bookmark_utils.h
index 3a72fbb..1d92e3e 100644
--- a/chrome/browser/bookmarks/bookmark_utils.h
+++ b/chrome/browser/bookmarks/bookmark_utils.h
@@ -75,6 +75,13 @@ void PasteFromClipboard(BookmarkModel* model,
// Returns true if the user can copy from the pasteboard.
bool CanPasteFromClipboard(BookmarkNode* node);
+// Number of bookmarks we'll open before prompting the user to see if they
+// really want to open all.
+//
+// NOTE: treat this as a const. It is not const as various tests change the
+// value.
+extern int num_urls_before_prompting;
+
} // namespace bookmark_utils
#endif // CHROME_BROWSER_BOOKMARKS_BOOKMARK_UTILS_H_
diff --git a/chrome/browser/views/bookmark_bar_view_test.cc b/chrome/browser/views/bookmark_bar_view_test.cc
index 94d9a9c..8d31eb1 100644
--- a/chrome/browser/views/bookmark_bar_view_test.cc
+++ b/chrome/browser/views/bookmark_bar_view_test.cc
@@ -5,6 +5,7 @@
#include "base/string_util.h"
#include "chrome/browser/automation/ui_controls.h"
#include "chrome/browser/bookmarks/bookmark_model.h"
+#include "chrome/browser/bookmarks/bookmark_utils.h"
#include "chrome/browser/page_navigator.h"
#include "chrome/browser/profile.h"
#include "chrome/browser/views/bookmark_bar_view.h"
@@ -892,3 +893,69 @@ class BookmarkBarViewTest11 : public BookmarkBarViewEventTestBase {
};
VIEW_TEST(BookmarkBarViewTest11, CloseMenuAfterClosingContextMenu)
+
+// Tests showing a modal dialog from a context menu.
+class BookmarkBarViewTest12 : public BookmarkBarViewEventTestBase {
+ protected:
+ virtual void DoTestOnMessageLoop() {
+ // Open up the other folder.
+ views::TextButton* button = bb_view_->other_bookmarked_button();
+ ui_controls::MoveMouseToCenterAndPress(button, ui_controls::LEFT,
+ ui_controls::DOWN | ui_controls::UP,
+ CreateEventTask(this, &BookmarkBarViewTest12::Step2));
+ bookmark_utils::num_urls_before_prompting = 1;
+ }
+
+ ~BookmarkBarViewTest12() {
+ bookmark_utils::num_urls_before_prompting = 15;
+ }
+
+ private:
+ void Step2() {
+ // Menu should be showing.
+ views::MenuItemView* menu = bb_view_->GetMenu();
+ ASSERT_TRUE(menu != NULL);
+ ASSERT_TRUE(menu->GetSubmenu()->IsShowing());
+
+ views::MenuItemView* child_menu =
+ menu->GetSubmenu()->GetMenuItemAt(1);
+ ASSERT_TRUE(child_menu != NULL);
+
+ // Right click on the second child (a folder) to get its context menu.
+ ui_controls::MoveMouseToCenterAndPress(child_menu, ui_controls::RIGHT,
+ ui_controls::DOWN | ui_controls::UP,
+ CreateEventTask(this, &BookmarkBarViewTest12::Step3));
+ }
+
+ void Step3() {
+ // Make sure the context menu is showing.
+ views::MenuItemView* menu = bb_view_->GetContextMenu();
+ ASSERT_TRUE(menu && menu->GetSubmenu() && menu->GetSubmenu()->IsShowing());
+
+ // Select the first item in the context menu (open all).
+ views::MenuItemView* child_menu =
+ menu->GetSubmenu()->GetMenuItemAt(0);
+ ASSERT_TRUE(child_menu != NULL);
+ ui_controls::MoveMouseToCenterAndPress(child_menu, ui_controls::LEFT,
+ ui_controls::DOWN | ui_controls::UP, NULL);
+
+ // Press tab to give focus to the cancel button.
+ ui_controls::SendKeyPressNotifyWhenDone(VK_TAB, false, false, false,
+ NULL);
+ // And press enter so that the cancel button is selected.
+ ui_controls::SendKeyPressNotifyWhenDone(VK_RETURN, false, false, false,
+ CreateEventTask(this, &BookmarkBarViewTest12::Step4));
+ }
+
+ void Step4() {
+ // Do a delayed task to give the dialog time to exit.
+ MessageLoop::current()->PostTask(
+ FROM_HERE, CreateEventTask(this, &BookmarkBarViewTest12::Step5));
+ }
+
+ void Step5() {
+ Done();
+ }
+};
+
+VIEW_TEST(BookmarkBarViewTest12, CloseWithModalDialog)
diff --git a/chrome/views/chrome_menu.cc b/chrome/views/chrome_menu.cc
index 62879cd..ffa4891 100644
--- a/chrome/views/chrome_menu.cc
+++ b/chrome/views/chrome_menu.cc
@@ -586,12 +586,12 @@ class MenuHostRootView : public RootView {
RootView::OnMouseReleased(event, canceled);
if (forward_drag_to_menu_controller_) {
+ forward_drag_to_menu_controller_ = false;
if (canceled) {
GetMenuController()->Cancel(true);
} else {
GetMenuController()->OnMouseReleased(submenu_, event);
}
- forward_drag_to_menu_controller_ = false;
}
}
@@ -702,6 +702,12 @@ class MenuHost : public ContainerWin {
ContainerWin::Hide();
}
+ virtual void HideWindow() {
+ // Make sure we release capture before hiding.
+ ReleaseCapture();
+ ContainerWin::Hide();
+ }
+
virtual void OnCaptureChanged(HWND hwnd) {
ContainerWin::OnCaptureChanged(hwnd);
owns_capture_ = false;
@@ -795,7 +801,10 @@ SubmenuView::SubmenuView(MenuItemView* parent)
}
SubmenuView::~SubmenuView() {
- DCHECK(!host_);
+ // The menu may not have been closed yet (it will be hidden, but not
+ // necessarily closed).
+ Close();
+
delete scroll_view_container_;
}
@@ -949,10 +958,18 @@ bool SubmenuView::OnMouseWheel(const MouseWheelEvent& e) {
return true;
}
+bool SubmenuView::IsShowing() {
+ return host_ && host_->IsVisible();
+}
+
void SubmenuView::ShowAt(HWND parent,
const gfx::Rect& bounds,
bool do_capture) {
- DCHECK(!host_);
+ if (host_) {
+ host_->ShowWindow(SW_SHOWNA);
+ return;
+ }
+
host_ = new MenuHost(this);
// Force construction of the scroll view container.
GetScrollViewContainer();
@@ -961,17 +978,18 @@ void SubmenuView::ShowAt(HWND parent,
host_->Init(parent, bounds, scroll_view_container_, do_capture);
}
-void SubmenuView::Close(bool destroy_host) {
+void SubmenuView::Close() {
if (host_) {
- if (destroy_host) {
- host_->Close();
- host_ = NULL;
- } else {
- host_->Hide();
- }
+ host_->Close();
+ host_ = NULL;
}
}
+void SubmenuView::Hide() {
+ if (host_)
+ host_->HideWindow();
+}
+
void SubmenuView::ReleaseCapture() {
host_->ReleaseCapture();
}
@@ -1502,9 +1520,9 @@ void MenuItemView::DestroyAllMenuHosts() {
if (!HasSubmenu())
return;
- submenu_->Close(true);
+ submenu_->Close();
for (int i = 0, item_count = submenu_->GetMenuItemCount(); i < item_count;
- ++i) {
+ ++i) {
submenu_->GetMenuItemAt(i)->DestroyAllMenuHosts();
}
}
@@ -1784,7 +1802,6 @@ void MenuController::OnMouseDragged(SubmenuView* source,
gfx::Point drag_loc(event.location());
View::ConvertPointToScreen(source->GetScrollViewContainer(), &drag_loc);
View::ConvertPointToView(NULL, item, &drag_loc);
- in_drag_ = true;
ChromeCanvas canvas(item->width(), item->height(), false);
item->Paint(&canvas, true);
@@ -2166,7 +2183,6 @@ MenuController::MenuController(bool blocking)
owner_(NULL),
possible_drag_(false),
valid_drop_coordinates_(false),
- in_drag_(false),
any_menu_contains_mouse_(false),
showing_submenu_(false),
result_mouse_event_flags_(0) {
@@ -2323,9 +2339,7 @@ void MenuController::CommitPendingSelection() {
// Hide the old menu.
for (size_t i = paths_differ_at; i < current_path.size(); ++i) {
if (current_path[i]->HasSubmenu()) {
- // See description of in_drag_ as to why close is conditionalized like
- // this.
- current_path[i]->GetSubmenu()->Close(!in_drag_);
+ current_path[i]->GetSubmenu()->Hide();
}
}
@@ -2370,10 +2384,7 @@ void MenuController::CommitPendingSelection() {
}
} else if (state_.item->HasSubmenu() &&
state_.item->GetSubmenu()->IsShowing()) {
- // The submenu is showing, but it shouldn't be, close it.
- // See description of in_drag_ as to why close is conditionalized like
- // this.
- state_.item->GetSubmenu()->Close(!in_drag_);
+ state_.item->GetSubmenu()->Hide();
}
if (scroll_task_.get() && scroll_task_->submenu()) {
@@ -2395,8 +2406,7 @@ void MenuController::CloseMenu(MenuItemView* item) {
DCHECK(item);
if (!item->HasSubmenu())
return;
- // See description of in_drag_ as to why close is conditionalized like this.
- item->GetSubmenu()->Close(!in_drag_);
+ item->GetSubmenu()->Hide();
}
void MenuController::OpenMenu(MenuItemView* item) {
diff --git a/chrome/views/chrome_menu.h b/chrome/views/chrome_menu.h
index a92a45c..dd17a84 100644
--- a/chrome/views/chrome_menu.h
+++ b/chrome/views/chrome_menu.h
@@ -528,14 +528,25 @@ class SubmenuView : public View {
virtual bool OnMouseWheel(const MouseWheelEvent& e);
// Returns true if the menu is showing.
- bool IsShowing() const { return (host_ != NULL); }
+ bool IsShowing();
// Shows the menu at the specified location. Coordinates are in screen
// coordinates. max_width gives the max width the view should be.
void ShowAt(HWND parent, const gfx::Rect& bounds, bool do_capture);
- // Closes the menu. If destroy_host is true, the host is deleted.
- void Close(bool destroy_host);
+ // Closes the menu, destroying the host.
+ void Close();
+
+ // Hides the hosting window.
+ //
+ // The hosting window is hidden first, then deleted (Close) when the menu is
+ // done running. This is done to avoid deletion ordering dependencies. In
+ // particular, during drag and drop (and when a modal dialog is shown as
+ // a result of choosing a context menu) it is possible that an event is
+ // being processed by the host, so that host is on the stack when we need to
+ // close the window. If we closed the window immediately (and deleted it),
+ // when control returned back to host we would crash as host was deleted.
+ void Hide();
// If mouse capture was grabbed, it is released. Does nothing if mouse was
// not captured.
@@ -921,18 +932,6 @@ class MenuController : public MessageLoopForUI::Dispatcher {
int drop_y_;
int last_drop_operation_;
- // If true, we've invoked DoDrag on the delegate.
- //
- // This is used to determine whether the windows used to show menus
- // are hidden or destroyed. As drag and drop is initiated during a
- // mouse gesture, and the call to initiate the drag is blocking, we
- // can't destroy the window immediately (otherwise when the drag and
- // drop call returns the window that initiated the call and is on
- // the stack has been deleted). During drag and drop (in_drag_ is
- // true) windows are hidden, once the drag and drop call completes
- // MenuItemView takes care of destroying the windows.
- bool in_drag_;
-
// If true, the mouse is over some menu.
bool any_menu_contains_mouse_;
generated by cgit v1.1 at 2025-02-02 08:07:00 +0000