[WebPayments] Factor out sheet-specific logic in Controllers.

chrome/browser/ui/views/payments/order_summary_view_controller.h

Index: chrome/browser/ui/views/payments/order_summary_view_controller.h
diff --git a/chrome/browser/ui/views/payments/order_summary_view_controller.h b/chrome/browser/ui/views/payments/order_summary_view_controller.h
new file mode 100644
index 0000000000000000000000000000000000000000..ab82388ffeeabdadef395bf4c7ee8f1e89301b0b
--- /dev/null
+++ b/chrome/browser/ui/views/payments/order_summary_view_controller.h
@@ -0,0 +1,39 @@
+// 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.
+
+#ifndef CHROME_BROWSER_UI_VIEWS_PAYMENTS_ORDER_SUMMARY_VIEW_CONTROLLER_H_
+#define CHROME_BROWSER_UI_VIEWS_PAYMENTS_ORDER_SUMMARY_VIEW_CONTROLLER_H_
+
+#include "chrome/browser/ui/views/payments/payment_request_sheet_controller.h"
+

[please use gerrit instead, 2016/12/15 19:11:14]

no newline here necessary.

[anthonyvd, 2016/12/15 19:55:06]

Done.

+#include "ui/views/controls/button/button.h"
+
+namespace views {
+class View;

[please use gerrit instead, 2016/12/15 19:11:14]

No need to forward-declare or #include in the header the types that are used
only to override parent methods.

[anthonyvd, 2016/12/15 19:55:05]

Remnants from a previous patch, good catch! Done.

+}
+
+namespace payments {
+
+// The PaymentRequestSheetController subtype for the Order Summary screen of the
+// Payment Request flow.
+class OrderSummaryViewController : public PaymentRequestSheetController,
+                                   public views::ButtonListener {
+ public:
+  OrderSummaryViewController(PaymentRequestImpl* impl,

[please use gerrit instead, 2016/12/15 19:11:14]

Forward declare PaymentRequestImpl.
Add a comment about who owns the impl and dialog params.

[anthonyvd, 2016/12/15 19:55:06]

This is a similar situation than the previous comment about forward-declares for
overrides. Should those two be forward-declared only because ctors aren't
actually overrides?

In any case, done.

As for the ownership comment, added it to the base class.

[please use gerrit instead, 2016/12/15 20:29:33]

Exactly.
Thx.
Would be nice to have it in the child class as well. When I'm reading this file
on its own, I would have to first look into .cc and then into
payment_request_sheet_controller.h to figure out what happens to the two
pointers. The comment does not need to be fancy or long. This is usually
sufficient:

// Does not take ownership of the arguments, which should outlive this object.

[anthonyvd, 2016/12/15 20:58:08]

Fair point, added the shorter-form comment to the concrete implementations.

+                             PaymentRequestDialog* dialog);

[please use gerrit instead, 2016/12/15 19:11:14]

Forward declare PaymentRequestDialog.

[anthonyvd, 2016/12/15 19:55:05]

See previous comment, done.

+  ~OrderSummaryViewController() override;
+
+  // PaymentRequestSheetController:
+  std::unique_ptr<views::View> CreateView() override;
+
+ private:
+  // views::ButtonListener:
+  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
+
+  DISALLOW_COPY_AND_ASSIGN(OrderSummaryViewController);

[please use gerrit instead, 2016/12/15 19:11:14]

#include "base/macros.h"

[anthonyvd, 2016/12/15 19:55:05]

Done.

+};
+
+}  // namespace payments
+
+#endif  // CHROME_BROWSER_UI_VIEWS_PAYMENTS_ORDER_SUMMARY_VIEW_CONTROLLER_H_