Clear drop info after performing drops

Otherwise, arrow can stay forever.

Bug: 838791
Change-Id: I2fe13be530a66b587511484ea3d093bc39ad4194
Reviewed-on: https://chromium-review.googlesource.com/1041366
Commit-Queue: Sang Woo Ko <sangwoo108@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556302}

chrome/browser/ui/views/frame/browser_root_view.cc

@@ -90,7 +90,11 @@ int GetDropEffect(const ui::DropTargetEvent& event, const GURL& url) {
 }  // namespace
 
 BrowserRootView::DropInfo::DropInfo() = default;
-BrowserRootView::DropInfo::~DropInfo() = default;
+
+BrowserRootView::DropInfo::~DropInfo() {
+  if (target)
+    target->HandleDragExited();
+}
 
 // static
 const char BrowserRootView::kViewClassName[] =
@@ -179,11 +183,6 @@ int BrowserRootView::OnDragUpdated(const ui::DropTargetEvent& event) {
 }
 
 void BrowserRootView::OnDragExited() {
-  if (drop_info_ && drop_info_->target) {
-    drop_info_->target->HandleDragExited();
-    drop_info_->target = nullptr;
-  }
-
   drop_info_.reset();
 }
 
@@ -193,6 +192,10 @@ int BrowserRootView::OnPerformDrop(const ui::DropTargetEvent& event) {
   if (!drop_info_)
     return ui::DragDropTypes::DRAG_NONE;
 
+  // Ensure we call HandleDragExited() on |drop_info_|'s |target| when this
+  // function returns.
+  std::unique_ptr<DropInfo> drop_info = std::move(drop_info_);
+
   // Extract the URL and create a new ui::OSExchangeData containing the URL. We
   // do this as the TabStrip doesn't know about the autocomplete edit and needs
   // to know about it to handle 'paste and go'.
@@ -204,15 +207,15 @@ int BrowserRootView::OnPerformDrop(const ui::DropTargetEvent& event) {
 
   // Do nothing if the file was unsupported, the URL is invalid, or this is a
   // javascript: URL (prevent self-xss). The URL may have been changed after
-  // |drop_info_| was created.
-  if (!drop_info_->file_supported || !url.is_valid() ||
+  // |drop_info| was created.
+  if (!drop_info->file_supported || !url.is_valid() ||
       url.SchemeIs(url::kJavaScriptScheme))
     return ui::DragDropTypes::DRAG_NONE;
 
   NavigateParams params(browser_view_->browser(), url,
                         ui::PAGE_TRANSITION_LINK);
-  params.tabstrip_index = drop_info_->index->value;
-  if (drop_info_->index->drop_before) {
+  params.tabstrip_index = drop_info->index->value;
+  if (drop_info->index->drop_before) {
     base::RecordAction(UserMetricsAction("Tab_DropURLBetweenTabs"));
     params.disposition = WindowOpenDisposition::NEW_FOREGROUND_TAB;
   } else {
@@ -220,7 +223,7 @@ int BrowserRootView::OnPerformDrop(const ui::DropTargetEvent& event) {
     params.disposition = WindowOpenDisposition::CURRENT_TAB;
     Browser* browser = browser_view_->browser();
     TabStripModel* model = browser->tab_strip_model();
-    params.source_contents = model->GetWebContentsAt(drop_info_->index->value);
+    params.source_contents = model->GetWebContentsAt(drop_info->index->value);
   }
 
   params.window_action = NavigateParams::SHOW_WINDOW;

chrome/browser/ui/views/frame/browser_root_view.h

@@ -72,6 +72,8 @@ class BrowserRootView : public views::internal::RootView {
   void OnMouseExited(const ui::MouseEvent& event) override;
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(BrowserRootViewBrowserTest, ClearDropInfo);
+
   // Used during a drop session of a url. Tracks the position of the drop.
   struct DropInfo {
     DropInfo();

chrome/browser/ui/views/frame/browser_root_view_browsertest.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/ui/views/frame/browser_root_view.h"
+
+#include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/views/scoped_macviews_browser_mode.h"
+#include "ui/base/dragdrop/drag_drop_types.h"
+#include "ui/base/dragdrop/drop_target_event.h"
+#include "ui/base/dragdrop/os_exchange_data.h"
+
+class BrowserRootViewBrowserTest : public InProcessBrowserTest {
+ public:
+  BrowserRootViewBrowserTest() = default;
+
+  BrowserRootView* browser_root_view() {
+    BrowserView* browser_view = static_cast<BrowserView*>(browser()->window());
+    return static_cast<BrowserRootView*>(
+        browser_view->GetWidget()->GetRootView());
+  }
+
+ private:
+  test::ScopedMacViewsBrowserMode views_mode_{true};
+
+  DISALLOW_COPY_AND_ASSIGN(BrowserRootViewBrowserTest);
+};
+
+// Clear drop info after performing drop. http://crbug.com/838791
+IN_PROC_BROWSER_TEST_F(BrowserRootViewBrowserTest, ClearDropInfo) {
+  ui::OSExchangeData data;
+  data.SetURL(GURL("http://www.chromium.org/"), base::string16());
+  ui::DropTargetEvent event(data, gfx::Point(), gfx::Point(),
+                            ui::DragDropTypes::DRAG_COPY);
+
+  BrowserRootView* root_view = browser_root_view();
+  root_view->OnDragUpdated(event);
+  root_view->OnPerformDrop(event);
+  EXPECT_FALSE(root_view->drop_info_);
+}

chrome/test/BUILD.gn

@@ -1480,6 +1480,7 @@ test("browser_tests") {
           "../browser/ui/views/extensions/extension_uninstall_dialog_view_browsertest.cc",
           "../browser/ui/views/extensions/media_galleries_dialog_views_browsertest.cc",
           "../browser/ui/views/frame/browser_non_client_frame_view_browsertest.cc",
+          "../browser/ui/views/frame/browser_root_view_browsertest.cc",
           "../browser/ui/views/frame/browser_view_browsertest.cc",
           "../browser/ui/views/hung_renderer_view_browsertest.cc",
           "../browser/ui/views/location_bar/location_bar_view_browsertest.cc",