diff options
author | tdanderson <tdanderson@chromium.org> | 2014-11-07 11:49:05 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2014-11-07 19:49:28 +0000 |
commit | f2d6fd8f14b22b4b15e5e8fd3ac2bb8141bef552 (patch) | |
tree | a89e676aef922d711fafd91f7f21444911533485 | |
parent | 0fef34d4f5b2e5cd138cc9696f5f189c8c8c2384 (diff) | |
download | chromium_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.cc | 2 | ||||
-rw-r--r-- | ui/views/widget/root_view.cc | 2 | ||||
-rw-r--r-- | ui/views/widget/widget.cc | 31 | ||||
-rw-r--r-- | ui/views/widget/widget.h | 6 | ||||
-rw-r--r-- | ui/views/widget/widget_unittest.cc | 78 |
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 |