Skip to content

Commit

Permalink
DLP: Remove OnPerformDrop from exo
Browse files Browse the repository at this point in the history 
Removing OnPerformDrop from exo since it's not used
& GetDropCallback is the one used instead.

Bug: 1272065
Change-Id: I5dd6c2b6a4578a7e4320bebb2288976e78ecf498
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3528405
Reviewed-by: Sean Kau <skau@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Aya Elsayed <ayaelattar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#983940}
  • Loading branch information
@ayamahmod
ayamahmod authored and Chromium LUCI CQ committed Mar 22, 2022
1 parent 01def6a commit f1a9c75
Show file tree
Hide file tree
Showing 9 changed files with 76 additions and 88 deletions.
72 changes: 36 additions & 36 deletions components/exo/data_device.cc
View file
Expand Up @@ -163,41 +163,7 @@ void DataDevice::OnDragExited() {
data_offer_.reset();
}

DragOperation DataDevice::OnPerformDrop() {
if (!data_offer_)
return DragOperation::kNone;

DndAction dnd_action = data_offer_->get()->dnd_action();

delegate_->OnDrop();

// TODO(crbug.com/1160925): Avoid using nested loop by adding asynchronous
// callback to aura::client::DragDropDelegate.
base::WeakPtr<DataDevice> alive(weak_factory_.GetWeakPtr());
base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE, run_loop.QuitClosure(), kDataOfferDestructionTimeout);
quit_closure_ = run_loop.QuitClosure();
run_loop.Run();

if (!alive)
return DragOperation::kNone;

if (quit_closure_) {
// DataOffer not destroyed by the client until the timeout.
quit_closure_.Reset();
data_offer_.reset();
drop_succeeded_ = false;
}

if (!drop_succeeded_)
return DragOperation::kNone;

return DndActionToDragOperation(dnd_action);
}

WMHelper::DragDropObserver::DropCallback DataDevice::GetDropCallback(
const ui::DropTargetEvent& event) {
WMHelper::DragDropObserver::DropCallback DataDevice::GetDropCallback() {
base::ScopedClosureRunner drag_exit(
base::BindOnce(&DataDevice::OnDragExited, weak_factory_.GetWeakPtr()));
return base::BindOnce(&DataDevice::PerformDropOrExitDrag,
Expand Down Expand Up @@ -272,8 +238,42 @@ void DataDevice::SetSelectionToCurrentClipboardData() {
void DataDevice::PerformDropOrExitDrag(
base::ScopedClosureRunner exit_drag,
ui::mojom::DragOperation& output_drag_op) {
output_drag_op = OnPerformDrop();
exit_drag.ReplaceClosure(base::DoNothing());

if (!data_offer_) {
output_drag_op = DragOperation::kNone;
return;
}

DndAction dnd_action = data_offer_->get()->dnd_action();

delegate_->OnDrop();

// TODO(crbug.com/1160925): Avoid using nested loop by adding asynchronous
// callback to aura::client::DragDropDelegate.
base::WeakPtr<DataDevice> alive(weak_factory_.GetWeakPtr());
base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE, run_loop.QuitClosure(), kDataOfferDestructionTimeout);
quit_closure_ = run_loop.QuitClosure();
run_loop.Run();

if (!alive) {
output_drag_op = DragOperation::kNone;
return;
}

if (quit_closure_) {
// DataOffer not destroyed by the client until the timeout.
quit_closure_.Reset();
data_offer_.reset();
drop_succeeded_ = false;
}

if (!drop_succeeded_)
output_drag_op = DragOperation::kNone;
else
output_drag_op = DndActionToDragOperation(dnd_action);
}

} // namespace exo
4 changes: 1 addition & 3 deletions components/exo/data_device.h
View file
Expand Up @@ -64,9 +64,7 @@ class DataDevice : public WMHelper::DragDropObserver,
aura::client::DragUpdateInfo OnDragUpdated(
const ui::DropTargetEvent& event) override;
void OnDragExited() override;
ui::mojom::DragOperation OnPerformDrop() override;
WMHelper::DragDropObserver::DropCallback GetDropCallback(
const ui::DropTargetEvent& event) override;
WMHelper::DragDropObserver::DropCallback GetDropCallback() override;

// Overridden from ui::ClipboardObserver:
void OnClipboardDataChanged() override;
Expand Down
38 changes: 25 additions & 13 deletions components/exo/data_device_unittest.cc
View file
Expand Up @@ -191,8 +191,10 @@ TEST_F(DataDeviceTest, DataEventsDrop) {
FROM_HERE, base::BindOnce(&TestDataDeviceDelegate::DeleteDataOffer,
base::Unretained(&delegate_), true));

DragOperation result = device_->OnPerformDrop();
EXPECT_EQ(DragOperation::kLink, result);
auto drop_cb = device_->GetDropCallback();
DragOperation output_drag_op;
std::move(drop_cb).Run(output_drag_op);
EXPECT_EQ(DragOperation::kLink, output_drag_op);
ASSERT_EQ(1u, delegate_.PopEvents(&events));
EXPECT_EQ(DataEvent::kDrop, events[0]);
}
Expand Down Expand Up @@ -284,8 +286,10 @@ TEST_F(DataDeviceTest, DataEventsPreventMotion) {
FROM_HERE, base::BindOnce(&TestDataDeviceDelegate::DeleteDataOffer,
base::Unretained(&delegate_), true));

DragOperation result = device_->OnPerformDrop();
EXPECT_EQ(DragOperation::kLink, result);
auto drop_cb = device_->GetDropCallback();
DragOperation output_drag_op;
std::move(drop_cb).Run(output_drag_op);
EXPECT_EQ(DragOperation::kLink, output_drag_op);
ASSERT_EQ(1u, delegate_.PopEvents(&events));
EXPECT_EQ(DataEvent::kDrop, events[0]);
}
Expand Down Expand Up @@ -319,8 +323,10 @@ TEST_F(DataDeviceTest, DeleteDataDeviceDuringDrop) {
device_->OnDragEntered(event);
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindLambdaForTesting([&]() { device_.reset(); }));
DragOperation result = device_->OnPerformDrop();
EXPECT_EQ(DragOperation::kNone, result);
auto drop_cb = device_->GetDropCallback();
DragOperation output_drag_op;
std::move(drop_cb).Run(output_drag_op);
EXPECT_EQ(DragOperation::kNone, output_drag_op);
}

TEST_F(DataDeviceTest, DeleteDataOfferDuringDrag) {
Expand All @@ -340,7 +346,9 @@ TEST_F(DataDeviceTest, DeleteDataOfferDuringDrag) {
device_->OnDragUpdated(event).drag_operation);
EXPECT_EQ(0u, delegate_.PopEvents(&events));

device_->OnPerformDrop();
auto drop_cb = device_->GetDropCallback();
DragOperation output_drag_op;
std::move(drop_cb).Run(output_drag_op);
EXPECT_EQ(0u, delegate_.PopEvents(&events));
}

Expand All @@ -364,8 +372,10 @@ TEST_F(DataDeviceTest, DataOfferNotFinished) {
FROM_HERE, base::BindOnce(&TestDataDeviceDelegate::DeleteDataOffer,
base::Unretained(&delegate_), false));

DragOperation result = device_->OnPerformDrop();
EXPECT_EQ(DragOperation::kNone, result);
auto drop_cb = device_->GetDropCallback();
DragOperation output_drag_op;
std::move(drop_cb).Run(output_drag_op);
EXPECT_EQ(DragOperation::kNone, output_drag_op);
ASSERT_EQ(1u, delegate_.PopEvents(&events));
EXPECT_EQ(DataEvent::kDrop, events[0]);
}
Expand All @@ -385,7 +395,9 @@ TEST_F(DataDeviceTest, NotAcceptDataEventsForSurface) {
device_->OnDragUpdated(event).drag_operation);
EXPECT_EQ(0u, delegate_.PopEvents(&events));

device_->OnPerformDrop();
auto drop_cb = device_->GetDropCallback();
DragOperation output_drag_op;
std::move(drop_cb).Run(output_drag_op);
EXPECT_EQ(0u, delegate_.PopEvents(&events));
}

Expand All @@ -405,7 +417,7 @@ TEST_F(DataDeviceTest, DropCallback_Run) {
ASSERT_EQ(1u, delegate_.PopEvents(&events));
EXPECT_EQ(DataEvent::kMotion, events[0]);

auto drop_cb = device_->GetDropCallback(event);
auto drop_cb = device_->GetDropCallback();

base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(&TestDataDeviceDelegate::DeleteDataOffer,
Expand Down Expand Up @@ -435,7 +447,7 @@ TEST_F(DataDeviceTest, DropCallback_Invalidated) {
ASSERT_EQ(1u, delegate_.PopEvents(&events));
EXPECT_EQ(DataEvent::kMotion, events[0]);

auto drop_cb = device_->GetDropCallback(event);
auto drop_cb = device_->GetDropCallback();

delegate_.DeleteDataOffer(false);

Expand All @@ -462,7 +474,7 @@ TEST_F(DataDeviceTest, DropCallback_Reset) {
ASSERT_EQ(1u, delegate_.PopEvents(&events));
EXPECT_EQ(DataEvent::kMotion, events[0]);

auto drop_cb = device_->GetDropCallback(event);
auto drop_cb = device_->GetDropCallback();
drop_cb.Reset();

ASSERT_EQ(1u, delegate_.PopEvents(&events));
Expand Down
5 changes: 1 addition & 4 deletions components/exo/pointer_unittest.cc
View file
Expand Up @@ -1191,10 +1191,7 @@ class PointerDragDropObserver : public WMHelper::DragDropObserver {
return aura::client::DragUpdateInfo();
}
void OnDragExited() override {}
ui::mojom::DragOperation OnPerformDrop() override {
return ui::mojom::DragOperation::kNone;
}
DropCallback GetDropCallback(const ui::DropTargetEvent& event) override {
DropCallback GetDropCallback() override {
return base::BindOnce([](std::unique_ptr<Surface> surface,
ui::mojom::DragOperation& output_drag_op) {},
std::move(surface_));
Expand Down
5 changes: 0 additions & 5 deletions components/exo/test/exo_test_base_views.cc
View file
Expand Up @@ -93,11 +93,6 @@ class WMHelperTester : public WMHelper, public VSyncTimingManager::Delegate {
return aura::client::DragUpdateInfo();
}
void OnDragExited() override {}
ui::mojom::DragOperation OnPerformDrop(
const ui::DropTargetEvent& event,
std::unique_ptr<ui::OSExchangeData> data) override {
return ui::mojom::DragOperation::kNone;
}
WMHelper::DropCallback GetDropCallback(
const ui::DropTargetEvent& event) override {
return base::DoNothing();
Expand Down
6 changes: 1 addition & 5 deletions components/exo/wm_helper.h
View file
Expand Up @@ -59,8 +59,7 @@ class WMHelper : public aura::client::DragDropDelegate {
virtual aura::client::DragUpdateInfo OnDragUpdated(
const ui::DropTargetEvent& event) = 0;
virtual void OnDragExited() = 0;
virtual ui::mojom::DragOperation OnPerformDrop() = 0;
virtual DropCallback GetDropCallback(const ui::DropTargetEvent& event) = 0;
virtual DropCallback GetDropCallback() = 0;

protected:
virtual ~DragDropObserver() {}
Expand Down Expand Up @@ -173,9 +172,6 @@ class WMHelper : public aura::client::DragDropDelegate {
aura::client::DragUpdateInfo OnDragUpdated(
const ui::DropTargetEvent& event) override = 0;
void OnDragExited() override = 0;
ui::mojom::DragOperation OnPerformDrop(
const ui::DropTargetEvent& event,
std::unique_ptr<ui::OSExchangeData> data) override = 0;
aura::client::DragDropDelegate::DropCallback GetDropCallback(
const ui::DropTargetEvent& event) override = 0;

Expand Down
11 changes: 1 addition & 10 deletions components/exo/wm_helper_chromeos.cc
View file
Expand Up @@ -147,21 +147,12 @@ void WMHelperChromeOS::OnDragExited() {
observer.OnDragExited();
}

ui::mojom::DragOperation WMHelperChromeOS::OnPerformDrop(
const ui::DropTargetEvent& event,
std::unique_ptr<ui::OSExchangeData> data) {
auto drop_cb = GetDropCallback(event);
auto output_drag_op = ui::mojom::DragOperation::kNone;
std::move(drop_cb).Run(std::move(data), output_drag_op);
return output_drag_op;
}

aura::client::DragDropDelegate::DropCallback WMHelperChromeOS::GetDropCallback(
const ui::DropTargetEvent& event) {
std::vector<WMHelper::DragDropObserver::DropCallback> drop_callbacks;
for (DragDropObserver& observer : drag_drop_observers_) {
WMHelper::DragDropObserver::DropCallback drop_cb =
observer.GetDropCallback(event);
observer.GetDropCallback();
if (!drop_cb.is_null()) {
drop_callbacks.push_back(std::move(drop_cb));
}
Expand Down
3 changes: 0 additions & 3 deletions components/exo/wm_helper_chromeos.h
View file
Expand Up @@ -112,9 +112,6 @@ class WMHelperChromeOS : public WMHelper, public VSyncTimingManager::Delegate {
aura::client::DragUpdateInfo OnDragUpdated(
const ui::DropTargetEvent& event) override;
void OnDragExited() override;
ui::mojom::DragOperation OnPerformDrop(
const ui::DropTargetEvent& event,
std::unique_ptr<ui::OSExchangeData> data) override;
aura::client::DragDropDelegate::DropCallback GetDropCallback(
const ui::DropTargetEvent& event) override;

Expand Down
20 changes: 11 additions & 9 deletions components/exo/wm_helper_chromeos_unittest.cc
View file
Expand Up @@ -19,6 +19,7 @@
#include "ui/aura/client/drag_drop_delegate.h"
#include "ui/base/dragdrop/drag_drop_types.h"
#include "ui/base/dragdrop/drop_target_event.h"
#include "ui/base/dragdrop/mojom/drag_drop_types.mojom-forward.h"
#include "ui/base/dragdrop/mojom/drag_drop_types.mojom.h"
#include "ui/base/dragdrop/os_exchange_data.h"
#include "ui/display/manager/display_manager.h"
Expand All @@ -44,9 +45,7 @@ class MockDragDropObserver : public WMHelper::DragDropObserver {
return aura::client::DragUpdateInfo();
}
void OnDragExited() override {}
DragOperation OnPerformDrop() override { return drop_result_; }
WMHelper::DragDropObserver::DropCallback GetDropCallback(
const ui::DropTargetEvent& event) override {
WMHelper::DragDropObserver::DropCallback GetDropCallback() override {
return base::BindOnce(
[](DragOperation drop_result, DragOperation& output_drag_op) {
output_drag_op = drop_result;
Expand Down Expand Up @@ -116,14 +115,17 @@ TEST_F(WMHelperChromeOSTest, MultipleDragDropObservers) {

ui::DropTargetEvent target_event(ui::OSExchangeData(), gfx::PointF(),
gfx::PointF(), ui::DragDropTypes::DRAG_NONE);
DragOperation op = wm_helper_chromeos->OnPerformDrop(
target_event, std::make_unique<ui::OSExchangeData>());
EXPECT_EQ(op, DragOperation::kNone);
auto drop_cb = wm_helper_chromeos->GetDropCallback(target_event);
DragOperation output_drop_op = DragOperation::kNone;
std::move(drop_cb).Run(std::make_unique<ui::OSExchangeData>(),
output_drop_op);
EXPECT_EQ(output_drop_op, DragOperation::kNone);

wm_helper_chromeos->AddDragDropObserver(&observer_copy_drop);
op = wm_helper_chromeos->OnPerformDrop(
target_event, std::make_unique<ui::OSExchangeData>());
EXPECT_NE(op, DragOperation::kNone);
drop_cb = wm_helper_chromeos->GetDropCallback(target_event);
std::move(drop_cb).Run(std::make_unique<ui::OSExchangeData>(),
output_drop_op);
EXPECT_NE(output_drop_op, DragOperation::kNone);

wm_helper_chromeos->RemoveDragDropObserver(&observer_no_drop);
wm_helper_chromeos->RemoveDragDropObserver(&observer_copy_drop);
Expand Down

0 comments on commit f1a9c75

Please sign in to comment.