Fixes stale hover or ink ripple visuals when dragging over extension buttons

ui/views/controls/button/custom_button_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/views/controls/button/custom_button.h"
 
 #include "base/macros.h"
 #include "build/build_config.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/layout.h"
 #include "ui/events/event_utils.h"
+#include "ui/events/test/event_generator.h"
 #include "ui/gfx/screen.h"
+#include "ui/views/animation/ink_drop_delegate.h"
+#include "ui/views/animation/ink_drop_host.h"
 #include "ui/views/controls/button/checkbox.h"
 #include "ui/views/controls/button/image_button.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/button/menu_button.h"
 #include "ui/views/controls/button/radio_button.h"
 #include "ui/views/controls/link.h"
 #include "ui/views/controls/textfield/textfield.h"
 #include "ui/views/test/views_test_base.h"
 
 #if defined(USE_AURA)
@@ skipping 54 matching lines @@
   bool IsActive() const override { return active_; }
 
   void set_active(bool active) { active_ = active; }
 
  private:
   bool active_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(TestWidget);
 };
 
+// An InkDropDelegate that keeps track of order of deletions.

[bruthig, 2016/01/07 16:55:09]

Is this comment still accurate?

[varkha, 2016/01/07 17:06:39]

Done.

+class TestInkDropDelegate : public InkDropDelegate {
+ public:
+  TestInkDropDelegate(InkDropHost* ink_drop_host,
+                      bool* ink_shown,
+                      bool* ink_hidden)
+      : ink_drop_host_(ink_drop_host),
+        ink_shown_(ink_shown),
+        ink_hidden_(ink_hidden) {
+    ink_drop_host_->AddInkDropLayer(nullptr);
+  }
+  ~TestInkDropDelegate() override {}
+
+  // InkDropDelegate:
+  void SetInkDropSize(int large_size,
+                      int large_corner_radius,
+                      int small_size,
+                      int small_corner_radius) override {}
+  void OnLayout() override {}
+  void OnAction(InkDropState state) override {
+    switch (state) {
+      case InkDropState::ACTION_PENDING:
+      case InkDropState::QUICK_ACTION:
+      case InkDropState::SLOW_ACTION_PENDING:
+      case InkDropState::SLOW_ACTION:
+      case InkDropState::ACTIVATED:
+        *ink_shown_ = true;
+        break;
+      case InkDropState::HIDDEN:
+      case InkDropState::DEACTIVATED:

[bruthig, 2016/01/07 16:55:09]

The DEACTIVATED, SLOW_ACTION and QUICK_ACTION states will auto transition to
HIDDEN but this logic is captured in the InkDropAnimationImpl.  It seems odd
that you group the HIDDEN and DEACTIVATED here.  I would think it should only be
the HIDDEN state.  Likewise above I don't think the QUICK_ACTION or SLOW_ACTION
should set |*ink_shown_| to true.
At the very least I think these 3 states should be handled in the same way.

[varkha, 2016/01/07 17:06:39]

Done.

+        *ink_hidden_ = true;
+        break;
+    }
+  }
+
+ private:
+  InkDropHost* ink_drop_host_;
+  bool* ink_shown_;
+  bool* ink_hidden_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestInkDropDelegate);
+};
+
+// A test Button class that owns a TestInkDropDelegate.
+class TestButtonWithInkDrop : public TestCustomButton,
+                              public views::InkDropHost {
+ public:
+  TestButtonWithInkDrop(bool* ink_shown, bool* ink_hidden)
+      : TestCustomButton(),
+        ink_drop_delegate_(
+            new TestInkDropDelegate(this, ink_shown, ink_hidden)) {
+    set_ink_drop_delegate(ink_drop_delegate_.get());
+  }
+  ~TestButtonWithInkDrop() override {}
+
+  // views::InkDropHost:
+  void AddInkDropLayer(ui::Layer* ink_drop_layer) override {}
+  void RemoveInkDropLayer(ui::Layer* ink_drop_layer) override {}
+  gfx::Point CalculateInkDropCenter() const override { return gfx::Point(); }
+
+ private:
+  scoped_ptr<views::InkDropDelegate> ink_drop_delegate_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestButtonWithInkDrop);
+};
+
 class CustomButtonTest : public ViewsTestBase {
  public:
   CustomButtonTest() {}
   ~CustomButtonTest() override {}
 
   void SetUp() override {
     ViewsTestBase::SetUp();
 
     // Create a widget so that the CustomButton can query the hover state
     // correctly.
     widget_.reset(new TestWidget);
     Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP);
     params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
     params.bounds = gfx::Rect(0, 0, 650, 650);
     widget_->Init(params);
     widget_->Show();
 
-    // Position the widget in a way so that it is under the cursor.
-    gfx::Point cursor = gfx::Screen::GetScreenFor(
-        widget_->GetNativeView())->GetCursorScreenPoint();
-    gfx::Rect widget_bounds = widget_->GetWindowBoundsInScreen();
-    widget_bounds.set_origin(cursor);
-    widget_->SetBounds(widget_bounds);

[varkha, 2016/01/07 01:40:35]

This code made sense in the original first submission but I don't think it is
necessary when used as it is now in SetUp. It causes the widget to be recreated
in a wrong spot if a mouse cursor is moved during the test.

-
     button_ = new TestCustomButton();
     widget_->SetContentsView(button_);
   }
 
   void TearDown() override {
     widget_.reset();
     ViewsTestBase::TearDown();
   }
 
+  void CreateButtonWithInkDrop() {
+    delete button_;

[bruthig, 2016/01/07 16:55:09]

Consider resetting |ink_shown_| and |ink_hidden_| here to false so values aren't
carried over if CreateButtonWithInkDrop() is called multiple times.

[varkha, 2016/01/07 17:06:39]

Done.

+    button_ = new TestButtonWithInkDrop(&ink_shown_, &ink_hidden_);
+    widget_->SetContentsView(button_);
+  }
+
+ protected:
   TestWidget* widget() { return widget_.get(); }
   TestCustomButton* button() { return button_; }
+  bool ink_shown() const { return ink_shown_; }
+  bool ink_hidden() const { return ink_hidden_; }
 
  private:
   scoped_ptr<TestWidget> widget_;
   TestCustomButton* button_;
+  bool ink_shown_ = false;
+  bool ink_hidden_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(CustomButtonTest);
 };
 
 }  // namespace
 
 // Tests that hover state changes correctly when visiblity/enableness changes.
 TEST_F(CustomButtonTest, HoverStateOnVisibilityChange) {
   gfx::Point center(10, 10);
   button()->OnMousePressed(ui::MouseEvent(
@@ skipping 218 matching lines @@
   Label label;
   EXPECT_FALSE(CustomButton::AsCustomButton(&label));
 
   Link link(text);
   EXPECT_FALSE(CustomButton::AsCustomButton(&link));
 
   Textfield textfield;
   EXPECT_FALSE(CustomButton::AsCustomButton(&textfield));
 }
 
+// Tests that pressing a button shows the ink drop and releasing the button
+// does not hide the ink drop.
+// Note: Ink drop is not hidden upon release because CustomButton descendants
+// may enter a different ink drop state.
+TEST_F(CustomButtonTest, ButtonClickTogglesInkDrop) {
+  gfx::Point old_cursor = gfx::Screen::GetScreenFor(
+      widget()->GetNativeView())->GetCursorScreenPoint();
+  CreateButtonWithInkDrop();
+
+  ui::test::EventGenerator generator(GetContext(), widget()->GetNativeWindow());
+  generator.set_current_location(gfx::Point(50, 50));
+  generator.PressLeftButton();
+  EXPECT_TRUE(ink_shown());
+  EXPECT_FALSE(ink_hidden());
+
+  generator.ReleaseLeftButton();
+  EXPECT_FALSE(ink_hidden());
+}
+
+// Tests that pressing a button shows and releasing capture hides ink drop.
+TEST_F(CustomButtonTest, CaptureLossHidesInkDrop) {
+  gfx::Point old_cursor = gfx::Screen::GetScreenFor(
+      widget()->GetNativeView())->GetCursorScreenPoint();
+  CreateButtonWithInkDrop();
+
+  ui::test::EventGenerator generator(GetContext(), widget()->GetNativeWindow());
+  generator.set_current_location(gfx::Point(50, 50));
+  generator.PressLeftButton();
+  EXPECT_TRUE(ink_shown());
+  EXPECT_FALSE(ink_hidden());
+
+  widget()->SetCapture(button());
+  widget()->ReleaseCapture();
+  EXPECT_TRUE(ink_hidden());
+}
+
 }  // namespace views