[Web Payments] Initial plumb of Payment Request dialog

chrome/browser/payments/payment_request_impl.cc

Index: chrome/browser/payments/payment_request_impl.cc
diff --git a/chrome/browser/payments/payment_request_impl.cc b/chrome/browser/payments/payment_request_impl.cc
new file mode 100644
index 0000000000000000000000000000000000000000..5704a0da70b93d59ff6366c6ad533a72b3bc49c8
--- /dev/null
+++ b/chrome/browser/payments/payment_request_impl.cc
@@ -0,0 +1,90 @@
+// 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/payments/payment_request_impl.h"
+
+#include <set>
+
+#include "base/lazy_instance.h"
+#include "base/synchronization/lock.h"

[Lei Zhang, 2016/11/16 00:59:33]

No longer needed.

[Kevin Bailey, 2016/11/16 14:13:47]

Done. Odd, git cl lint doesn't complain.

[Lei Zhang, 2016/11/16 18:39:30]

git cl lint is not a "find what #includes you don't need" tool.

[Kevin Bailey, 2016/11/16 22:34:09]

As I stated. My point was, since it does do 'iwyu',
maybe it should.

+#include "chrome/browser/payments/ui/payment_request_dialog.h"
+#include "components/web_modal/web_contents_modal_dialog_host.h"
+#include "components/web_modal/web_contents_modal_dialog_manager.h"
+#include "components/web_modal/web_contents_modal_dialog_manager_delegate.h"
+#include "content/public/browser/web_contents.h"
+
+namespace {
+
+class PaymentRequestFactory {
+ public:
+  bool AssignPaymentRequest(
+      content::WebContents* web_contents,
+      mojo::InterfaceRequest<payments::mojom::PaymentRequest> request) {
+    if (set_.find(web_contents) == set_.end()) {
+      scoped_refptr<payments::PaymentRequestImpl> p(
+          new payments::PaymentRequestImpl(web_contents, std::move(request)));
+      set_.insert(web_contents);
+      p->AddRef();
+      return true;
+    }
+    return false;
+  }
+
+  void UnassignPaymentRequest(content::WebContents* web_contents) {
+    set_.erase(web_contents);
+  }
+
+ private:
+  std::set<content::WebContents*> set_;

[Lei Zhang, 2016/11/16 00:59:33]

If you make this a map of: content::WebContents* to
std::unique_ptr<PaymentRequestImpl>, then you don't need refcounting, and
there's clear ownership. You'll have to call UnassignPaymentRequest() from
onError().

[Kevin Bailey, 2016/11/16 14:13:47]

Funny that you mention it, but it was a map<> and I changed it
because the binding, for Mojo below, seems to prefer ref-counting,
I suppose to prevent referencing a deleted object. I assumed this
is what people preferred. Not true?

[Lei Zhang, 2016/11/16 18:39:30]

I haven't played with Mojo in a while. Ok, how about the same map suggestion,
but with a scoped_refptr instead of a unique_ptr? Basically avoid the need for
manual AddRef/Release calls.

[Lei Zhang, 2016/11/16 18:43:19]

FWIW, most of the set_connection_error_handler() calls found in Code Search
passes pointers wrapped in base::Unretained(). I don't know Mojo well enough to
tell you why. Maybe ask the Mojo experts if a refcounted object is really needed
here?

[Kevin Bailey, 2016/11/16 22:34:09]

Works for me, since it prevents one without the other; just realize
that it only swaps e.g. Release() for something else.

BTW, I realize that "what people preferred" was an ambiguous phrase.
I meant, "the right way" as opposed to "the popular way".

The function takes a base::Closure(), so it's really the binding
setting the policy for ownership, not Mojo.

[Lei Zhang, 2016/11/16 23:02:25]

I don't know what "the right way" is, so I would suggest asking mojo/OWNERS if
refcounting is necessary.
I meant |binding_| is a Mojo thing in general.

+};
+
+base::LazyInstance<PaymentRequestFactory> payment_request_factory;
+
+}  // namespace
+
+namespace payments {
+
+PaymentRequestImpl::PaymentRequestImpl(
+    content::WebContents* web_contents,
+    mojo::InterfaceRequest<payments::mojom::PaymentRequest> request)
+    : web_contents_(web_contents),
+      binding_(this, std::move(request)),
+      dialog_(nullptr) {
+  binding_.set_connection_error_handler(
+      base::Bind(&PaymentRequestImpl::onError, this));
+}
+
+PaymentRequestImpl::~PaymentRequestImpl() {
+  payment_request_factory.Get().UnassignPaymentRequest(web_contents_);
+}
+
+void PaymentRequestImpl::Init(
+    payments::mojom::PaymentRequestClientPtr client,
+    std::vector<payments::mojom::PaymentMethodDataPtr> methodData,
+    payments::mojom::PaymentDetailsPtr details,
+    payments::mojom::PaymentOptionsPtr options) {
+  dialog_ = new PaymentRequestDialog(std::move(client));
+  views::DialogDelegate::CreateDialogWidget(
+      dialog_, nullptr,
+      web_modal::WebContentsModalDialogManager::FromWebContents(web_contents_)
+          ->delegate()
+          ->GetWebContentsModalDialogHost()
+          ->GetHostView())
+      ->Show();
+}
+
+void PaymentRequestImpl::onError() {
+  binding_.Close();
+  // TODO(krb): Call dialog_->Close() here, but avoid double-free
+  Release();
+}
+
+}  // namespace payments
+
+void CreatePaymentRequestHandler(
+    content::WebContents* web_contents,
+    mojo::InterfaceRequest<payments::mojom::PaymentRequest> request) {
+  payment_request_factory.Get().AssignPaymentRequest(web_contents,
+                                                     std::move(request));
+}