0

Make sure EndDrag is called

Created a new ScopedClosureRunner in the beginning of the Drop process
that does the following at function return:

1. Flip the `drag_in_progress_` bit.
2. Run any existing `end_drag_runner_`.

This makes sure that the `drag_in_progress_` bit is flipped even if
the function returns early, signaling EndDrag to run immediately. This
also makes sure the bit is flipped *after* the async
GetVirtualFilesAsTempFiles, making use of the `end_drag_runner_`.

(cherry picked from commit 33fd5bb318)

Bug: b:399044195, b:391019233
Change-Id: Ib4e625fb9edc82bfd5235fcb9443308f7928711d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6354726
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Nancy Xiao <nancylanxiao@google.com>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Vincent Chiang <vincentchiang@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1433727}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6369545
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/7049@{#947}
Cr-Branched-From: 2dab7846d0951a552bdc4f350dad497f986e6fed-refs/heads/main@{#1427262}
This commit is contained in:
Vincent Chiang
2025-03-18 15:30:24 -07:00
committed by Chromium LUCI CQ
parent 43a81a1ecb
commit 927f5352ae
3 changed files with 57 additions and 11 deletions

@ -376,14 +376,14 @@ WebContentsViewAura::OnPerformingDropContext::OnPerformingDropContext(
std::unique_ptr<DropData> drop_data,
DropMetadata drop_metadata,
std::unique_ptr<ui::OSExchangeData> data,
base::ScopedClosureRunner end_drag_runner,
base::ScopedClosureRunner drop_exit_cleanup,
std::optional<gfx::PointF> transformed_pt,
gfx::PointF screen_pt)
: target_rwh(target_rwh->GetWeakPtr()),
drop_data(std::move(drop_data)),
drop_metadata(drop_metadata),
data(std::move(data)),
end_drag_runner(std::move(end_drag_runner)),
drop_exit_cleanup(std::move(drop_exit_cleanup)),
transformed_pt(std::move(transformed_pt)),
screen_pt(screen_pt) {}
@ -796,10 +796,6 @@ void WebContentsViewAura::PrepareDropData(
void WebContentsViewAura::EndDrag(
base::WeakPtr<RenderWidgetHostImpl> source_rwh_weak_ptr,
DragOperation op) {
// `drag_in_progress_` could still be true, if the `PerformDropCallback()`
// terminates early and the `end_drag_runner` runs `EndDrag()` before reaching
// the CompleteDrop() call.
drag_in_progress_ = false;
drag_security_info_.OnDragEnded();
if (!web_contents_)
@ -1555,6 +1551,11 @@ void WebContentsViewAura::CompleteDragExit() {
current_drag_data_.reset();
}
void WebContentsViewAura::OnDropExit(
base::ScopedClosureRunner end_drag_runner) {
drag_in_progress_ = false;
}
// PerformDropCallback() is called once the user releases the mouse button
// over this window. This function completes the drop if possible. A drop
// may not be possible for example if the RWH has changed since the user's drag
@ -1607,7 +1608,11 @@ void WebContentsViewAura::PerformDropCallback(
std::unique_ptr<ui::OSExchangeData> data,
base::WeakPtr<RenderWidgetHostViewBase> target,
std::optional<gfx::PointF> transformed_pt) {
base::ScopedClosureRunner end_drag_runner(std::move(end_drag_runner_));
// Exit callback to make sure |drag_in_pregress_| 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_)));
if (!target) {
return;
@ -1642,7 +1647,7 @@ void WebContentsViewAura::PerformDropCallback(
OnPerformingDropContext drop_context(
target_rwh, std::move(current_drag_data_), drop_metadata, std::move(data),
std::move(end_drag_runner), transformed_pt, screen_pt);
std::move(drop_exit_cleanup), transformed_pt, screen_pt);
#if BUILDFLAG(IS_WIN)
if (ShouldIncludeVirtualFiles(*drop_context.drop_data) &&
@ -1725,7 +1730,6 @@ WebContentsViewAura::GetDropCallback(const ui::DropTargetEvent& event) {
}
void WebContentsViewAura::CompleteDrop(OnPerformingDropContext drop_context) {
drag_in_progress_ = false;
web_contents_->Focus();
const int key_modifiers =

@ -107,7 +107,7 @@ class CONTENT_EXPORT WebContentsViewAura
std::unique_ptr<DropData> drop_data,
DropMetadata drop_metadata,
std::unique_ptr<ui::OSExchangeData> data,
base::ScopedClosureRunner end_drag_runner,
base::ScopedClosureRunner drop_exit_cleanup,
std::optional<gfx::PointF> transformed_pt,
gfx::PointF screen_pt);
OnPerformingDropContext(const OnPerformingDropContext& other) = delete;
@ -120,7 +120,7 @@ class CONTENT_EXPORT WebContentsViewAura
std::unique_ptr<DropData> drop_data;
DropMetadata drop_metadata;
std::unique_ptr<ui::OSExchangeData> data;
base::ScopedClosureRunner end_drag_runner;
base::ScopedClosureRunner drop_exit_cleanup;
std::optional<gfx::PointF> transformed_pt;
gfx::PointF screen_pt;
};
@ -151,6 +151,8 @@ class CONTENT_EXPORT WebContentsViewAura
FRIEND_TEST_ALL_PREFIXES(WebContentsViewAuraTest, GetDropCallback_Run);
FRIEND_TEST_ALL_PREFIXES(WebContentsViewAuraTest,
DragInProgressFinishesAfterDrop);
FRIEND_TEST_ALL_PREFIXES(WebContentsViewAuraTest,
DragInProgressFinishesAfterNoDrop);
FRIEND_TEST_ALL_PREFIXES(WebContentsViewAuraTest, GetDropCallback_Cancelled);
FRIEND_TEST_ALL_PREFIXES(
WebContentsViewAuraTest,
@ -328,6 +330,10 @@ class CONTENT_EXPORT WebContentsViewAura
ui::mojom::DragOperation& output_drag_op,
std::unique_ptr<ui::LayerTreeOwner> drag_image_layer_owner);
// Run when drop callback completes to ensure |drag_in_progess_| is
// flipped to false before EndDrag runs.
void OnDropExit(base::ScopedClosureRunner end_drag_runner);
// For unit testing, registers a callback for when a drop operation
// completes.
using DropCallbackForTesting =

@ -1137,9 +1137,45 @@ IN_PROC_BROWSER_TEST_F(WebContentsViewAuraTest,
delegate->DelayedFinishOnPerformingDrop();
async_drop_run_loop.Run();
EXPECT_EQ(1, drag_dest_delegate_.GetOnDropCalledCount());
ASSERT_FALSE(view->drag_in_progress_);
end_drag_run_loop.Run();
EXPECT_EQ(drop_target_widget_,
RenderWidgetHostImpl::From(contents->GetPrimaryFrameTree()
.root()
->current_frame_host()
->GetRenderWidgetHost()));
}
// This test is the same as `DragInProgressFinishesAfterDrop`, but it tests the
// scenario where drag_in_progress_ should still be flipped even when drop is
// blocked.
IN_PROC_BROWSER_TEST_F(WebContentsViewAuraTest,
DragInProgressFinishesAfterNoDrop) {
StartTestWithPage("/simple_page.html");
WebContentsImpl* contents = GetWebContentsImpl();
WebContentsViewAura* view = GetWebContentsViewAura();
base::RunLoop async_drop_run_loop;
async_drop_closure_ = async_drop_run_loop.QuitClosure();
base::RunLoop end_drag_run_loop;
async_end_drag_closure_ = end_drag_run_loop.QuitClosure();
view->end_drag_runner_.ReplaceClosure(base::BindOnce(
&WebContentsViewAuraTest::EndDrag, base::Unretained(this)));
TestWebContentsViewDelegate* delegate =
PrepareWebContentsViewForDropTest(/*delegate_allows_drop=*/false);
SimulateDragEnterAndDrop(/*document_is_handling_drag=*/true);
// `drag_in_progress_` should still be true before `CompleteDrop()` is called.
ASSERT_TRUE(view->drag_in_progress_);
delegate->DelayedFinishOnPerformingDrop();
async_drop_run_loop.Run();
EXPECT_EQ(0, drag_dest_delegate_.GetOnDropCalledCount());
ASSERT_FALSE(view->drag_in_progress_);
end_drag_run_loop.Run();
EXPECT_EQ(drop_target_widget_,
RenderWidgetHostImpl::From(contents->GetPrimaryFrameTree()