Fix crash when the source browser window is deleted during a drag

This CL makes Widget::RunShellDrag() and DesktopDragDropClientWin not crash if
the browser window is deleted during a drag-drop operation. An extension can
delete the browser window during the drag-drop operation.

BUG=440534
TEST=BookmarkBarViewTest22.*

Review URL: https://codereview.chromium.org/795303002

Cr-Commit-Position: refs/heads/master@{#309360}

chrome/browser/ui/views/bookmarks/bookmark_bar_view_test.cc

@@ -206,7 +206,7 @@ class TestingPageNavigator : public PageNavigator {
 };
 
 // TODO(erg): Fix bookmark DND tests on linux_aura. crbug.com/163931
-#if defined(OS_LINUX) && defined(USE_AURA)
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
 #define MAYBE(x) DISABLED_##x
 #else
 #define MAYBE(x) x
@@ -485,7 +485,7 @@ class BookmarkBarViewTest2 : public BookmarkBarViewEventTestBase {
   }
 };
 
-#if defined(OS_LINUX) && !defined(OS_CHROMEOS) && defined(USE_AURA)
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
 // TODO(erg): linux_aura bringup: http://crbug.com/163931
 #define MAYBE_HideOnDesktopClick DISABLED_HideOnDesktopClick
 #else
@@ -1234,7 +1234,7 @@ class BookmarkBarViewTest11 : public BookmarkBarViewEventTestBase {
   BookmarkContextMenuNotificationObserver observer_;
 };
 
-#if defined(OS_LINUX) && !defined(OS_CHROMEOS) && defined(USE_AURA)
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
 // TODO(erg): linux_aura bringup: http://crbug.com/163931
 #define MAYBE_CloseMenuAfterClosingContextMenu \
   DISABLED_CloseMenuAfterClosingContextMenu
@@ -1335,7 +1335,7 @@ class BookmarkBarViewTest12 : public BookmarkBarViewEventTestBase {
   }
 };
 
-#if defined(OS_LINUX) && !defined(OS_CHROMEOS) && defined(USE_AURA)
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
 // TODO(erg): linux_aura bringup: http://crbug.com/163931
 #define MAYBE_CloseWithModalDialog DISABLED_CloseWithModalDialog
 #else
@@ -1672,7 +1672,7 @@ class BookmarkBarViewTest17 : public BookmarkBarViewEventTestBase {
   BookmarkContextMenuNotificationObserver observer_;
 };
 
-#if defined(OS_LINUX) && !defined(OS_CHROMEOS) && defined(USE_AURA)
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
 // TODO(erg): linux_aura bringup: http://crbug.com/163931
 #define MAYBE_ContextMenus3 DISABLED_ContextMenus3
 #elif defined(USE_OZONE)
@@ -1997,3 +1997,74 @@ class BookmarkBarViewTest21 : public BookmarkBarViewEventTestBase {
 };
 
 VIEW_TEST(BookmarkBarViewTest21, ContextMenusForEmptyFolder)
+
+// Test that closing the source browser window while dragging a bookmark does
+// not cause a crash.
+class BookmarkBarViewTest22 : public BookmarkBarViewEventTestBase {
+ protected:
+  void DoTestOnMessageLoop() override {
+    // Move the mouse to the first folder on the bookmark bar and press the
+    // mouse.
+    views::LabelButton* button = GetBookmarkButton(0);
+    ui_test_utils::MoveMouseToCenterAndPress(button, ui_controls::LEFT,
+        ui_controls::DOWN | ui_controls::UP,
+        CreateEventTask(this, &BookmarkBarViewTest22::Step2));
+  }
+
+ 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(0);
+    ASSERT_TRUE(child_menu != NULL);
+
+    // Move mouse to center of menu and press button.
+    ui_test_utils::MoveMouseToCenterAndPress(child_menu, ui_controls::LEFT,
+        ui_controls::DOWN,
+        CreateEventTask(this, &BookmarkBarViewTest22::Step3));
+  }
+
+  void Step3() {
+    views::MenuItemView* target_menu =
+        bb_view_->GetMenu()->GetSubmenu()->GetMenuItemAt(1);
+    gfx::Point loc(1, target_menu->height() - 1);
+    views::View::ConvertPointToScreen(target_menu, &loc);
+
+    // Start a drag.
+    ui_controls::SendMouseMoveNotifyWhenDone(loc.x() + 10, loc.y(),
+        CreateEventTask(this, &BookmarkBarViewTest22::Step4));
+
+    // See comment above this method as to why we do this.
+    ScheduleMouseMoveInBackground(loc.x(), loc.y());
+  }
+
+  void Step4() {
+    window_->Close();
+    window_ = NULL;
+
+#if defined(OS_CHROMEOS)
+    ui_controls::SendMouseEventsNotifyWhenDone(
+        ui_controls::LEFT, ui_controls::UP,
+        CreateEventTask(this, &BookmarkBarViewTest22::Done));
+#else
+    // There are no widgets to send the mouse release to.
+    Done();
+#endif
+  }
+};
+
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
+// TODO(pkotwicz): Enable on Desktop Linux once crbug.com/438365 is fixed.
+#define MAYBE_CloseSourceBrowserDuringDrag DISABLED_CloseSourceBrowserDuringDrag
+#elif defined(OS_WIN)
+// This test times out on Windows. TODO(pkotwicz): Find out why.
+#define MAYBE_CloseSourceBrowserDuringDrag DISABLED_CloseSourceBrowserDuringDrag
+#else
+#define MAYBE_CloseSourceBrowserDuringDrag CloseSourceBrowserDuringDrag
+#endif
+
+VIEW_TEST(BookmarkBarViewTest22, MAYBE_CloseSourceBrowserDuringDrag)

chrome/test/base/view_event_test_base.cc

@@ -88,6 +88,7 @@ void ViewEventTestBase::SetUp() {
   platform_part_.reset(ViewEventTestPlatformPart::Create(context_factory));
   gfx::NativeWindow context = platform_part_->GetContext();
   window_ = views::Widget::CreateWindowWithContext(this, context);
+  window_->Show();
 }
 
 void ViewEventTestBase::TearDown() {

ui/views/widget/desktop_aura/desktop_drag_drop_client_win.cc

@@ -18,11 +18,14 @@ DesktopDragDropClientWin::DesktopDragDropClientWin(
     aura::Window* root_window,
     HWND window)
     : drag_drop_in_progress_(false),
-      drag_operation_(0) {
+      drag_operation_(0),
+      weak_factory_(this) {
   drop_target_ = new DesktopDropTargetWin(root_window, window);
 }
 
 DesktopDragDropClientWin::~DesktopDragDropClientWin() {
+  if (drag_drop_in_progress_)
+    DragCancel();
 }
 
 int DesktopDragDropClientWin::StartDragAndDrop(
@@ -35,7 +38,11 @@ int DesktopDragDropClientWin::StartDragAndDrop(
   drag_drop_in_progress_ = true;
   drag_operation_ = operation;
 
+  base::WeakPtr<DesktopDragDropClientWin> alive(weak_factory_.GetWeakPtr());
+
   drag_source_ = new ui::DragSourceWin;
+  scoped_refptr<ui::DragSourceWin> drag_source_copy = drag_source_;
+
   DWORD effect;
 
   // Use task stopwatch to exclude the drag-drop time current task, if any.
@@ -46,7 +53,8 @@ int DesktopDragDropClientWin::StartDragAndDrop(
       ui::DragDropTypes::DragOperationToDropEffect(operation), &effect);
   stopwatch.Stop();
 
-  drag_drop_in_progress_ = false;
+  if (alive)
+    drag_drop_in_progress_ = false;
 
   if (result != DRAGDROP_S_DROP)
     effect = DROPEFFECT_NONE;

ui/views/widget/desktop_aura/desktop_drag_drop_client_win.h

@@ -7,6 +7,7 @@
 
 #include "base/compiler_specific.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
 #include "ui/views/views_export.h"
 #include "ui/wm/public/drag_drop_client.h"
 
@@ -49,6 +50,8 @@ class VIEWS_EXPORT DesktopDragDropClientWin
 
   scoped_refptr<DesktopDropTargetWin> drop_target_;
 
+  base::WeakPtrFactory<DesktopDragDropClientWin> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(DesktopDragDropClientWin);
 };
 

ui/views/widget/widget.cc

@@ -788,7 +788,14 @@ void Widget::RunShellDrag(View* view,
                           ui::DragDropTypes::DragEventSource source) {
   dragged_view_ = view;
   OnDragWillStart();
+
+  WidgetDeletionObserver widget_deletion_observer(this);
   native_widget_->RunShellDrag(view, data, location, operation, source);
+
+  // The widget may be destroyed during the drag operation.
+  if (!widget_deletion_observer.IsWidgetAlive())
+    return;
+
   // If the view is removed during the drag operation, dragged_view_ is set to
   // NULL.
   if (view && dragged_view_ == view) {