Properly cleans up state when mouse capture changes.

Properly cleans up state when mouse capture changes.

BUG=98345
TEST=none
R=ben@chromium.org


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=113916

[Ben Goodger (Google), 2011-12-09 21:55:40]

LGTM

On Fri, Dec 9, 2011 at 1:51 PM, <sky@chromium.org> wrote:

> Reviewers: Ben Goodger (Google),
>
> Description:
> Properly cleans up state when mouse capture changes.
>
> BUG=98345
> TEST=none
> R=ben@chromium.org
>
>
> Please review this at
http://codereview.chromium.**org/8892013/<http://codereview.chromium.org/8892...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
>
> Affected files:
>  M ui/aura/root_window.cc
>  M ui/aura/window_unittest.cc
>
>
> Index: ui/aura/root_window.cc
> diff --git a/ui/aura/root_window.cc b/ui/aura/root_window.cc
> index 48d2798693f08439b1bbd2de22737f**cfa6cab4f4..**
> 79eab4ebc249bbe711f9f7a66b2226**a83fb283a3 100644
> --- a/ui/aura/root_window.cc
> +++ b/ui/aura/root_window.cc
> @@ -361,15 +361,16 @@ void RootWindow::SetCapture(Window* window) {
>     // Make all subsequent mouse events and touch go to the capture
> window. We
>     // shouldn't need to send an event here as OnCaptureLost should take
> care of
>     // that.
> -    if (mouse_pressed_handler_)
> -      mouse_pressed_handler_ = capture_window_;
>     if (touch_event_handler_)
>       touch_event_handler_ = capture_window_;
> +    if (mouse_moved_handler_ || mouse_button_flags_ != 0)
> +      mouse_moved_handler_ = capture_window_;
>   } else {
>     // When capture is lost, we must reset the event handlers.
> -    mouse_pressed_handler_ = NULL;
>     touch_event_handler_ = NULL;
> +    mouse_moved_handler_ = NULL;
>   }
> +  mouse_pressed_handler_ = NULL;
>  }
>
>  void RootWindow::ReleaseCapture(**Window* window) {
> Index: ui/aura/window_unittest.cc
> diff --git a/ui/aura/window_unittest.cc b/ui/aura/window_unittest.cc
> index f16fd54285e747b1b909ae3291e8a7**8af1850f57..**
> d8124464cb8911baf0574997414695**29847cc892 100644
> --- a/ui/aura/window_unittest.cc
> +++ b/ui/aura/window_unittest.cc
> @@ -371,7 +371,7 @@ TEST_F(WindowTest, StackChildAbove) {
>   EXPECT_EQ(child3.layer(), parent.layer()->children()[2])**;
>  }
>
> -// Various destruction assertions.
> +// Various capture assertions.
>  TEST_F(WindowTest, CaptureTests) {
>   aura::RootWindow* root_window = aura::RootWindow::GetInstance(**);
>   CaptureWindowDelegateImpl delegate;
> @@ -415,6 +415,35 @@ TEST_F(WindowTest, CaptureTests) {
>   EXPECT_EQ(NULL, root_window->capture_window())**;
>  }
>
> +// Changes capture while capture is already ongoing.
> +TEST_F(WindowTest, ChangeCaptureWhileMouseDown) {
> +  CaptureWindowDelegateImpl delegate;
> +  scoped_ptr<Window> window(**CreateTestWindowWithDelegate(
> +      &delegate, 0, gfx::Rect(0, 0, 20, 20), NULL));
> +  CaptureWindowDelegateImpl delegate2;
> +  scoped_ptr<Window> w2(**CreateTestWindowWithDelegate(
> +      &delegate2, 0, gfx::Rect(20, 20, 20, 20), NULL));
> +  EXPECT_FALSE(window->**HasCapture());
> +
> +  // Do a capture.
> +  window->SetCapture();
> +  delegate.set_mouse_event_**count(0);
> +  EXPECT_TRUE(window->**HasCapture());
> +  EXPECT_EQ(0, delegate.capture_lost_count())**;
> +  EventGenerator generator(gfx::Point(50, 50));
> +  generator.PressLeftButton();
> +  EXPECT_EQ(1, delegate.mouse_event_count());
> +  delegate.set_mouse_event_**count(0);
> +
> +  // Reset the capture.
> +  window->ReleaseCapture();
> +  w2->SetCapture();
> +  delegate2.set_mouse_event_**count(0);
> +  generator.MoveMouseTo(gfx::**Point(40, 40));
> +  EXPECT_EQ(0, delegate.mouse_event_count());
> +  EXPECT_EQ(2, delegate2.mouse_event_count())**;
> +}
> +
>  // Verifies capture is reset when a window is destroyed.
>  TEST_F(WindowTest, ReleaseCaptureOnDestroy) {
>   RootWindow* root_window = RootWindow::GetInstance();
>
>
>

[commit-bot: I haz the power, 2011-12-09 22:59:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sky@chromium.org/8892013/2001

[commit-bot: I haz the power, 2011-12-10 01:34:46]

Change committed as 113916