[WebPayments] Implement state transitions in desktop WebPayments dialog.

chrome/browser/ui/views/payments/view_stack.cc

+// Copyright 2016 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 "chrome/browser/ui/views/payments/view_stack.h"
+
+#include "ui/views/layout/fill_layout.h"
+
+namespace {
+
+// Observes the Slide Out view animation and removes the view when it's done
+// being animated, then deletes itself. This takes ownership of the view so
+// it's garanteed to be properly disposed of when the animation is over.
+class SlideOutObserver : public views::BoundsAnimatorObserver {

[sky, 2016/12/12 17:38:07]

Is this class needed? If so, what for? It seems its like the only thing it does
is to wait for the animation to be done and then delete itself.

[anthonyvd, 2016/12/12 18:47:57]

Its initial purpose was to remove the view from the hierarchy and properly
delete it after the animation was over. The ViewStack can do that itself though,
so moved the cleanup to ViewStack::OnBoundsAnimatorDone() below.

+ public:
+  explicit SlideOutObserver(std::unique_ptr<views::View> view_animating_out)
+    : view_animating_out_(std::move(view_animating_out)) {}
+
+  views::View* view_animating_out() {
+    return view_animating_out_.get();
+  }
+
+ private:
+  // views::BoundsAnimatorObserver:
+  void OnBoundsAnimatorProgressed(views::BoundsAnimator* animator) override {}
+
+  void OnBoundsAnimatorDone(views::BoundsAnimator* animator) override {
+    animator->RemoveObserver(this);
+    delete this;
+  }
+
+  std::unique_ptr<views::View> view_animating_out_;
+
+  DISALLOW_COPY_AND_ASSIGN(SlideOutObserver);
+};
+
+}  // namespace
+
+ViewStack::ViewStack(std::unique_ptr<views::View> initial_view)
+    : slide_in_animator_(new views::BoundsAnimator(this)),

[sky, 2016/12/12 17:38:07]

MakeUnique where possible.

[anthonyvd, 2016/12/12 18:47:57]

Done.

+      slide_out_animator_(new views::BoundsAnimator(this)),
+      can_process_events_within_subtree_(true) {
+  SetLayoutManager(new views::FillLayout());
+
+  stack_.push(std::move(initial_view));
+  AddChildView(stack_.top().get());
+
+  slide_in_animator_->AddObserver(this);
+  slide_out_animator_->AddObserver(this);
+}
+
+ViewStack::~ViewStack() {
+  // Cancel animations to make sure their observer's OnBoundsAnimatorDone is
+  // called.
+  slide_in_animator_->Cancel();
+  slide_out_animator_->Cancel();
+}
+

[sky, 2016/12/12 17:38:07]

You should override Layout to resize the views. This is important for the
initial view. You'll have to decide how to deal with a layout during animation
(more specifically a resize).

[anthonyvd, 2016/12/12 18:47:57]

I was under the impression that propagating Layout to children was handled by
the LayoutManager. Is that not the case?  What should this override do? Is this
still relevant now that the constructor doesn't take an initial view anymore?

Sorry for the avalanche of questions, I'd like to understand the Layout
mechanics a little better :-)

[sky, 2016/12/12 20:30:56]

You are absolutely right. The only wrinkle here is if a layout happens during
animation everything is snapped to the origin, which is why I was suggesting
overriding Layout.

[anthonyvd, 2016/12/12 22:20:48]

A yeah, makes sense. I added code that re-targets the animations if a Layout
happens while views are being animated (and a unit test for it). To achieve
that, I had to change the std::stack member of ViewStack to an std::vector (to
be able to iterate on the views and find which ones are being animated). I'd be
interested in knowing if there's an easier way of doing that but it doesn't seem
like it from the BoundsAnimator interface.

+void ViewStack::Push(std::unique_ptr<views::View> view) {
+  can_process_events_within_subtree_ = false;
+
+  // First add the new view out of bounds since it'll slide in from right to
+  // left.
+  view->SetBounds(width(), 0, width(), height());
+  view->Layout();
+
+  AddChildView(view.get());
+  // Animate the new view to be right on top of this one.
+  gfx::Rect destination = bounds();
+  destination.set_origin(gfx::Point(0, 0));
+  slide_in_animator_->AnimateViewTo(view.get(), destination);
+
+  view->set_owned_by_client();
+  // Add the new view to the stack so it can be popped later when navigating
+  // back to the previous screen.
+  stack_.push(std::move(view));
+}
+
+void ViewStack::Pop() {
+  can_process_events_within_subtree_ = false;
+
+  gfx::Rect destination = bounds();
+  destination.set_origin(gfx::Point(width(), 0));
+
+  // This deletes itself.
+  SlideOutObserver* observer =
+      new SlideOutObserver(std::move(stack_.top()));
+  slide_out_animator_->AddObserver(observer);
+
+  stack_.pop();
+  DCHECK(!stack_.empty()) << "State stack should never be empty";
+
+  slide_out_animator_->AnimateViewTo(
+      observer->view_animating_out(), destination);
+}
+
+bool ViewStack::CanProcessEventsWithinSubtree() const {
+  return can_process_events_within_subtree_;
+}
+
+void ViewStack::OnBoundsAnimatorDone(
+    views::BoundsAnimator* animator) {
+  can_process_events_within_subtree_ = true;
+}