Skip to content

GitLab

T
tangled
Commit 8993d590 authored by Findit's avatar Findit
Browse files
Options

Revert "Transfer touch events back to the original when the window move ends" 

This reverts commit 0b5ca9b6.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 604473 as the
culprit for flakes in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vMGI1Y2E5YjY5NzIwMGVhNTdlMDQyMGJlYjE0N2Y0ZmE4ZmJkMTUzZAw

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.chromiumos/linux-chromeos-dbg/8644

Sample Failed Step: views_mus_unittests

Sample Flaky Test: DesktopWindowTreeHostMusTest.WindowMoveShouldNotTransfersBack

Original change's description:
> Transfer touch events back to the original when the window move ends
> 
> The current behavior makes TransferEventsTo() before the window
> move starts, but don't care at its end. This causes errors like
> crbug.com/900363. But we can't simply transfer back to the original
> window unconditionally since someone (like TabDragController) may
> also want to continue dragging on a window other than the original
> source window.
> 
> This CL introduces a new scoped class to control this; it makes
> TransferEventsTo first, and then invokes TransferEventsTo back
> again at the end, but it skips invoking the second transfer if
> someone else also transfers the touch events on the same window.
> 
> BUG=900363
> TEST=the new test case
> 
> Change-Id: I81792872a13f0e4bb2c88b526c92cf689d656b71
> Reviewed-on: https://chromium-review.googlesource.com/c/1310501
> Commit-Queue: Jun Mukai <mukai@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#604473}

No-Presubmit: true
No-Tree-Checks: true
No-Try: true
BUG=900363, 900848

Change-Id: Ie326f3b57b2869b0981beb73344fbb530ca21721
Reviewed-on: https://chromium-review.googlesource.com/c/1312613
Cr-Commit-Position: refs/heads/master@{#604528}
parent 48375c9d
Hide whitespace changes
Inline Side-by-side
Showing with 3 additions and 179 deletions
ui/views/mus/desktop_window_tree_host_mus.cc
View file @ 8993d590
......@@ -19,11 +19,9 @@
#include "ui/aura/mus/window_tree_host_mus.h"
#include "ui/aura/mus/window_tree_host_mus_init_params.h"
#include "ui/aura/window.h"
#include "ui/aura/window_tracker.h"
#include "ui/base/hit_test.h"
#include "ui/display/screen.h"
#include "ui/events/gestures/gesture_recognizer.h"
#include "ui/events/gestures/gesture_recognizer_observer.h"
#include "ui/gfx/geometry/dip_util.h"
#include "ui/gfx/geometry/vector2d_conversions.h"
#include "ui/views/accessibility/view_accessibility.h"
......@@ -237,54 +235,6 @@ void OnMoveLoopEnd(bool* out_success,
quit_closure.Run();
}
// ScopedTouchTransferController controls the transfer of touch events for
// window move loop. It transfers touches before the window move starts, and
// then transfers them back to the original window when the window move ends.
// However this transferring back to the original shouldn't happen if the client
// wants to continue the dragging on another window (like attaching the dragged
// tab to another window).
class ScopedTouchTransferController : public ui::GestureRecognizerObserver {
public:
ScopedTouchTransferController(aura::Window* source, aura::Window* dest)
: tracker_({source, dest}),
gesture_recognizer_(source->env()->gesture_recognizer()) {
gesture_recognizer_->TransferEventsTo(
source, dest, ui::TransferTouchesBehavior::kDontCancel);
gesture_recognizer_->AddObserver(this);
}
~ScopedTouchTransferController() override {
gesture_recognizer_->RemoveObserver(this);
if (tracker_.windows().size() == 2) {
aura::Window* source = tracker_.Pop();
aura::Window* dest = tracker_.Pop();
gesture_recognizer_->TransferEventsTo(
dest, source, ui::TransferTouchesBehavior::kDontCancel);
}
}
private:
// ui::GestureRecognizerObserver:
void OnActiveTouchesCanceledExcept(
ui::GestureConsumer* not_cancelled) override {}
void OnEventsTransferred(
ui::GestureConsumer* current_consumer,
ui::GestureConsumer* new_consumer,
ui::TransferTouchesBehavior transfer_touches_behavior) override {
if (tracker_.windows().size() <= 1)
return;
aura::Window* dest = tracker_.windows()[1];
if (current_consumer == dest)
tracker_.Remove(dest);
}
void OnActiveTouchesCanceled(ui::GestureConsumer* consumer) override {}
aura::WindowTracker tracker_;
ui::GestureRecognizer* gesture_recognizer_;
DISALLOW_COPY_AND_ASSIGN(ScopedTouchTransferController);
};
} // namespace
// See description in RestoreToPreminimizedState() for details.
......@@ -854,7 +804,9 @@ Widget::MoveLoopResult DesktopWindowTreeHostMus::RunMoveLoop(
// When using WindowService, the touch events for the window move will
// happen on the root window, so the events need to be transferred from
// widget to its root before starting move loop.
ScopedTouchTransferController scoped_controller(content_window(), window());
window()->env()->gesture_recognizer()->TransferEventsTo(
desktop_native_widget_aura_->content_window(), window(),
ui::TransferTouchesBehavior::kDontCancel);
static_cast<internal::NativeWidgetPrivate*>(
desktop_native_widget_aura_)->ReleaseCapture();
......
ui/views/mus/desktop_window_tree_host_mus_unittest.cc
View file @ 8993d590
......@@ -16,14 +16,11 @@
#include "ui/aura/mus/in_flight_change.h"
#include "ui/aura/mus/window_mus.h"
#include "ui/aura/mus/window_tree_client.h"
#include "ui/aura/mus/window_tree_client_test_observer.h"
#include "ui/aura/test/mus/change_completion_waiter.h"
#include "ui/aura/test/mus/window_tree_client_private.h"
#include "ui/aura/window.h"
#include "ui/events/base_event_utils.h"
#include "ui/events/event.h"
#include "ui/events/gestures/gesture_recognizer.h"
#include "ui/events/gestures/gesture_recognizer_observer.h"
#include "ui/views/accessibility/view_accessibility.h"
#include "ui/views/mus/mus_client.h"
#include "ui/views/mus/mus_client_test_api.h"
......@@ -601,129 +598,4 @@ TEST_F(DesktopWindowTreeHostMusTest, MaximizeMinimizeRestore) {
EXPECT_FALSE(widget->IsMaximized());
}
class TransferTouchEventsCounter : public ui::GestureRecognizerObserver {
public:
TransferTouchEventsCounter() {
aura::Env::GetInstance()->gesture_recognizer()->AddObserver(this);
}
~TransferTouchEventsCounter() override {
aura::Env::GetInstance()->gesture_recognizer()->RemoveObserver(this);
}
int GetTransferCount(ui::GestureConsumer* source,
ui::GestureConsumer* dest) const {
return std::count(transfers_.begin(), transfers_.end(),
std::make_pair(source, dest));
}
int GetTotalCount() const { return transfers_.size(); }
private:
// ui::GestureRecotnizerObserver:
void OnActiveTouchesCanceledExcept(
ui::GestureConsumer* not_cancelled) override {}
void OnEventsTransferred(
ui::GestureConsumer* current_consumer,
ui::GestureConsumer* new_consumer,
ui::TransferTouchesBehavior transfer_touches_behavior) override {
transfers_.push_back(std::make_pair(current_consumer, new_consumer));
}
void OnActiveTouchesCanceled(ui::GestureConsumer* consumer) override {}
std::vector<std::pair<ui::GestureConsumer*, ui::GestureConsumer*>> transfers_;
DISALLOW_COPY_AND_ASSIGN(TransferTouchEventsCounter);
};
class WindowMoveCanceller : public aura::WindowTreeClientTestObserver {
public:
WindowMoveCanceller(aura::WindowTreeClient* client) : client_(client) {
client_->AddTestObserver(this);
}
~WindowMoveCanceller() override { client_->RemoveTestObserver(this); }
void FinishWindowMove() {
DCHECK(window_move_change_id_);
static_cast<ws::mojom::WindowTreeClient*>(client_)->OnChangeCompleted(
window_move_change_id_, true);
}
private:
// aura::WindowTreeClientTestObserver:
void OnChangeStarted(uint32_t change_id, aura::ChangeType type) override {
if (type == aura::ChangeType::MOVE_LOOP)
window_move_change_id_ = change_id;
}
void OnChangeCompleted(uint32_t change_id,
aura::ChangeType type,
bool success) override {}
uint32_t window_move_change_id_ = 0;
aura::WindowTreeClient* client_;
DISALLOW_COPY_AND_ASSIGN(WindowMoveCanceller);
};
TEST_F(DesktopWindowTreeHostMusTest, WindowMoveTransfersTouchEvent) {
std::unique_ptr<Widget> widget(CreateWidget());
widget->Show();
TransferTouchEventsCounter counter;
WindowMoveCanceller canceller(MusClient::Get()->window_tree_client());
aura::Window* window = widget->GetNativeWindow();
auto runner = base::ThreadTaskRunnerHandle::Get();
runner->PostTask(
FROM_HERE,
base::BindOnce(
[](TransferTouchEventsCounter* counter, aura::Window* window) {
EXPECT_EQ(
1, counter->GetTransferCount(window, window->GetRootWindow()));
EXPECT_EQ(1, counter->GetTotalCount());
},
base::Unretained(&counter), window));
runner->PostTask(FROM_HERE,
base::BindOnce(&WindowMoveCanceller::FinishWindowMove,
base::Unretained(&canceller)));
widget->RunMoveLoop(gfx::Vector2d(), Widget::MOVE_LOOP_SOURCE_TOUCH,
Widget::MOVE_LOOP_ESCAPE_BEHAVIOR_DONT_HIDE);
EXPECT_EQ(1, counter.GetTransferCount(window->GetRootWindow(), window));
EXPECT_EQ(2, counter.GetTotalCount());
}
TEST_F(DesktopWindowTreeHostMusTest, WindowMoveShouldNotTransfersBack) {
std::unique_ptr<Widget> widget(CreateWidget());
widget->Show();
std::unique_ptr<Widget> widget2(CreateWidget());
widget2->Show();
TransferTouchEventsCounter counter;
WindowMoveCanceller canceller(MusClient::Get()->window_tree_client());
aura::Window* window = widget->GetNativeWindow();
aura::Window* root = window->GetRootWindow();
aura::Window* window2 = widget2->GetNativeWindow();
auto runner = base::ThreadTaskRunnerHandle::Get();
runner->PostTask(
FROM_HERE,
base::BindOnce(
[](aura::Window* w1, aura::Window* w2) {
aura::Env::GetInstance()->gesture_recognizer()->TransferEventsTo(
w1, w2, ui::TransferTouchesBehavior::kDontCancel);
},
root, window2));
runner->PostTask(FROM_HERE,
base::BindOnce(&WindowMoveCanceller::FinishWindowMove,
base::Unretained(&canceller)));
widget->RunMoveLoop(gfx::Vector2d(), Widget::MOVE_LOOP_SOURCE_TOUCH,
Widget::MOVE_LOOP_ESCAPE_BEHAVIOR_DONT_HIDE);
EXPECT_EQ(0, counter.GetTransferCount(root, window));
EXPECT_EQ(1, counter.GetTransferCount(root, window2));
EXPECT_EQ(2, counter.GetTotalCount());
}
} // namespace views
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment