summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authortdanderson <tdanderson@chromium.org>2014-11-07 11:49:05 -0800
committerCommit bot <commit-bot@chromium.org>2014-11-07 19:49:28 +0000
commitf2d6fd8f14b22b4b15e5e8fd3ac2bb8141bef552 (patch)
treea89e676aef922d711fafd91f7f21444911533485
parent0fef34d4f5b2e5cd138cc9696f5f189c8c8c2384 (diff)
downloadchromium_src-f2d6fd8f14b22b4b15e5e8fd3ac2bb8141bef552.zip
chromium_src-f2d6fd8f14b22b4b15e5e8fd3ac2bb8141bef552.tar.gz
chromium_src-f2d6fd8f14b22b4b15e5e8fd3ac2bb8141bef552.tar.bz2
Revert of Do not interchange MOUSE_MOVED and MOUSE_DRAGGED events in Widget (patchset #8 id:140001 of https://codereview.chromium.org/560053002/)
Reason for revert: Responsible for the crash reported in crbug.com/430427. Relanding will require addressing crbug.com/431350 first. Original issue's description: > Do not interchange MOUSE_MOVED and MOUSE_DRAGGED events in Widget > > In Widget::OnMouseEvent(), do not handle MOUSE_MOVED > events as MOUSE_DRAGGED events, and do not handle > MOUSE_DRAGGED events as MOUSE_MOVED events; instead > trust that the mouse event type has been set correctly > before arriving at Widget. This also allows us to > remove the member |is_mouse_button_pressed_| > from Widget. > > BUG=412929, 412931 > TEST=WidgetTest.DragIntoView BUG=412929, 412931 TBR=sadrul@chromium.org Review URL: https://codereview.chromium.org/703643004 Cr-Commit-Position: refs/heads/master@{#303274}
-rw-r--r--ui/views/view_unittest.cc2
-rw-r--r--ui/views/widget/root_view.cc2
-rw-r--r--ui/views/widget/widget.cc31
-rw-r--r--ui/views/widget/widget.h6
-rw-r--r--ui/views/widget/widget_unittest.cc78
5 files changed, 27 insertions, 92 deletions
diff --git a/ui/views/view_unittest.cc b/ui/views/view_unittest.cc
index aef5632..85c2f48 100644
--- a/ui/views/view_unittest.cc
+++ b/ui/views/view_unittest.cc
@@ -367,7 +367,7 @@ TEST_F(ViewTest, MouseEvent) {
v2->Reset();
ui::MouseEvent released(ui::ET_MOUSE_RELEASED, gfx::Point(), gfx::Point(), 0,
0);
- root->OnMouseReleased(released);
+ root->OnMouseDragged(released);
EXPECT_EQ(v2->last_mouse_event_type_, ui::ET_MOUSE_RELEASED);
EXPECT_EQ(v2->location_.x(), -100);
EXPECT_EQ(v2->location_.y(), -100);
diff --git a/ui/views/widget/root_view.cc b/ui/views/widget/root_view.cc
index c4cb51f..f282b9a 100644
--- a/ui/views/widget/root_view.cc
+++ b/ui/views/widget/root_view.cc
@@ -413,7 +413,6 @@ bool RootView::OnMousePressed(const ui::MouseEvent& event) {
}
bool RootView::OnMouseDragged(const ui::MouseEvent& event) {
- CHECK_EQ(ui::ET_MOUSE_DRAGGED, event.type());
if (mouse_pressed_handler_) {
SetMouseLocationAndFlags(event);
@@ -471,7 +470,6 @@ void RootView::OnMouseCaptureLost() {
}
void RootView::OnMouseMoved(const ui::MouseEvent& event) {
- CHECK_EQ(ui::ET_MOUSE_MOVED, event.type());
View* v = GetEventHandlerForPoint(event.location());
// Find the first enabled view, or the existing move handler, whichever comes
// first. The check for the existing handler is because if a view becomes
diff --git a/ui/views/widget/widget.cc b/ui/views/widget/widget.cc
index 74bd54b..ae43a31 100644
--- a/ui/views/widget/widget.cc
+++ b/ui/views/widget/widget.cc
@@ -162,6 +162,7 @@ Widget::Widget()
is_top_level_(false),
native_widget_initialized_(false),
native_widget_destroyed_(false),
+ is_mouse_button_pressed_(false),
ignore_capture_loss_(false),
last_mouse_event_was_move_(false),
auto_release_capture_(true),
@@ -344,6 +345,10 @@ void Widget::Init(const InitParams& in_params) {
AsNativeWidgetPrivate();
root_view_.reset(CreateRootView());
default_theme_provider_.reset(new ui::DefaultThemeProvider);
+ if (params.type == InitParams::TYPE_MENU) {
+ is_mouse_button_pressed_ =
+ internal::NativeWidgetPrivate::IsMouseButtonDown();
+ }
native_widget_->InitNativeWidget(params);
if (RequiresNonClientView(params.type)) {
non_client_view_ = new NonClientView;
@@ -941,6 +946,8 @@ void Widget::SetCapture(View* view) {
return;
}
+ if (internal::NativeWidgetPrivate::IsMouseButtonDown())
+ is_mouse_button_pressed_ = true;
root_view_->SetMouseHandler(view);
}
@@ -1181,12 +1188,11 @@ void Widget::OnKeyEvent(ui::KeyEvent* event) {
// RootView from anywhere in Widget. Use
// SendEventToProcessor() instead. See crbug.com/348087.
void Widget::OnMouseEvent(ui::MouseEvent* event) {
- if (event->type() != ui::ET_MOUSE_MOVED)
- last_mouse_event_was_move_ = false;
-
View* root_view = GetRootView();
switch (event->type()) {
case ui::ET_MOUSE_PRESSED: {
+ last_mouse_event_was_move_ = false;
+
// We may get deleted by the time we return from OnMousePressed. So we
// use an observer to make sure we are still alive.
WidgetDeletionObserver widget_deletion_observer(this);
@@ -1204,6 +1210,7 @@ void Widget::OnMouseEvent(ui::MouseEvent* event) {
if (root_view && root_view->OnMousePressed(*event) &&
widget_deletion_observer.IsWidgetAlive() && IsVisible() &&
internal::NativeWidgetPrivate::IsMouseButtonDown()) {
+ is_mouse_button_pressed_ = true;
if (!native_widget_->HasCapture())
native_widget_->SetCapture();
event->SetHandled();
@@ -1212,6 +1219,8 @@ void Widget::OnMouseEvent(ui::MouseEvent* event) {
}
case ui::ET_MOUSE_RELEASED:
+ last_mouse_event_was_move_ = false;
+ is_mouse_button_pressed_ = false;
// Release capture first, to avoid confusion if OnMouseReleased blocks.
if (auto_release_capture_ && native_widget_->HasCapture()) {
base::AutoReset<bool> resetter(&ignore_capture_loss_, true);
@@ -1224,8 +1233,13 @@ void Widget::OnMouseEvent(ui::MouseEvent* event) {
return;
case ui::ET_MOUSE_MOVED:
- if (!last_mouse_event_was_move_ ||
- last_mouse_event_position_ != event->location()) {
+ case ui::ET_MOUSE_DRAGGED:
+ if (native_widget_->HasCapture() && is_mouse_button_pressed_) {
+ last_mouse_event_was_move_ = false;
+ if (root_view)
+ root_view->OnMouseDragged(*event);
+ } else if (!last_mouse_event_was_move_ ||
+ last_mouse_event_position_ != event->location()) {
last_mouse_event_position_ = event->location();
last_mouse_event_was_move_ = true;
if (root_view)
@@ -1233,12 +1247,8 @@ void Widget::OnMouseEvent(ui::MouseEvent* event) {
}
return;
- case ui::ET_MOUSE_DRAGGED:
- if (root_view)
- root_view->OnMouseDragged(*event);
- return;
-
case ui::ET_MOUSE_EXITED:
+ last_mouse_event_was_move_ = false;
if (root_view)
root_view->OnMouseExited(*event);
return;
@@ -1261,6 +1271,7 @@ void Widget::OnMouseCaptureLost() {
View* root_view = GetRootView();
if (root_view)
root_view->OnMouseCaptureLost();
+ is_mouse_button_pressed_ = false;
}
void Widget::OnScrollEvent(ui::ScrollEvent* event) {
diff --git a/ui/views/widget/widget.h b/ui/views/widget/widget.h
index fa7c771..c7e46d3 100644
--- a/ui/views/widget/widget.h
+++ b/ui/views/widget/widget.h
@@ -946,13 +946,17 @@ class VIEWS_EXPORT Widget : public internal::NativeWidgetDelegate,
// Whether native widget has been destroyed.
bool native_widget_destroyed_;
+ // TODO(beng): Remove NativeWidgetGtk's dependence on these:
+ // If true, the mouse is currently down.
+ bool is_mouse_button_pressed_;
+
// True if capture losses should be ignored.
bool ignore_capture_loss_;
+ // TODO(beng): Remove NativeWidgetGtk's dependence on these:
// The following are used to detect duplicate mouse move events and not
// deliver them. Displaying a window may result in the system generating
// duplicate move events even though the mouse hasn't moved.
- // TODO(tdanderson): We may be able to remove these members.
bool last_mouse_event_was_move_;
gfx::Point last_mouse_event_position_;
diff --git a/ui/views/widget/widget_unittest.cc b/ui/views/widget/widget_unittest.cc
index 7801c2d..1bbc384 100644
--- a/ui/views/widget/widget_unittest.cc
+++ b/ui/views/widget/widget_unittest.cc
@@ -3240,84 +3240,6 @@ TEST_F(WidgetTest, MouseEventTypesViaGenerator) {
widget->CloseNow();
}
-// Tests that a view does not receive entered, dragged, or moved events if
-// a mouse cursor is moved into it while the left mouse button is pressed.
-TEST_F(WidgetTest, DragIntoView) {
- EventCountView* container = new EventCountView;
- container->SetBounds(0, 0, 100, 80);
-
- EventCountView* consume_view = new EventCountView;
- consume_view->set_handle_mode(EventCountView::CONSUME_EVENTS);
- consume_view->SetBounds(10, 10, 50, 40);
-
- Widget* widget = CreateTopLevelFramelessPlatformWidget();
- widget->SetBounds(gfx::Rect(0, 0, 100, 80));
- widget->SetContentsView(container);
- container->AddChildView(consume_view);
-
- widget->Show();
-
- ui::test::EventGenerator generator(GetContext(), widget->GetNativeWindow());
- generator.set_current_location(gfx::Point(75, 15));
-
- generator.PressLeftButton();
- generator.MoveMouseTo(gfx::Point(20, 20));
- EXPECT_EQ(0, consume_view->GetEventCount(ui::ET_MOUSE_ENTERED));
- EXPECT_EQ(0, consume_view->GetEventCount(ui::ET_MOUSE_DRAGGED));
- EXPECT_EQ(0, consume_view->GetEventCount(ui::ET_MOUSE_MOVED));
-
- widget->CloseNow();
-}
-
-// Tests that a view receives the correct mouse events if a mouse cursor
-// is moved out of its bounds while the left mouse button is pressed.
-TEST_F(WidgetTest, DragOutOfView) {
- EventCountView* container = new EventCountView;
- container->SetBounds(0, 0, 100, 80);
-
- EventCountView* consume_view = new EventCountView;
- consume_view->set_handle_mode(EventCountView::CONSUME_EVENTS);
- consume_view->SetBounds(10, 10, 50, 40);
-
- Widget* widget = CreateTopLevelFramelessPlatformWidget();
- widget->SetBounds(gfx::Rect(0, 0, 100, 80));
- widget->SetContentsView(container);
- container->AddChildView(consume_view);
-
- widget->Show();
-
- ui::test::EventGenerator generator(GetContext(), widget->GetNativeWindow());
- generator.set_current_location(gfx::Point(20, 20));
-
- generator.PressLeftButton();
- EXPECT_EQ(1, consume_view->GetEventCount(ui::ET_MOUSE_PRESSED));
- EXPECT_EQ(0, container->GetEventCount(ui::ET_MOUSE_PRESSED));
- consume_view->ResetCounts();
-
- generator.MoveMouseTo(gfx::Point(70, 70));
- EXPECT_EQ(0, consume_view->GetEventCount(ui::ET_MOUSE_EXITED));
- EXPECT_EQ(1, consume_view->GetEventCount(ui::ET_MOUSE_DRAGGED));
- EXPECT_EQ(0, consume_view->GetEventCount(ui::ET_MOUSE_MOVED));
- EXPECT_EQ(0, container->GetEventCount(ui::ET_MOUSE_ENTERED));
- EXPECT_EQ(0, container->GetEventCount(ui::ET_MOUSE_DRAGGED));
- EXPECT_EQ(0, container->GetEventCount(ui::ET_MOUSE_MOVED));
- consume_view->ResetCounts();
-
- generator.MoveMouseTo(gfx::Point(71, 71));
- EXPECT_EQ(1, consume_view->GetEventCount(ui::ET_MOUSE_DRAGGED));
- EXPECT_EQ(0, consume_view->GetEventCount(ui::ET_MOUSE_MOVED));
- EXPECT_EQ(0, container->GetEventCount(ui::ET_MOUSE_DRAGGED));
- EXPECT_EQ(0, container->GetEventCount(ui::ET_MOUSE_MOVED));
- consume_view->ResetCounts();
-
- generator.ReleaseLeftButton();
- EXPECT_EQ(1, consume_view->GetEventCount(ui::ET_MOUSE_RELEASED));
- EXPECT_EQ(0, container->GetEventCount(ui::ET_MOUSE_RELEASED));
- consume_view->ResetCounts();
-
- widget->CloseNow();
-}
-
// Tests that the root view is correctly set up for Widget types that do not
// require a non-client view, before any other views are added to the widget.
// That is, before Widget::ReorderNativeViews() is called which, if called with