WebOTP: Use WeakPtr to hold onto RenderFrameHost

Change-Id: Id9ab078652ae626170b51fb5e6697808de67f34a
Bug: 1068395
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2141114Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Commit-Queue: Ayu Ishii <ayui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757875}

content/browser/frame_host/render_frame_host_impl.cc

@@ -6966,7 +6966,8 @@ void RenderFrameHostImpl::BindSmsReceiverReceiver(
     mojo::ReportBadMessage("Must have the same origin as the top-level frame.");
     return;
   }
-  auto* fetcher = SmsFetcher::Get(GetProcess()->GetBrowserContext(), this);
+  auto* fetcher = SmsFetcher::Get(GetProcess()->GetBrowserContext(),
+                                  weak_ptr_factory_.GetWeakPtr());
   SmsService::Create(fetcher, this, std::move(receiver));
 }
 

content/browser/sms/sms_fetcher_impl.cc

@@ -38,12 +38,13 @@ SmsFetcher* SmsFetcher::Get(BrowserContext* context) {
       context->GetUserData(kSmsFetcherImplKeyName));
 }
 
-SmsFetcher* SmsFetcher::Get(BrowserContext* context, RenderFrameHost* rfh) {
+SmsFetcher* SmsFetcher::Get(BrowserContext* context,
+                            base::WeakPtr<RenderFrameHost> rfh) {
   auto* stored_fetcher = static_cast<SmsFetcherImpl*>(
       context->GetUserData(kSmsFetcherImplKeyName));
   if (!stored_fetcher || !stored_fetcher->CanReceiveSms()) {
-    auto fetcher =
-        std::make_unique<SmsFetcherImpl>(context, SmsProvider::Create(rfh));
+    auto fetcher = std::make_unique<SmsFetcherImpl>(
+        context, SmsProvider::Create(std::move(rfh)));
     context->SetUserData(kSmsFetcherImplKeyName, std::move(fetcher));
   }
   return static_cast<SmsFetcherImpl*>(

content/browser/sms/sms_provider.cc

@@ -24,14 +24,15 @@ SmsProvider::SmsProvider() = default;
 SmsProvider::~SmsProvider() = default;
 
 // static
-std::unique_ptr<SmsProvider> SmsProvider::Create(RenderFrameHost* rfh) {
+std::unique_ptr<SmsProvider> SmsProvider::Create(
+    base::WeakPtr<RenderFrameHost> rfh) {
 #if defined(OS_ANDROID)
   if (base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
           switches::kWebOtpBackend) ==
       switches::kWebOtpBackendSmsVerification) {
     return std::make_unique<SmsProviderGmsVerification>();
   }
-  return std::make_unique<SmsProviderGmsUserConsent>(rfh);
+  return std::make_unique<SmsProviderGmsUserConsent>(std::move(rfh));
 #else
   return nullptr;
 #endif

content/browser/sms/sms_provider.h

@@ -8,6 +8,7 @@
 #include <memory>
 
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/observer_list_types.h"
 #include "content/browser/sms/sms_parser.h"
@@ -40,7 +41,8 @@ class CONTENT_EXPORT SmsProvider {
   // it is received or (exclusively) when it timeouts.
   virtual void Retrieve() = 0;
 
-  static std::unique_ptr<SmsProvider> Create(RenderFrameHost* rfh);
+  static std::unique_ptr<SmsProvider> Create(
+      base::WeakPtr<RenderFrameHost> rfh);
 
   void AddObserver(Observer*);
   void RemoveObserver(const Observer*);

content/browser/sms/sms_provider_gms_user_consent.cc

@@ -19,8 +19,9 @@ using base::android::ConvertJavaStringToUTF8;
 
 namespace content {
 
-SmsProviderGmsUserConsent::SmsProviderGmsUserConsent(RenderFrameHost* rfh)
-    : SmsProvider(), render_frame_host_(rfh) {
+SmsProviderGmsUserConsent::SmsProviderGmsUserConsent(
+    base::WeakPtr<RenderFrameHost> rfh)
+    : SmsProvider(), render_frame_host_(std::move(rfh)) {
   // This class is constructed a single time whenever the
   // first web page uses the SMS Retriever API to wait for
   // SMSes.
@@ -35,13 +36,15 @@ SmsProviderGmsUserConsent::~SmsProviderGmsUserConsent() {
 }
 
 void SmsProviderGmsUserConsent::Retrieve() {
-  JNIEnv* env = AttachCurrentThread();
+  if (!render_frame_host_)
+    return;
 
   WebContents* web_contents =
-      WebContents::FromRenderFrameHost(render_frame_host_);
+      WebContents::FromRenderFrameHost(render_frame_host_.get());
   if (!web_contents || !web_contents->GetTopLevelNativeWindow())
     return;
 
+  JNIEnv* env = AttachCurrentThread();
   Java_SmsUserConsentReceiver_listen(
       env, j_sms_receiver_,
       web_contents->GetTopLevelNativeWindow()->GetJavaObject());

content/browser/sms/sms_provider_gms_user_consent.h

@@ -17,7 +17,7 @@ namespace content {
 
 class CONTENT_EXPORT SmsProviderGmsUserConsent : public SmsProvider {
  public:
-  SmsProviderGmsUserConsent(RenderFrameHost* rfh);
+  SmsProviderGmsUserConsent(base::WeakPtr<RenderFrameHost> rfh);
   ~SmsProviderGmsUserConsent() override;
 
   void Retrieve() override;
@@ -30,7 +30,7 @@ class CONTENT_EXPORT SmsProviderGmsUserConsent : public SmsProvider {
 
  private:
   base::android::ScopedJavaGlobalRef<jobject> j_sms_receiver_;
-  RenderFrameHost* const render_frame_host_;
+  const base::WeakPtr<RenderFrameHost> render_frame_host_;
 
   DISALLOW_COPY_AND_ASSIGN(SmsProviderGmsUserConsent);
 };

content/browser/sms/sms_provider_gms_user_consent_unittest.cc

@@ -9,6 +9,7 @@
 #include "base/android/jni_string.h"
 #include "base/android/scoped_java_ref.h"
 #include "base/test/scoped_feature_list.h"
+#include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/browser/sms/sms_provider.h"
 #include "content/browser/sms/sms_provider_gms_user_consent.h"
 #include "content/public/common/content_features.h"
@@ -49,7 +50,7 @@ class SmsProviderGmsUserConsentTest : public RenderViewHostTestHarness {
   void SetUp() {
     RenderViewHostTestHarness::SetUp();
     provider_ = std::make_unique<SmsProviderGmsUserConsent>(
-        web_contents()->GetMainFrame());
+        static_cast<RenderFrameHostImpl*>(main_rfh())->GetWeakPtr());
     j_fake_sms_retriever_client_.Reset(
         Java_FakeSmsUserConsentRetrieverClient_create(AttachCurrentThread()));
     Java_SmsUserConsentFakes_setUserConsentClientForTesting(

content/public/browser/sms_fetcher.h

@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "base/memory/weak_ptr.h"
 #include "base/observer_list_types.h"
 #include "content/common/content_export.h"
 
@@ -30,7 +31,7 @@ class SmsFetcher {
   // Retrieval for devices that have telephony capabilities and can receive
   // SMSes coming from the installed device locally. (eg. Android phones)
   CONTENT_EXPORT static SmsFetcher* Get(BrowserContext* context,
-                                        RenderFrameHost* rfh);
+                                        base::WeakPtr<RenderFrameHost> rfh);
 
   class Subscriber : public base::CheckedObserver {
    public: