Ensure SecurePaymentConfirmationAppFactory owns InternalAuthenticator

This patch ensures that the InternalAuthenticator created by
SecurePaymentConfirmationAppFactory is always owned by a
WebContentsObserver, to ensure that the authenticator is outlived by
its RenderFrameHost.

Bug: 1169317
Change-Id: Ie22c3796d642589f912b880bcdadeb3badde8180
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2644743
Commit-Queue: Nick Burris <nburris@chromium.org>
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Auto-Submit: Nick Burris <nburris@chromium.org>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#846426}

components/payments/content/secure_payment_confirmation_app_factory.cc

@@ -72,13 +72,13 @@ void SecurePaymentConfirmationAppFactory::
     OnIsUserVerifyingPlatformAuthenticatorAvailable(
         base::WeakPtr<PaymentAppFactory::Delegate> delegate,
         mojom::SecurePaymentConfirmationRequestPtr request,
-        std::unique_ptr<autofill::InternalAuthenticator> authenticator,
         bool is_available) {
   if (!delegate || !delegate->GetWebContents())
     return;
 
-  if (!is_available && !base::FeatureList::IsEnabled(
-                           features::kSecurePaymentConfirmationDebug)) {
+  if (!authenticator_ ||
+      (!is_available && !base::FeatureList::IsEnabled(
+                            features::kSecurePaymentConfirmationDebug))) {
     delegate->OnDoneCreatingPaymentApps();
     return;
   }
@@ -98,8 +98,9 @@ void SecurePaymentConfirmationAppFactory::
   WebDataServiceBase::Handle handle =
       web_data_service->GetSecurePaymentConfirmationInstruments(
           std::move(request->credential_ids), this);
-  requests_[handle] = std::make_unique<Request>(
-      delegate, web_data_service, std::move(request), std::move(authenticator));
+  requests_[handle] =
+      std::make_unique<Request>(delegate, web_data_service, std::move(request),
+                                std::move(authenticator_));
 }
 
 SecurePaymentConfirmationAppFactory::SecurePaymentConfirmationAppFactory()
@@ -133,16 +134,16 @@ void SecurePaymentConfirmationAppFactory::Create(
         return;
       }
 
-      std::unique_ptr<autofill::InternalAuthenticator> authenticator =
-          delegate->CreateInternalAuthenticator();
-      auto* authenticator_ptr = authenticator.get();
+      // Observe the web contents to ensure the authenticator outlives it.
+      Observe(delegate->GetWebContents());
 
-      authenticator_ptr->IsUserVerifyingPlatformAuthenticatorAvailable(
+      authenticator_ = delegate->CreateInternalAuthenticator();
+
+      authenticator_->IsUserVerifyingPlatformAuthenticatorAvailable(
           base::BindOnce(&SecurePaymentConfirmationAppFactory::
                              OnIsUserVerifyingPlatformAuthenticatorAvailable,
                          weak_ptr_factory_.GetWeakPtr(), delegate,
-                         method_data->secure_payment_confirmation.Clone(),
-                         std::move(authenticator)));
+                         method_data->secure_payment_confirmation.Clone()));
       return;
     }
   }
@@ -150,6 +151,14 @@ void SecurePaymentConfirmationAppFactory::Create(
   delegate->OnDoneCreatingPaymentApps();
 }
 
+void SecurePaymentConfirmationAppFactory::RenderFrameDeleted(
+    content::RenderFrameHost* render_frame_host) {
+  if (authenticator_ &&
+      authenticator_->GetRenderFrameHost() == render_frame_host) {
+    authenticator_.reset();
+  }
+}
+
 struct SecurePaymentConfirmationAppFactory::Request
     : public content::WebContentsObserver {
   Request(
@@ -168,6 +177,15 @@ struct SecurePaymentConfirmationAppFactory::Request
   Request(const Request& other) = delete;
   Request& operator=(const Request& other) = delete;
 
+  // WebContentsObserver:
+  void RenderFrameDeleted(
+      content::RenderFrameHost* render_frame_host) override {
+    if (authenticator &&
+        authenticator->GetRenderFrameHost() == render_frame_host) {
+      authenticator.reset();
+    }
+  }
+
   base::WeakPtr<PaymentAppFactory::Delegate> delegate;
   scoped_refptr<payments::PaymentManifestWebDataService> web_data_service;
   mojom::SecurePaymentConfirmationRequestPtr mojo_request;
@@ -224,7 +242,7 @@ void SecurePaymentConfirmationAppFactory::OnAppIconDecoded(
     const SkBitmap& decoded_icon) {
   DCHECK(request);
   if (!request->delegate || !request->web_contents() ||
-      !request->delegate->GetSpec() ||
+      !request->delegate->GetSpec() || !request->authenticator ||
       request->authenticator->GetRenderFrameHost() !=
           request->web_contents()->GetMainFrame()) {
     request->delegate->OnDoneCreatingPaymentApps();

components/payments/content/secure_payment_confirmation_app_factory.h

@@ -11,13 +11,16 @@
 #include "base/memory/weak_ptr.h"
 #include "components/payments/content/payment_app_factory.h"
 #include "components/webdata/common/web_data_service_consumer.h"
+#include "content/public/browser/web_contents_observer.h"
 
 namespace payments {
 
 struct SecurePaymentConfirmationInstrument;
 
-class SecurePaymentConfirmationAppFactory : public PaymentAppFactory,
-                                            public WebDataServiceConsumer {
+class SecurePaymentConfirmationAppFactory
+    : public PaymentAppFactory,
+      public WebDataServiceConsumer,
+      public content::WebContentsObserver {
  public:
   SecurePaymentConfirmationAppFactory();
   ~SecurePaymentConfirmationAppFactory() override;
@@ -38,10 +41,12 @@ class SecurePaymentConfirmationAppFactory : public PaymentAppFactory,
       WebDataServiceBase::Handle handle,
       std::unique_ptr<WDTypedResult> result) override;
 
+  // WebContentsObserver:
+  void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override;
+
   void OnIsUserVerifyingPlatformAuthenticatorAvailable(
       base::WeakPtr<PaymentAppFactory::Delegate> delegate,
       mojom::SecurePaymentConfirmationRequestPtr request,
-      std::unique_ptr<autofill::InternalAuthenticator> authenticator,
       bool is_available);
 
   void OnAppIconDecoded(
@@ -49,6 +54,8 @@ class SecurePaymentConfirmationAppFactory : public PaymentAppFactory,
       std::unique_ptr<Request> request,
       const SkBitmap& decoded_image);
 
+  std::unique_ptr<autofill::InternalAuthenticator> authenticator_;
+
   std::map<WebDataServiceBase::Handle, std::unique_ptr<Request>> requests_;
   base::WeakPtrFactory<SecurePaymentConfirmationAppFactory> weak_ptr_factory_{
       this};