Cleanup plumbing of chrome::mojom::PrerenderCanceler

Three changes:

1- Avoid returning a raw pointer to chrome::mojom::PrerenderCanceler
   from GetPrerenderCanceller helper functions in favor returning a
   mojo::PendingRemote<chrome::mojom::PrerenderCanceler>. This avoids
   the need to schedule a DeleteSoon (in the renderer case) to cleanup
   the memory associated with that object later.

2- By returning a mojo::PendingRemote<..> instead of a raw pointer to
   the interface, it is no longer necessary to manually PostTask before
   accessing that interface. A mojo::PendingRemote<..> can instead be
   bound to a mojo::Remote<..> that implicitly schedules function calls
   on the right thread. The PostTask is implicit.

3- Make PrerenderContents support having multiple bound cancelers. This
   makes the above possible. In the renderer, the canceler is acquired
   immediately instead of lazily, and that is extra cost (an extra IPC)
   in the case where a canceler is not needed. This only happens for a
   page being prerendered, and so that overhead should be relatively
   small and seems worth the benefit of simple code.

Change-Id: If43a1f6fa607f07eded8511639898ea99a023cd6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2023358
Commit-Queue: Darin Fisher <darin@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736455}

chrome/browser/chrome_browser_interface_binders.cc

@@ -238,7 +238,7 @@ void BindPrerenderCanceler(
       prerender::PrerenderContents::FromWebContents(web_contents);
   if (!prerender_contents)
     return;
-  prerender_contents->OnPrerenderCancelerReceiver(std::move(receiver));
+  prerender_contents->AddPrerenderCancelerReceiver(std::move(receiver));
 }
 
 void BindDocumentCoordinationUnit(

chrome/browser/chrome_content_browser_client.cc

@@ -967,14 +967,12 @@ bool URLHasExtensionBackgroundPermission(
 
 #endif
 
-chrome::mojom::PrerenderCanceler* GetPrerenderCanceller(
+mojo::PendingRemote<chrome::mojom::PrerenderCanceler> GetPrerenderCanceler(
     const base::Callback<content::WebContents*()>& wc_getter) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  auto* web_contents = wc_getter.Run();
-  if (!web_contents)
-    return nullptr;
-
-  return prerender::PrerenderContents::FromWebContents(web_contents);
+  mojo::PendingRemote<chrome::mojom::PrerenderCanceler> canceler;
+  prerender::PrerenderContents::FromWebContents(wc_getter.Run())
+      ->AddPrerenderCancelerReceiver(canceler.InitWithNewPipeAndPassReceiver());
+  return canceler;
 }
 
 void LaunchURL(const GURL& url,
@@ -4306,8 +4304,7 @@ ChromeContentBrowserClient::CreateURLLoaderThrottles(
     result.push_back(std::make_unique<prerender::PrerenderURLLoaderThrottle>(
         chrome_navigation_ui_data->prerender_mode(),
         chrome_navigation_ui_data->prerender_histogram_prefix(),
-        base::BindOnce(GetPrerenderCanceller, wc_getter),
-        base::CreateSingleThreadTaskRunner({BrowserThread::UI})));
+        GetPrerenderCanceler(wc_getter)));
   }
 
   signin::IdentityManager* identity_manager =

chrome/browser/prerender/prerender_contents.cc

@@ -719,10 +719,9 @@ void PrerenderContents::CancelPrerenderForUnsupportedScheme(const GURL& url) {
   ReportUnsupportedPrerenderScheme(url);
 }
 
-void PrerenderContents::OnPrerenderCancelerReceiver(
+void PrerenderContents::AddPrerenderCancelerReceiver(
     mojo::PendingReceiver<chrome::mojom::PrerenderCanceler> receiver) {
-  if (!prerender_canceler_receiver_.is_bound())
-    prerender_canceler_receiver_.Bind(std::move(receiver));
+  prerender_canceler_receiver_set_.Add(this, std::move(receiver));
 }
 
 void PrerenderContents::AddNetworkBytes(int64_t bytes) {

chrome/browser/prerender/prerender_contents.h

@@ -26,8 +26,7 @@
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/referrer.h"
-#include "mojo/public/cpp/bindings/pending_receiver.h"
-#include "mojo/public/cpp/bindings/receiver.h"
+#include "mojo/public/cpp/bindings/receiver_set.h"
 #include "ui/gfx/geometry/rect.h"
 #include "url/origin.h"
 
@@ -233,7 +232,7 @@ class PrerenderContents : public content::NotificationObserver,
   // Running byte count. Increased when each resource completes loading.
   int64_t network_bytes() { return network_bytes_; }
 
-  void OnPrerenderCancelerReceiver(
+  void AddPrerenderCancelerReceiver(
       mojo::PendingReceiver<chrome::mojom::PrerenderCanceler> receiver);
 
  protected:
@@ -295,8 +294,8 @@ class PrerenderContents : public content::NotificationObserver,
   void CancelPrerenderForUnsupportedMethod() override;
   void CancelPrerenderForUnsupportedScheme(const GURL& url) override;
 
-  mojo::Receiver<chrome::mojom::PrerenderCanceler> prerender_canceler_receiver_{
-      this};
+  mojo::ReceiverSet<chrome::mojom::PrerenderCanceler>
+      prerender_canceler_receiver_set_;
 
   base::ObserverList<Observer>::Unchecked observer_list_;
 

chrome/common/prerender_url_loader_throttle.cc

@@ -8,6 +8,7 @@
 #include "build/build_config.h"
 #include "chrome/common/prerender_util.h"
 #include "content/public/common/content_constants.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #include "net/base/load_flags.h"
 #include "net/url_request/redirect_info.h"
 #include "services/network/public/cpp/resource_request.h"
@@ -20,19 +21,17 @@ namespace {
 const char kPurposeHeaderName[] = "Purpose";
 const char kPurposeHeaderValue[] = "prefetch";
 
-void CancelPrerenderForUnsupportedMethod(
-    PrerenderURLLoaderThrottle::CancelerGetterCallback callback) {
-  chrome::mojom::PrerenderCanceler* canceler = std::move(callback).Run();
-  if (canceler)
-    canceler->CancelPrerenderForUnsupportedMethod();
+void CallCancelPrerenderForUnsupportedMethod(
+    mojo::PendingRemote<chrome::mojom::PrerenderCanceler> canceler) {
+  mojo::Remote<chrome::mojom::PrerenderCanceler>(std::move(canceler))
+      ->CancelPrerenderForUnsupportedMethod();
 }
 
-void CancelPrerenderForUnsupportedScheme(
-    PrerenderURLLoaderThrottle::CancelerGetterCallback callback,
+void CallCancelPrerenderForUnsupportedScheme(
+    mojo::PendingRemote<chrome::mojom::PrerenderCanceler> canceler,
     const GURL& url) {
-  chrome::mojom::PrerenderCanceler* canceler = std::move(callback).Run();
-  if (canceler)
-    canceler->CancelPrerenderForUnsupportedScheme(url);
+  mojo::Remote<chrome::mojom::PrerenderCanceler>(std::move(canceler))
+      ->CancelPrerenderForUnsupportedScheme(url);
 }
 
 // Returns true if the response has a "no-store" cache control header.
@@ -46,12 +45,11 @@ bool IsNoStoreResponse(const network::mojom::URLResponseHead& response_head) {
 PrerenderURLLoaderThrottle::PrerenderURLLoaderThrottle(
     PrerenderMode mode,
     const std::string& histogram_prefix,
-    CancelerGetterCallback canceler_getter,
-    scoped_refptr<base::SequencedTaskRunner> canceler_getter_task_runner)
+    mojo::PendingRemote<chrome::mojom::PrerenderCanceler> canceler)
     : mode_(mode),
       histogram_prefix_(histogram_prefix),
-      canceler_getter_(std::move(canceler_getter)),
-      canceler_getter_task_runner_(canceler_getter_task_runner) {
+      canceler_(std::move(canceler)) {
+  DCHECK(canceler_);
 }
 
 PrerenderURLLoaderThrottle::~PrerenderURLLoaderThrottle() {
@@ -90,9 +88,7 @@ void PrerenderURLLoaderThrottle::WillStartRequest(
     // prefetch going.
     delegate_->CancelWithError(net::ERR_ABORTED);
     if (mode_ == DEPRECATED_FULL_PRERENDER) {
-      canceler_getter_task_runner_->PostTask(
-          FROM_HERE, base::BindOnce(CancelPrerenderForUnsupportedMethod,
-                                    std::move(canceler_getter_)));
+      CallCancelPrerenderForUnsupportedMethod(std::move(canceler_));
       return;
     }
   }
@@ -107,9 +103,7 @@ void PrerenderURLLoaderThrottle::WillStartRequest(
     // WillRedirectRequest() and PrerenderContents::CheckURL(). See
     // http://crbug.com/673771.
     delegate_->CancelWithError(net::ERR_ABORTED);
-    canceler_getter_task_runner_->PostTask(
-        FROM_HERE, base::BindOnce(CancelPrerenderForUnsupportedScheme,
-                                  std::move(canceler_getter_), request->url));
+    CallCancelPrerenderForUnsupportedScheme(std::move(canceler_), request->url);
     return;
   }
 
@@ -166,10 +160,8 @@ void PrerenderURLLoaderThrottle::WillRedirectRequest(
   // Abort any prerenders with requests which redirect to invalid schemes.
   if (!DoesURLHaveValidScheme(redirect_info->new_url)) {
     delegate_->CancelWithError(net::ERR_ABORTED);
-    canceler_getter_task_runner_->PostTask(
-        FROM_HERE,
-        base::BindOnce(CancelPrerenderForUnsupportedScheme,
-                       std::move(canceler_getter_), redirect_info->new_url));
+    CallCancelPrerenderForUnsupportedScheme(std::move(canceler_),
+                                            redirect_info->new_url);
   } else if (follow_only_when_prerender_shown_header == "1" &&
              resource_type_ != content::ResourceType::kMainFrame) {
     // Only defer redirects with the Follow-Only-When-Prerender-Shown

chrome/common/prerender_url_loader_throttle.h

@@ -22,15 +22,10 @@ class PrerenderURLLoaderThrottle
     : public blink::URLLoaderThrottle,
       public base::SupportsWeakPtr<PrerenderURLLoaderThrottle> {
  public:
-  // If the throttle needs to cancel the prerender, it will run
-  // |canceler_getter| on |canceler_getter_task_runner| to do so.
-  using CancelerGetterCallback =
-      base::OnceCallback<chrome::mojom::PrerenderCanceler*()>;
   PrerenderURLLoaderThrottle(
       PrerenderMode mode,
       const std::string& histogram_prefix,
-      CancelerGetterCallback canceler_getter,
-      scoped_refptr<base::SequencedTaskRunner> canceler_getter_task_runner);
+      mojo::PendingRemote<chrome::mojom::PrerenderCanceler> canceler);
   ~PrerenderURLLoaderThrottle() override;
 
   // Called when the prerender is used. This will unpaused requests and set the
@@ -64,8 +59,7 @@ class PrerenderURLLoaderThrottle
   int redirect_count_ = 0;
   content::ResourceType resource_type_;
 
-  CancelerGetterCallback canceler_getter_;
-  scoped_refptr<base::SequencedTaskRunner> canceler_getter_task_runner_;
+  mojo::PendingRemote<chrome::mojom::PrerenderCanceler> canceler_;
 
   // The throttle changes most request priorities to IDLE during prerendering.
   // The priority is reset back to the original priority when prerendering is

chrome/renderer/url_loader_throttle_provider_impl.cc

@@ -44,21 +44,12 @@
 
 namespace {
 
-chrome::mojom::PrerenderCanceler* GetPrerenderCanceller(int render_frame_id) {
-  content::RenderFrame* render_frame =
-      content::RenderFrame::FromRoutingID(render_frame_id);
-  if (!render_frame)
-    return nullptr;
-  prerender::PrerenderHelper* helper =
-      prerender::PrerenderHelper::Get(render_frame);
-  if (!helper)
-    return nullptr;
-
-  auto* canceler = new mojo::Remote<chrome::mojom::PrerenderCanceler>;
+mojo::PendingRemote<chrome::mojom::PrerenderCanceler> GetPrerenderCanceler(
+    content::RenderFrame* render_frame) {
+  mojo::PendingRemote<chrome::mojom::PrerenderCanceler> canceler;
   render_frame->GetBrowserInterfaceBroker()->GetInterface(
-      canceler->BindNewPipeAndPassReceiver());
-  base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE, canceler);
-  return canceler->get();
+      canceler.InitWithNewPipeAndPassReceiver());
+  return canceler;
 }
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
@@ -200,8 +191,7 @@ URLLoaderThrottleProviderImpl::CreateThrottles(
       auto throttle = std::make_unique<prerender::PrerenderURLLoaderThrottle>(
           prerender_helper->prerender_mode(),
           prerender_helper->histogram_prefix(),
-          base::BindOnce(GetPrerenderCanceller, render_frame_id),
-          base::ThreadTaskRunnerHandle::Get());
+          GetPrerenderCanceler(render_frame));
       prerender_helper->AddThrottle(throttle->AsWeakPtr());
       if (prerender_helper->prerender_mode() == prerender::PREFETCH_ONLY) {
         auto* prerender_dispatcher =