[WebPayments] Implement state transitions in desktop WebPayments dialog.

chrome/browser/payments/ui/order_summary_state.cc

Index: chrome/browser/payments/ui/order_summary_state.cc
diff --git a/chrome/browser/payments/ui/order_summary_state.cc b/chrome/browser/payments/ui/order_summary_state.cc
new file mode 100644
index 0000000000000000000000000000000000000000..36d3e369396425d00c2698d675bdfe84d34d0233
--- /dev/null
+++ b/chrome/browser/payments/ui/order_summary_state.cc
@@ -0,0 +1,65 @@
+// 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 "base/observer_list.h"
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/payments/ui/order_summary_state.h"
+#include "ui/views/controls/button/md_text_button.h"
+#include "ui/views/controls/label.h"
+#include "ui/views/layout/grid_layout.h"
+
+namespace payments {
+
+const int kBackButtonTag = 0;
+
+OrderSummaryState::OrderSummaryState()
+  : view_(nullptr) {}
+
+OrderSummaryState::~OrderSummaryState() {}
+
+views::View* OrderSummaryState::GetView() {
+  if (!view_) {
+    view_ = new views::View();
+    view_->SetSize(gfx::Size(300, 300));
+
+    views::GridLayout* layout = new views::GridLayout(view_);

[please use gerrit instead, 2016/11/22 21:39:32]

Who owns this object? Raw pointers are scary.

If you own it throughout the lifetime of OrderSummaryState, put it in
std::unique_ptr<> member variable.

If you own it initially and then pass ownership to someone else, put it in
std::unique_ptr<> here and then call .release() when you've passed the
ownership.

If you don't own an object, add a comment about it's ownership. This point does
not apply to |layout|, because you own it when you created it on line 26.

[anthonyvd, 2016/11/22 22:11:18]

The view owns the layout manager implicitly. From SetLayoutManage: "The
LayoutManager is owned by the View".

I can change it to a unique_ptr and release it in the line below, although
that's pretty unidiomatic for Views code since the views owning their
LayoutManager is part of the contract between the two. Looking at GridLayout
uses in the codebase, it looks like the rest of Chrome does a naked-new followed
by a SetLayoutManager call.

I can see the argument for the unique_ptr but I can also see the argument for
consistency with the rest of the codebase. WDYT?

+    view_->SetLayoutManager(layout);
+    views::ColumnSet* columns = layout->AddColumnSet(0);

[please use gerrit instead, 2016/11/22 21:39:32]

Who owns this object?

[anthonyvd, 2016/11/22 22:11:18]

The GridLayout does, this function returns a non-owned/weak pointer: "GridLayout
takes ownership of the ColumnSet".

+    columns->AddColumn(views::GridLayout::TRAILING, views::GridLayout::CENTER,
+                       0, views::GridLayout::USE_PREF, 0, 0);
+
+    layout->StartRow(0, 0);
+    layout->AddView(new views::Label(base::ASCIIToUTF16("Order Summary")));
+
+    layout->StartRow(0, 0);
+    views::LabelButton* back_button =

[please use gerrit instead, 2016/11/22 21:39:32]

Who owns this object?

[anthonyvd, 2016/11/22 22:11:19]

Views are always owned by their parent unless set_owned_by_client() is called
(https://cs.chromium.org/chromium/src/ui/views/view.h?rcl=0&l=169). As long as a
view is added to the hierarchy, it's lifetime is managed by the views stack.

+        views::MdTextButton::CreateSecondaryUiBlueButton(
+            this, base::ASCIIToUTF16("Back"));
+    back_button->set_tag(kBackButtonTag);
+    layout->AddView(back_button);
+  }
+  return view_;
+}
+
+void OrderSummaryState::AddObserver(Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void OrderSummaryState::RemoveObserver(Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
+void OrderSummaryState::ButtonPressed(
+    views::Button* sender, const ui::Event& event) {
+  if (!interactable())
+    return;
+
+  if (sender->tag() == kBackButtonTag) {
+    for (auto& observer : observers_) {
+      observer.OnBackClicked();
+    }
+  }
+}
+
+}  // namespace payments