Commit bfc78a0a authored by Jun Mukai's avatar Jun Mukai Committed by Commit Bot

Fix the last touch point in Mus

Currently GestureRecognizerImplMus recceives the last touch point
location through ui::EventObserver (which is supplied through
ObserveEventTypes() mojo call). However, during window move, this
can be late since the OnObservedInputEvent will be called after
processing other event handlings in the window server.

As is written in the bug, if the client observes through
OnBoundsChanged(), this bounds change happens in the client before
OnObservedInputEvent. This results with GetLastTouchPointForTarget
returning 1-frame older location. This isn't quite observable
on normal user operation but has significant differences on test
scenario.

Instead of observing the input, it can simply remember the offset
of the touch location when the window move starts.

BUG=901540
TEST=see the filter update

Change-Id: I84bf19d9b705ec582b5359383a914b19c506adf9
Reviewed-on: https://chromium-review.googlesource.com/c/1321257Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605921}
parent a1586d84
...@@ -39,7 +39,6 @@ ...@@ -39,7 +39,6 @@
# TabDragging: crbug.com/890071 # TabDragging: crbug.com/890071
-TabDragging/DetachToBrowserTabDragControllerTest.DetachToOwnWindowWhileInImmersiveFullscreenMode/1 -TabDragging/DetachToBrowserTabDragControllerTest.DetachToOwnWindowWhileInImmersiveFullscreenMode/1
-TabDragging/DetachToBrowserTabDragControllerTest.DragToSeparateWindow/1
-TabDragging/DetachToBrowserTabDragControllerTestTouch.PressSecondFingerWhileDetached/0 -TabDragging/DetachToBrowserTabDragControllerTestTouch.PressSecondFingerWhileDetached/0
# This test is flaky. https://crbug.com/897879 # This test is flaky. https://crbug.com/897879
......
...@@ -5,12 +5,9 @@ ...@@ -5,12 +5,9 @@
#include "ui/aura/mus/gesture_recognizer_impl_mus.h" #include "ui/aura/mus/gesture_recognizer_impl_mus.h"
#include "ui/aura/client/screen_position_client.h" #include "ui/aura/client/screen_position_client.h"
#include "ui/aura/env.h"
#include "ui/aura/mus/window_tree_client.h" #include "ui/aura/mus/window_tree_client.h"
#include "ui/aura/window.h" #include "ui/aura/window.h"
#include "ui/aura/window_tree_host.h" #include "ui/aura/window_tree_host.h"
#include "ui/display/display.h"
#include "ui/display/screen.h"
namespace aura { namespace aura {
...@@ -42,19 +39,10 @@ void GestureRecognizerImplMus::OnWindowMoveStarted( ...@@ -42,19 +39,10 @@ void GestureRecognizerImplMus::OnWindowMoveStarted(
if (source != ws::mojom::MoveLoopSource::TOUCH) if (source != ws::mojom::MoveLoopSource::TOUCH)
return; return;
moving_window_ = window; moving_window_ = window;
last_location_in_screen_ = cursor_location; cursor_offset_ = cursor_location - window->GetBoundsInScreen().origin();
Env* env = Env::GetInstance();
std::set<ui::EventType> types = {
ui::ET_TOUCH_RELEASED, ui::ET_TOUCH_PRESSED, ui::ET_TOUCH_MOVED,
ui::ET_TOUCH_CANCELLED,
};
env->AddEventObserver(this, env, types);
} }
void GestureRecognizerImplMus::OnWindowMoveEnded(bool success) { void GestureRecognizerImplMus::OnWindowMoveEnded(bool success) {
if (!moving_window_)
return;
Env::GetInstance()->RemoveEventObserver(this);
moving_window_ = nullptr; moving_window_ = nullptr;
} }
...@@ -71,25 +59,15 @@ bool GestureRecognizerImplMus::GetLastTouchPointForTarget( ...@@ -71,25 +59,15 @@ bool GestureRecognizerImplMus::GetLastTouchPointForTarget(
aura::client::ScreenPositionClient* client = aura::client::ScreenPositionClient* client =
aura::client::GetScreenPositionClient(target_window->GetRootWindow()); aura::client::GetScreenPositionClient(target_window->GetRootWindow());
if (client) { if (client) {
gfx::Point location_in_window = last_location_in_screen_; // Use the original offset when the window move started. ui::EventObserver
client->ConvertPointFromScreen(target_window, &location_in_window); // isn't used since its OnEvent may be called slightly later than window
point->set_x(location_in_window.x()); // move (bounds change) is conducted. See crbug.com/901540.
point->set_y(location_in_window.y()); point->set_x(cursor_offset_.x());
point->set_y(cursor_offset_.y());
return true; return true;
} }
} }
return GestureRecognizerImpl::GetLastTouchPointForTarget(consumer, point); return GestureRecognizerImpl::GetLastTouchPointForTarget(consumer, point);
} }
void GestureRecognizerImplMus::OnEvent(const ui::Event& event) {
DCHECK(moving_window_);
last_location_in_screen_ = event.AsLocatedEvent()->location();
display::Display display;
if (display::Screen::GetScreen()->GetDisplayWithDisplayId(
moving_window_->GetHost()->GetDisplayId(), &display)) {
last_location_in_screen_ += display.bounds().OffsetFromOrigin();
}
}
} // namespace aura } // namespace aura
...@@ -7,7 +7,6 @@ ...@@ -7,7 +7,6 @@
#include "base/macros.h" #include "base/macros.h"
#include "ui/aura/mus/window_tree_client_observer.h" #include "ui/aura/mus/window_tree_client_observer.h"
#include "ui/events/event_observer.h"
#include "ui/events/gestures/gesture_recognizer_impl.h" #include "ui/events/gestures/gesture_recognizer_impl.h"
namespace ui { namespace ui {
...@@ -23,8 +22,7 @@ class WindowTreeClient; ...@@ -23,8 +22,7 @@ class WindowTreeClient;
// GestureRecognizerImpl, but it handles keeping GetLastTouchPointForTarget in // GestureRecognizerImpl, but it handles keeping GetLastTouchPointForTarget in
// sync with the server when the touch events are handled within the server. // sync with the server when the touch events are handled within the server.
class GestureRecognizerImplMus : public ui::GestureRecognizerImpl, class GestureRecognizerImplMus : public ui::GestureRecognizerImpl,
public aura::WindowTreeClientObserver, public aura::WindowTreeClientObserver {
public ui::EventObserver {
public: public:
explicit GestureRecognizerImplMus(aura::WindowTreeClient* client); explicit GestureRecognizerImplMus(aura::WindowTreeClient* client);
~GestureRecognizerImplMus() override; ~GestureRecognizerImplMus() override;
...@@ -41,12 +39,9 @@ class GestureRecognizerImplMus : public ui::GestureRecognizerImpl, ...@@ -41,12 +39,9 @@ class GestureRecognizerImplMus : public ui::GestureRecognizerImpl,
ws::mojom::MoveLoopSource source) override; ws::mojom::MoveLoopSource source) override;
void OnWindowMoveEnded(bool success) override; void OnWindowMoveEnded(bool success) override;
// ui::EventObserver:
void OnEvent(const ui::Event& event) override;
aura::WindowTreeClient* client_; aura::WindowTreeClient* client_;
aura::Window* moving_window_ = nullptr; aura::Window* moving_window_ = nullptr;
gfx::Point last_location_in_screen_; gfx::Vector2d cursor_offset_;
DISALLOW_COPY_AND_ASSIGN(GestureRecognizerImplMus); DISALLOW_COPY_AND_ASSIGN(GestureRecognizerImplMus);
}; };
......
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