Fixes drag and drop crash. The problem was when I made

MenuController::SetExitType do a QuitNow it would prematurely exit out
of drag and drop, causing a crash. The fix is to not QuitNow if drag
and drop is on going. (Have I mentioned I hate nested message loops?).

I also changed DragDropController to QuitNow, this way tests can work
and seems like what we want anyway.

Lastly I made all the bookmarkbarviewtests work again as they provide
coverage of this.

BUG=127348
TEST=covered by tests
R=ben@chromium.org,varunjain@chromium.org

Review URL: https://chromiumcodereview.appspot.com/10388056

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@136365 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 33 insertions, 8 deletions

diff --git a/ui/views/controls/menu/menu_controller.cc b/ui/views/controls/menu/menu_controller.cc
index 01ace1f..6f43994 100644
--- a/ui/views/controls/menu/menu_controller.cc
+++ b/ui/views/controls/menu/menu_controller.cc
@@ -27,6 +27,7 @@
 
 #if defined(USE_AURA)
 #include "ui/aura/client/dispatcher_client.h"
+#include "ui/aura/client/drag_drop_client.h"
 #include "ui/aura/env.h"
 #include "ui/aura/root_window.h"
 #endif
@@ -320,9 +321,9 @@ MenuItemView* MenuController::Run(Widget* parent,
   message_loop_depth_++;
   DCHECK_LE(message_loop_depth_, 2);
 #if defined(USE_AURA)
-  aura::client::GetDispatcherClient(
-      parent->GetNativeWindow()->GetRootWindow())->
-          RunWithDispatcher(this, parent->GetNativeWindow(), true);
+  root_window_ = parent->GetNativeWindow()->GetRootWindow();
+  aura::client::GetDispatcherClient(root_window_)->
+      RunWithDispatcher(this, parent->GetNativeWindow(), true);
 #else
   {
     MessageLoopForUI* loop = MessageLoopForUI::current();
@@ -1025,6 +1026,9 @@ MenuController::MenuController(bool blocking,
       drop_target_(NULL),
       drop_position_(MenuDelegate::DROP_UNKNOWN),
       owner_(NULL),
+#if defined(USE_AURA)
+      root_window_(NULL),
+#endif
       possible_drag_(false),
       drag_in_progress_(false),
       valid_drop_coordinates_(false),
@@ -1990,7 +1994,18 @@ void MenuController::SetExitType(ExitType type) {
   //
   // It's safe to invoke QuitNow multiple times, it only effects the current
   // loop.
-  if (exit_type_ != EXIT_NONE && message_loop_depth_)
+  bool quit_now = exit_type_ != EXIT_NONE && message_loop_depth_;
+
+#if defined(USE_AURA)
+  // On aura drag and drop runs a nested messgae loop too. If drag and drop is
+  // active and we quit we would prematurely cancel drag and drop, which we
+  // don't want.
+  if (aura::client::GetDragDropClient(root_window_) &&
+      aura::client::GetDragDropClient(root_window_)->IsDragDropInProgress())
+    quit_now = false;
+#endif
+
+  if (quit_now)
     MessageLoop::current()->QuitNow();
 }
 
diff --git a/ui/views/controls/menu/menu_controller.h b/ui/views/controls/menu/menu_controller.h
index 8f3f054..088e813 100644
--- a/ui/views/controls/menu/menu_controller.h
+++ b/ui/views/controls/menu/menu_controller.h
@@ -20,11 +20,15 @@
 #include "ui/views/controls/menu/menu_delegate.h"
 #include "ui/views/controls/menu/menu_item_view.h"
 
+#if defined(USE_AURA)
+namespace aura {
+class RootWindow;
+}
+#endif
+
 namespace ui {
 class OSExchangeData;
 }
-using ui::OSExchangeData;
-
 namespace views {
 
 class DropTargetEvent;
@@ -110,9 +114,9 @@ class VIEWS_EXPORT MenuController : public MessageLoop::Dispatcher {
   bool GetDropFormats(
       SubmenuView* source,
       int* formats,
-      std::set<OSExchangeData::CustomFormat>* custom_formats);
+      std::set<ui::OSExchangeData::CustomFormat>* custom_formats);
   bool AreDropTypesRequired(SubmenuView* source);
-  bool CanDrop(SubmenuView* source, const OSExchangeData& data);
+  bool CanDrop(SubmenuView* source, const ui::OSExchangeData& data);
   void OnDragEntered(SubmenuView* source, const DropTargetEvent& event);
   int OnDragUpdated(SubmenuView* source, const DropTargetEvent& event);
   void OnDragExited(SubmenuView* source);
@@ -495,6 +499,12 @@ class VIEWS_EXPORT MenuController : public MessageLoop::Dispatcher {
   // Owner of child windows.
   Widget* owner_;
 
+#if defined(USE_AURA)
+  // |owner_|s RootWindow. Cached as at the time we need it |owner_| may have
+  // been deleted.
+  aura::RootWindow* root_window_;
+#endif
+
   // Indicates a possible drag operation.
   bool possible_drag_;