Move AutoHighlightMode to InkDropImpl constructor

Removes InkDropImpl::SetAutoHighlightMode which wasn't intended to be
runtime-configured either way.

Bug: 931964
Change-Id: If9b2070149e84899fcad6d9383878e38bea788fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2897427
Commit-Queue: Peter Boström <pbos@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Allen Bauer <kylixrd@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#883541}

ash/shelf/shelf_view_unittest.cc

@@ -2400,7 +2400,8 @@ class ShelfViewInkDropTest : public ShelfViewTest {
     browser_button_ = test_api_->GetButton(0);
 
     auto ink_drop_impl = std::make_unique<views::InkDropImpl>(
-        browser_button_->ink_drop(), browser_button_->size());
+        browser_button_->ink_drop(), browser_button_->size(),
+        views::InkDropImpl::AutoHighlightMode::NONE);
     browser_button_ink_drop_impl_ = ink_drop_impl.get();
 
     auto browser_button_ink_drop =

ui/message_center/views/notification_view_md.cc

@@ -490,9 +490,10 @@ class NotificationInkDropImpl : public views::InkDropImpl {
  public:
   NotificationInkDropImpl(views::InkDropHost* ink_drop_host,
                           const gfx::Size& host_size)
-      : views::InkDropImpl(ink_drop_host, host_size) {
-    SetAutoHighlightMode(views::InkDropImpl::AutoHighlightMode::SHOW_ON_RIPPLE);
-  }
+      : views::InkDropImpl(
+            ink_drop_host,
+            host_size,
+            views::InkDropImpl::AutoHighlightMode::SHOW_ON_RIPPLE) {}
 
   void HostSizeChanged(const gfx::Size& new_size) override {
     // Prevent a call to InkDropImpl::HostSizeChanged which recreates the ripple

ui/views/animation/ink_drop.cc

@@ -25,9 +25,8 @@ std::unique_ptr<InkDrop> CreateInkDropImpl(
     InkDropImpl::AutoHighlightMode auto_highlight_mode,
     bool highlight_on_hover,
     bool highlight_on_focus) {
-  auto ink_drop =
-      std::make_unique<InkDropImpl>(host, host->host_view()->size());
-  ink_drop->SetAutoHighlightMode(auto_highlight_mode);
+  auto ink_drop = std::make_unique<InkDropImpl>(host, host->host_view()->size(),
+                                                auto_highlight_mode);
   ink_drop->SetShowHighlightOnHover(highlight_on_hover);
   ink_drop->SetShowHighlightOnFocus(highlight_on_focus);
   return ink_drop;

ui/views/animation/ink_drop_impl.cc

@@ -562,12 +562,15 @@ InkDropImpl::HighlightStateFactory::CreateVisibleState(
   return nullptr;
 }
 
-InkDropImpl::InkDropImpl(InkDropHost* ink_drop_host, const gfx::Size& host_size)
+InkDropImpl::InkDropImpl(InkDropHost* ink_drop_host,
+                         const gfx::Size& host_size,
+                         AutoHighlightMode auto_highlight_mode)
     : ink_drop_host_(ink_drop_host),
+      highlight_state_factory_(auto_highlight_mode, this),
       root_layer_(new ui::Layer(ui::LAYER_NOT_DRAWN)) {
   root_layer_->SetBounds(gfx::Rect(host_size));
-  SetAutoHighlightMode(AutoHighlightMode::NONE);
   root_layer_->SetName("InkDropImpl:RootLayer");
+  SetHighlightState(highlight_state_factory_.CreateStartState());
 }
 
 InkDropImpl::~InkDropImpl() {
@@ -582,15 +585,6 @@ InkDropImpl::~InkDropImpl() {
   DestroyInkDropHighlight();
 }
 
-void InkDropImpl::SetAutoHighlightMode(AutoHighlightMode auto_highlight_mode) {
-  // Exit the current state completely first in case state tear down accesses
-  // the current |highlight_state_factory_| instance.
-  ExitHighlightState();
-  highlight_state_factory_ =
-      std::make_unique<HighlightStateFactory>(auto_highlight_mode, this);
-  SetHighlightState(highlight_state_factory_->CreateStartState());
-}
-
 void InkDropImpl::HostSizeChanged(const gfx::Size& new_size) {
   // |root_layer_| should fill the entire host because it affects the clipping
   // when a mask layer is applied to it. This will not affect clipping if no

ui/views/animation/ink_drop_impl.h

@@ -49,21 +49,12 @@ class VIEWS_EXPORT InkDropImpl : public InkDrop,
   // |ink_drop_host|. |host_size| is used to set the size of the ink drop layer.
   //
   // By default the highlight will be made visible while |this| is hovered but
-  // not focused and the NONE AutoHighlightMode will be used.
-  InkDropImpl(InkDropHost* ink_drop_host, const gfx::Size& host_size);
+  // not focused.
+  InkDropImpl(InkDropHost* ink_drop_host,
+              const gfx::Size& host_size,
+              AutoHighlightMode auto_highlight_mode);
   ~InkDropImpl() override;
 
-  // Auto highlighting is a mechanism to show/hide the highlight based on the
-  // visibility of the ripple. See the documentation of the AutoHighlightMode
-  // for more info on the different modes.
-  //
-  // This method is intended as a configuration option to be used after
-  // construction. Behavior is undefined if |this| has already handled any
-  // InkDrop inherited functions.
-  // TODO(pbos): Move along with AutoHighlightMode to views::InkDrop so users
-  // can configure inkdrops created by parent classes.
-  void SetAutoHighlightMode(AutoHighlightMode auto_highlight_mode);
-
   const absl::optional<base::TimeDelta>& hover_highlight_fade_duration() const {
     return hover_highlight_fade_duration_;
   }
@@ -156,7 +147,7 @@ class VIEWS_EXPORT InkDropImpl : public InkDrop,
 
    private:
     // Used by |this| to create the new states to transition to.
-    HighlightStateFactory* state_factory_;
+    HighlightStateFactory* const state_factory_;
 
     DISALLOW_COPY_AND_ASSIGN(HighlightState);
   };
@@ -269,6 +260,10 @@ class VIEWS_EXPORT InkDropImpl : public InkDrop,
   // add/remove the root layer to/from it.
   InkDropHost* const ink_drop_host_;
 
+  // Used by |this| to initialize the starting |highlight_state_| and by the
+  // current |highlight_state_| to create the next state.
+  HighlightStateFactory highlight_state_factory_;
+
   // The root Layer that parents the InkDropRipple layers and the
   // InkDropHighlight layers. The |root_layer_| is the one that is added and
   // removed from the |ink_drop_host_|.
@@ -298,10 +293,6 @@ class VIEWS_EXPORT InkDropImpl : public InkDrop,
   // The current InkDropRipple. Created on demand using CreateInkDropRipple().
   std::unique_ptr<InkDropRipple> ink_drop_ripple_;
 
-  // Used by |this| to initialize the starting |highlight_state_| and by the
-  // current |highlight_state_| to create the next state.
-  std::unique_ptr<HighlightStateFactory> highlight_state_factory_;
-
   // The current state object that handles all inputs that affect the visibility
   // of the |highlight_|.
   std::unique_ptr<HighlightState> highlight_state_;

ui/views/animation/ink_drop_impl_unittest.cc

@@ -26,7 +26,8 @@ namespace views {
 // NOTE: The InkDropImpl class is also tested by the InkDropFactoryTest tests.
 class InkDropImplTest : public testing::Test {
  public:
-  InkDropImplTest();
+  explicit InkDropImplTest(InkDropImpl::AutoHighlightMode auto_highlight_mode =
+                               InkDropImpl::AutoHighlightMode::NONE);
   ~InkDropImplTest() override;
 
  protected:
@@ -79,7 +80,9 @@ class InkDropImplTest : public testing::Test {
   DISALLOW_COPY_AND_ASSIGN(InkDropImplTest);
 };
 
-InkDropImplTest::InkDropImplTest() {
+InkDropImplTest::InkDropImplTest(
+    InkDropImpl::AutoHighlightMode auto_highlight_mode)
+    : ink_drop_host_(auto_highlight_mode) {
   ink_drop_host()->ink_drop()->SetMode(views::InkDropHost::InkDropMode::ON);
   test_api_ = std::make_unique<test::InkDropImplTestApi>(ink_drop());
   ink_drop_host()->set_disable_timers_for_test(true);
@@ -110,24 +113,15 @@ class InkDropImplAutoHighlightTest
   InkDropImplAutoHighlightTest();
   ~InkDropImplAutoHighlightTest() override;
 
-  InkDropImpl::AutoHighlightMode GetAutoHighlightMode() const;
-
  private:
   DISALLOW_COPY_AND_ASSIGN(InkDropImplAutoHighlightTest);
 };
 
 InkDropImplAutoHighlightTest::InkDropImplAutoHighlightTest()
-    : InkDropImplTest() {
-  ink_drop()->SetAutoHighlightMode(GetAutoHighlightMode());
-}
+    : InkDropImplTest(testing::get<0>(GetParam())) {}
 
 InkDropImplAutoHighlightTest::~InkDropImplAutoHighlightTest() = default;
 
-InkDropImpl::AutoHighlightMode
-InkDropImplAutoHighlightTest::GetAutoHighlightMode() const {
-  return testing::get<0>(GetParam());
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 //
 // InkDropImpl tests

ui/views/animation/ink_drop_unittest.cc

@@ -56,8 +56,9 @@ InkDropTest::InkDropTest() : ink_drop_(nullptr) {
       ink_drop_ = std::make_unique<InkDropStub>();
       break;
     case INK_DROP_IMPL:
-      ink_drop_ = std::make_unique<InkDropImpl>(test_ink_drop_host_.ink_drop(),
-                                                gfx::Size());
+      ink_drop_ = std::make_unique<InkDropImpl>(
+          test_ink_drop_host_.ink_drop(), gfx::Size(),
+          InkDropImpl::AutoHighlightMode::NONE);
       // The Timer's used by the InkDropImpl class require a
       // base::ThreadTaskRunnerHandle instance.
       scoped_refptr<base::TestMockTimeTaskRunner> task_runner(

ui/views/animation/test/ink_drop_impl_test_api.h

@@ -85,7 +85,7 @@ class InkDropImplTestApi
 
   // Wrappers to InkDropImpl internals:
   InkDropImpl::HighlightStateFactory* state_factory() {
-    return ink_drop_->highlight_state_factory_.get();
+    return &ink_drop_->highlight_state_factory_;
   }
 
   void SetHighlightState(

ui/views/animation/test/test_ink_drop_host.cc

@@ -77,8 +77,16 @@ class TestInkDropHighlight : public InkDropHighlight {
 
 }  // namespace
 
-TestInkDropHost::TestInkDropHost() {
-  InkDrop::UseInkDropWithoutAutoHighlight(ink_drop());
+TestInkDropHost::TestInkDropHost(
+    InkDropImpl::AutoHighlightMode auto_highlight_mode) {
+  ink_drop()->SetCreateInkDropCallback(base::BindRepeating(
+      [](TestInkDropHost* host,
+         InkDropImpl::AutoHighlightMode auto_highlight_mode)
+          -> std::unique_ptr<views::InkDrop> {
+        return std::make_unique<views::InkDropImpl>(
+            host->ink_drop(), host->size(), auto_highlight_mode);
+      },
+      this, auto_highlight_mode));
 
   ink_drop()->SetCreateHighlightCallback(base::BindRepeating(
       [](TestInkDropHost* host) -> std::unique_ptr<views::InkDropHighlight> {

ui/views/animation/test/test_ink_drop_host.h

@@ -7,14 +7,16 @@
 
 #include "base/macros.h"
 #include "ui/views/animation/ink_drop_host_view.h"
+#include "ui/views/animation/ink_drop_impl.h"
 
 namespace views {
 
 // A non-functional implementation of an View with an ink drop that can be used
 // during tests.  Tracks the number of hosted ink drop layers.
 class TestInkDropHost : public View {
  public:
-  TestInkDropHost();
+  explicit TestInkDropHost(InkDropImpl::AutoHighlightMode auto_highlight_mode =
+                               InkDropImpl::AutoHighlightMode::NONE);
   ~TestInkDropHost() override;
 
   int num_ink_drop_layers_added() const { return num_ink_drop_layers_added_; }