[Merge to 140] Update OnDropExit() to make sure EndDrag() is called.
When drag & drop is async, we previously passed the ownership of
`end_drag_runner_` to OnDropExit() when EndDrag() has not been bound to
the runner at line 1192 [code location], causing EndDrag() not to be
called after drop.
This CL delays the ownership handover to when OnDropExit() is actually
run.
[code location]:
https://source.chromium.org/chromium/chromium/src/+/main:content/browser/web_contents/web_contents_view_aura.cc;l=1192;drc=0386de8bea6ef8b56f888fd445f5a171f066c9d2
(cherry picked from commit 06917ef5b8813c92e2ab1ce962a3af30f5e9561f)
Bug: 438703482
Fixed: 435733196, 435037345, 415005391, 438705130
Change-Id: I2dfc8c3b6757634e08c03718adbd54ac56f16ebf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6801736
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Dominique Fauteux-Chapleau <domfc@chromium.org>
Commit-Queue: Nancy Xiao <nancylanxiao@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1500294}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6854404
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Nancy Xiao <nancylanxiao@google.com>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/branch-heads/7339@{#675}
Cr-Branched-From: 27be8b77710f4405fdfeb4ee946fcabb0f6c92b2-refs/heads/main@{#1496484}
diff --git a/content/browser/web_contents/web_contents_view_aura.cc b/content/browser/web_contents/web_contents_view_aura.cc
index 1ffb73b..3467c9eb 100644
--- a/content/browser/web_contents/web_contents_view_aura.cc
+++ b/content/browser/web_contents/web_contents_view_aura.cc
@@ -1561,9 +1561,9 @@
current_drag_data_.reset();
}
-void WebContentsViewAura::OnDropExit(
- base::ScopedClosureRunner end_drag_runner) {
+void WebContentsViewAura::OnDropExit() {
drag_in_progress_ = false;
+ auto end_drag_runner = std::move(end_drag_runner_);
}
// PerformDropCallback() is called once the user releases the mouse button
@@ -1621,8 +1621,7 @@
// Exit callback to make sure |drag_in_progress_| is flipped on exit and
// |end_drag_runner_| is run after OnGotVirtualFilesAsTempFiles finishes.
base::ScopedClosureRunner drop_exit_cleanup(base::BindOnce(
- &WebContentsViewAura::OnDropExit, weak_ptr_factory_.GetWeakPtr(),
- std::move(end_drag_runner_)));
+ &WebContentsViewAura::OnDropExit, weak_ptr_factory_.GetWeakPtr()));
if (!target) {
return;
diff --git a/content/browser/web_contents/web_contents_view_aura.h b/content/browser/web_contents/web_contents_view_aura.h
index e064e1b..9212764 100644
--- a/content/browser/web_contents/web_contents_view_aura.h
+++ b/content/browser/web_contents/web_contents_view_aura.h
@@ -174,6 +174,8 @@
UrlInDropDataReturnsUrlInOSExchangeDataGetString);
FRIEND_TEST_ALL_PREFIXES(WebContentsViewAuraTest,
IgnoreInputs_OngoingDropGetsCleared);
+ FRIEND_TEST_ALL_PREFIXES(WebContentsViewAuraTest,
+ EndDragIsCalledAfterAsyncDrop);
class WindowObserver;
@@ -333,7 +335,7 @@
// Run when drop callback completes to ensure |drag_in_progess_| is
// flipped to false before EndDrag runs.
- void OnDropExit(base::ScopedClosureRunner end_drag_runner);
+ void OnDropExit();
// For unit testing, registers a callback for when a drop operation
// completes.
diff --git a/content/browser/web_contents/web_contents_view_aura_unittest.cc b/content/browser/web_contents/web_contents_view_aura_unittest.cc
index 3ebe2328..6fe365d 100644
--- a/content/browser/web_contents/web_contents_view_aura_unittest.cc
+++ b/content/browser/web_contents/web_contents_view_aura_unittest.cc
@@ -12,6 +12,7 @@
#include "base/files/file_util.h"
#include "base/run_loop.h"
#include "base/strings/utf_string_conversions.h"
+#include "base/task/single_thread_task_runner.h"
#include "base/test/scoped_command_line.h"
#include "base/test/scoped_feature_list.h"
#include "build/build_config.h"
@@ -970,4 +971,63 @@
EXPECT_EQ(exchange_data->GetString(), url_string);
}
+TEST_F(WebContentsViewAuraTest, EndDragIsCalledAfterAsyncDrop) {
+ const char kGoogleUrl[] = "https://google.com/";
+
+ // Declare an empty but NON-NULL string
+ std::u16string empty_string;
+ NavigateAndCommit(GURL(kGoogleUrl));
+ DropData drop_data;
+ drop_data.text = empty_string;
+ drop_data.url = GURL(kGoogleUrl);
+
+ TestDragDropClient drag_drop_client;
+ aura::client::SetDragDropClient(root_window(), &drag_drop_client);
+
+ // Mark the Web Contents as native UI.
+ WebContentsViewAura* view = GetView();
+
+ // Make sure EndDrag() is called async.
+ view->drag_in_progress_ = true;
+
+ auto data = std::make_unique<ui::OSExchangeData>();
+ const std::u16string string_data = u"Some string data";
+ data->SetString(string_data);
+ ui::DropTargetEvent event(*data.get(), kClientPt, kScreenPt,
+ ui::DragDropTypes::DRAG_COPY);
+
+ view->StartDragging(drop_data, url::Origin::Create(GURL(kGoogleUrl)),
+ blink::DragOperationsMask::kDragOperationNone,
+ gfx::ImageSkia(), gfx::Vector2d(), gfx::Rect(),
+ blink::mojom::DragEventSourceInfo(),
+ RenderWidgetHostImpl::From(rvh()->GetWidget()));
+
+ // Simulate drop.
+ auto callback = base::BindOnce(&WebContentsViewAuraTest::OnDropComplete,
+ base::Unretained(this));
+ view->RegisterDropCallbackForTesting(std::move(callback));
+
+ // Simulate `EndDrag`.
+ base::RunLoop end_drag_run_loop;
+ view->end_drag_runner_.ReplaceClosure(end_drag_run_loop.QuitClosure());
+
+ base::RunLoop end_drop_run_loop;
+ async_drop_closure_ = end_drop_run_loop.QuitClosure();
+ auto drop_cb = view->GetDropCallback(event);
+ ASSERT_TRUE(drop_cb);
+ ui::mojom::DragOperation output_drag_op = ui::mojom::DragOperation::kNone;
+
+ // Post `drop_cb` to simulate an async drop processing. This happens
+ // when `PerformDropOrExitDrag` or `PerformDropCallback` is async.
+ base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
+ FROM_HERE, base::BindOnce(std::move(drop_cb), std::move(data),
+ std::ref(output_drag_op),
+ /*drag_image_layer_owner=*/nullptr));
+
+ end_drop_run_loop.Run();
+ CheckDropData(view);
+
+ end_drag_run_loop.Run();
+}
+
} // namespace content