Implemented prototype for new ink drop specs.

ui/views/animation/ink_drop_animation_controller_factory.cc

 // Copyright 2015 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/animation/ink_drop_animation_controller_factory.h"
 
 #include "ui/base/resource/material_design/material_design_controller.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/views/animation/ink_drop_animation_controller.h"
 #include "ui/views/animation/ink_drop_animation_controller_impl.h"
 #include "ui/views/views_export.h"
 
 namespace views {
 
 namespace {
 
 // A stub implementation of an InkDropAnimationController that can be used when
 // material design is not enabled.
 class VIEWS_EXPORT InkDropAnimationControllerStub

[jonross, 2015/08/19 22:13:36]

Thoughts on just having IDAC be this, instead of pure virtual?

[bruthig, 2015/08/20 20:35:35]

I'm of the opinion that I want to make it hard for users to do things they
shouldn't.  In this case I don't want them to incorrectly (or unknowingly)
create and use an IDAC that is actually a non-functional stub and the only
indication of that fact is documentation or implementation details.  By using a
pure virtual base class and an IDAC with a 'Stub' suffix it should be painfully
obvious what the purpose of each of the classes is.

It's not immediately obvious to me what the benefits would be of implementing it
like that.  Perhaps you can elaborate?

[jonross, 2015/08/21 15:14:48]

I was mostly thinking of condensing the code. Where the default IDAC would
always just be a no-opt. However I'm fine with your reasoning for this
differentiation. SGTM.

     : public InkDropAnimationController {
  public:
   explicit InkDropAnimationControllerStub(InkDropHost* ink_drop_host);
   ~InkDropAnimationControllerStub() override;
 
   // InkDropAnimationController:
+  InkDropState GetInkDropState() const override;
   void AnimateToState(InkDropState state) override;
-  void SetInkDropSize(const gfx::Size& size) override;
-  gfx::Rect GetInkDropBounds() const override;
-  void SetInkDropBounds(const gfx::Rect& bounds) override;
+  gfx::Size GetInkDropLargeSize() const override;
+  void SetInkDropSize(const gfx::Size& large_size,
+                      int large_corner_radius,
+                      const gfx::Size& small_size,
+                      int small_corner_radius) override;
+  gfx::Point GetInkDropOrigin() const override;
+  void SetInkDropOrigin(const gfx::Point& origin) override;
 
  private:
-  // The bounds of the ink drop layers. Defined in the coordinate space of the
-  // parent ui::Layer that the ink drop layers were added to.
-  gfx::Rect ink_drop_bounds_;
-
   DISALLOW_COPY_AND_ASSIGN(InkDropAnimationControllerStub);
 };
 
 InkDropAnimationControllerStub::InkDropAnimationControllerStub(
     InkDropHost* ink_drop_host) {}
 
 InkDropAnimationControllerStub::~InkDropAnimationControllerStub() {}
 
+InkDropState InkDropAnimationControllerStub::GetInkDropState() const {
+  return InkDropState::HIDDEN;
+}
+
 void InkDropAnimationControllerStub::AnimateToState(InkDropState state) {}
 
-void InkDropAnimationControllerStub::SetInkDropSize(const gfx::Size& size) {
-  ink_drop_bounds_.set_size(size);
+gfx::Size InkDropAnimationControllerStub::GetInkDropLargeSize() const {
+  return gfx::Size();
 }
 
-gfx::Rect InkDropAnimationControllerStub::GetInkDropBounds() const {
-  return ink_drop_bounds_;
+void InkDropAnimationControllerStub::SetInkDropSize(const gfx::Size& large_size,
+                                                    int large_corner_radius,
+                                                    const gfx::Size& small_size,
+                                                    int small_corner_radius) {}
+
+gfx::Point InkDropAnimationControllerStub::GetInkDropOrigin() const {
+  return gfx::Point();
 }
 
-void InkDropAnimationControllerStub::SetInkDropBounds(const gfx::Rect& bounds) {
-  ink_drop_bounds_ = bounds;
-}
+void InkDropAnimationControllerStub::SetInkDropOrigin(
+    const gfx::Point& origin) {}
 
 }  // namespace
 
 InkDropAnimationControllerFactory::InkDropAnimationControllerFactory() {}
 
 InkDropAnimationControllerFactory::~InkDropAnimationControllerFactory() {}
 
 scoped_ptr<InkDropAnimationController>
 InkDropAnimationControllerFactory::CreateInkDropAnimationController(
     InkDropHost* ink_drop_host) {
 #if defined(OS_CHROMEOS)
+  // The ink drop animation is only targeted at ChromeOS because there is
+  // concern it will conflict with OS level touch feedback in a bad way.

[tdanderson, 2015/08/19 22:11:19]

Since this is behind a flag anyway, I don't think there's any harm in allowing
on all three platforms just to see what it actually looks like (unless "conflict
with OS level touch feedback in a bad way" actually means "crash").

[bruthig, 2015/08/20 20:35:35]

This was the request of the designers.  If we do change it we should probably
loop them in and I'm assuming we would have to update the hover borders that
will be currently be shown on those platforms.  Is it ok if we defer that for a
follow-up review if necessary?

   if (ui::MaterialDesignController::IsModeMaterial()) {
     return scoped_ptr<InkDropAnimationController>(
         new InkDropAnimationControllerImpl(ink_drop_host));
   } else {
     return scoped_ptr<InkDropAnimationController>(
         new InkDropAnimationControllerStub(ink_drop_host));
   }
 #endif  // defined(OS_CHROMEOS)
 
   return scoped_ptr<InkDropAnimationController>(
       new InkDropAnimationControllerStub(ink_drop_host));
 }
 
 }  // namespace views