Refactor renderer-side prerender code.

This change teases apart the code that runs in a renderer that observes
link tags from the code that runs in a renderer used to perform no-state
prefetching. That translates to moving the PrefetchFinished signal from
PrerenderDispatcher to PrerenderHelper.

This change also goes one step further and, recognizing that
PrerenderHelper is a RenderFrameObserver, ditches the PrefetchFinished
method on WebPrerenderingSupport in favor of just observing
DidFinishDocumentLoad (aka DOMContentLoaded). The timing of the two
signals is approximately the same and for what the no-state prefetching
code requires, this seems sufficient.

URLLoaderThrottleProvider is simplified by moving most of the logic
associated with creating the PrerenderURLLoaderThrottle moved into
PrerenderHelper, further consolidating logic around the throttles (again
used only by the renderer process being used to drive the no-state
prefetch).

There's a slight change to the calculation of the start time for the
parse time histogram. It should not be material, and if anything, this
should be a less errorprone way of calculating it. Having it align to
the creation of the PrerenderHelper means that we aren't reliant on
prefetch happening in a newly created process to have a properly
computed start time. Hopefully this change does not goof up any metrics.

This CL sets the stage for onion-souping the renderer code used to
observe link tags, kick-off prefetches and observe their activity. We
should be able to eliminate WebPrerenderingSupport and WebPrerender,
for example.

Change-Id: Ia337c74484d8ff9fd8e1fdb57f0a3e282a03b8df
Bug: 967834
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2032424
Commit-Queue: Darin Fisher <darin@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Robert Ogden <robertogden@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737559}

chrome/renderer/prerender/prerender_dispatcher.cc

@@ -10,7 +10,6 @@
 
 #include "base/bind.h"
 #include "base/logging.h"
-#include "base/metrics/histogram_macros.h"
 #include "chrome/common/prerender_messages.h"
 #include "chrome/common/prerender_types.h"
 #include "chrome/renderer/prerender/prerender_extra_data.h"
@@ -29,8 +28,7 @@ namespace prerender {
 using blink::WebPrerender;
 using blink::WebPrerenderingSupport;
 
-PrerenderDispatcher::PrerenderDispatcher()
-    : process_start_time_(base::TimeTicks::Now()) {
+PrerenderDispatcher::PrerenderDispatcher() {
   WebPrerenderingSupport::Initialize(this);
 }
 
@@ -38,19 +36,6 @@ PrerenderDispatcher::~PrerenderDispatcher() {
   WebPrerenderingSupport::Shutdown();
 }
 
-void PrerenderDispatcher::IncrementPrefetchCount() {
-  prefetch_count_++;
-}
-
-void PrerenderDispatcher::DecrementPrefetchCount() {
-  if (!--prefetch_count_ && prefetch_finished_) {
-    UMA_HISTOGRAM_MEDIUM_TIMES(
-        "Prerender.NoStatePrefetchRendererLifetimeExtension",
-        base::TimeTicks::Now() - prefetch_parsed_time_);
-    content::RenderThread::Get()->Send(new PrerenderHostMsg_PrefetchFinished());
-  }
-}
-
 void PrerenderDispatcher::PrerenderStart(int prerender_id) {
   auto it = prerenders_.find(prerender_id);
   if (it == prerenders_.end())
@@ -176,16 +161,4 @@ void PrerenderDispatcher::Abandon(const WebPrerender& prerender) {
   prerenders_.erase(extra_data.prerender_id());
 }
 
-void PrerenderDispatcher::PrefetchFinished() {
-  prefetch_parsed_time_ = base::TimeTicks::Now();
-  if (prefetch_count_) {
-    prefetch_finished_ = true;
-  } else {
-    UMA_HISTOGRAM_MEDIUM_TIMES(
-        "Prerender.NoStatePrefetchRendererParseTime",
-        prefetch_parsed_time_ - process_start_time_);
-    content::RenderThread::Get()->Send(new PrerenderHostMsg_PrefetchFinished());
-  }
-}
-
 }  // namespace prerender

chrome/renderer/prerender/prerender_dispatcher.h

@@ -33,12 +33,7 @@ class PrerenderDispatcher : public content::RenderThreadObserver,
   PrerenderDispatcher();
   ~PrerenderDispatcher() override;
 
-  void IncrementPrefetchCount();
-  void DecrementPrefetchCount();
-
  private:
-  friend class PrerenderDispatcherTest;
-
   // chrome::mojom::PrerenderDispatcher:
   void PrerenderStart(int prerender_id) override;
   void PrerenderStopLoading(int prerender_id) override;
@@ -59,16 +54,10 @@ class PrerenderDispatcher : public content::RenderThreadObserver,
   void Add(const blink::WebPrerender& prerender) override;
   void Cancel(const blink::WebPrerender& prerender) override;
   void Abandon(const blink::WebPrerender& prerender) override;
-  void PrefetchFinished() override;
 
   // From WebKit, prerender elements launched by renderers in our process.
   std::map<int, blink::WebPrerender> prerenders_;
 
-  int prefetch_count_ = 0;
-  bool prefetch_finished_ = false;
-  base::TimeTicks process_start_time_;
-  base::TimeTicks prefetch_parsed_time_;
-
   mojo::AssociatedReceiverSet<chrome::mojom::PrerenderDispatcher> receivers_;
 };
 

chrome/renderer/prerender/prerender_helper.cc

@@ -5,12 +5,15 @@
 #include "chrome/renderer/prerender/prerender_helper.h"
 
 #include "base/metrics/field_trial.h"
-#include "base/metrics/histogram.h"
+#include "base/metrics/histogram_macros.h"
+#include "chrome/common/prerender.mojom.h"
 #include "chrome/common/prerender_messages.h"
 #include "chrome/common/prerender_url_loader_throttle.h"
 #include "content/public/renderer/document_state.h"
 #include "content/public/renderer/render_frame.h"
+#include "content/public/renderer/render_thread.h"
 #include "content/public/renderer/render_view.h"
+#include "third_party/blink/public/common/browser_interface_broker_proxy.h"
 #include "third_party/blink/public/web/web_frame.h"
 #include "third_party/blink/public/web/web_view.h"
 
@@ -22,15 +25,34 @@ PrerenderHelper::PrerenderHelper(content::RenderFrame* render_frame,
     : content::RenderFrameObserver(render_frame),
       content::RenderFrameObserverTracker<PrerenderHelper>(render_frame),
       prerender_mode_(prerender_mode),
-      histogram_prefix_(histogram_prefix) {
+      histogram_prefix_(histogram_prefix),
+      start_time_(base::TimeTicks::Now()) {
   DCHECK_NE(prerender_mode_, NO_PRERENDER);
 }
 
 PrerenderHelper::~PrerenderHelper() = default;
 
-void PrerenderHelper::AddThrottle(
-    const base::WeakPtr<PrerenderURLLoaderThrottle>& throttle) {
-  throttles_.push_back(throttle);
+// static
+std::unique_ptr<blink::URLLoaderThrottle> PrerenderHelper::MaybeCreateThrottle(
+    int render_frame_id) {
+  content::RenderFrame* render_frame =
+      content::RenderFrame::FromRoutingID(render_frame_id);
+  auto* prerender_helper =
+      render_frame ? PrerenderHelper::Get(
+                         render_frame->GetRenderView()->GetMainRenderFrame())
+                   : nullptr;
+  if (!prerender_helper)
+    return nullptr;
+
+  mojo::PendingRemote<chrome::mojom::PrerenderCanceler> canceler;
+  render_frame->GetBrowserInterfaceBroker()->GetInterface(
+      canceler.InitWithNewPipeAndPassReceiver());
+
+  auto throttle = std::make_unique<PrerenderURLLoaderThrottle>(
+      prerender_helper->prerender_mode(), prerender_helper->histogram_prefix(),
+      std::move(canceler));
+  prerender_helper->AddThrottle(throttle->AsWeakPtr());
+  return throttle;
 }
 
 // static.
@@ -49,6 +71,15 @@ PrerenderMode PrerenderHelper::GetPrerenderMode(
   return helper->prerender_mode_;
 }
 
+void PrerenderHelper::DidFinishDocumentLoad() {
+  if (prerender_mode_ != PREFETCH_ONLY)
+    return;
+  parsed_time_ = base::TimeTicks::Now();
+  prefetch_finished_ = true;
+  if (prefetch_count_ == 0)
+    SendPrefetchFinished();
+}
+
 bool PrerenderHelper::OnMessageReceived(
     const IPC::Message& message) {
   IPC_BEGIN_MESSAGE_MAP(PrerenderHelper, message)
@@ -59,6 +90,10 @@ bool PrerenderHelper::OnMessageReceived(
   return false;
 }
 
+void PrerenderHelper::OnDestruct() {
+  delete this;
+}
+
 void PrerenderHelper::OnSetIsPrerendering(PrerenderMode mode,
                                           const std::string& histogram_prefix) {
   // Immediately after construction, |this| may receive the message that
@@ -66,21 +101,49 @@ void PrerenderHelper::OnSetIsPrerendering(PrerenderMode mode,
   if (mode != prerender::NO_PRERENDER)
     return;
 
-  auto throttles = std::move(throttles_);
+  std::vector<base::WeakPtr<PrerenderURLLoaderThrottle>> throttles =
+      std::move(throttles_);
 
   // |this| must be deleted so PrerenderHelper::IsPrerendering returns false
   // when the visibility state is updated, so the visibility state string will
   // not be "prerendered".
   delete this;
 
-  for (auto resource : throttles) {
-    if (resource)
-      resource->PrerenderUsed();
+  for (auto& throttle : throttles) {
+    if (throttle)
+      throttle->PrerenderUsed();
   }
 }
 
-void PrerenderHelper::OnDestruct() {
-  delete this;
+void PrerenderHelper::AddThrottle(
+    const base::WeakPtr<PrerenderURLLoaderThrottle>& throttle) {
+  // Keep track of how many pending throttles we have, as we want to defer
+  // sending the "prefetch finished" signal until they are destroyed. This is
+  // important since that signal tells the browser that it can tear down this
+  // renderer which could interrupt subresource prefetching.
+  if (prerender_mode_ == PREFETCH_ONLY) {
+    prefetch_count_++;
+    throttle->set_destruction_closure(base::BindOnce(
+        &PrerenderHelper::OnThrottleDestroyed, weak_factory_.GetWeakPtr()));
+  }
+  throttles_.push_back(throttle);
+}
+
+void PrerenderHelper::OnThrottleDestroyed() {
+  if (--prefetch_count_ == 0 && prefetch_finished_) {
+    UMA_HISTOGRAM_MEDIUM_TIMES(
+        "Prerender.NoStatePrefetchRendererLifetimeExtension",
+        base::TimeTicks::Now() - parsed_time_);
+    SendPrefetchFinished();
+  }
+}
+
+void PrerenderHelper::SendPrefetchFinished() {
+  DCHECK(prefetch_count_ == 0 && prefetch_finished_);
+  UMA_HISTOGRAM_MEDIUM_TIMES("Prerender.NoStatePrefetchRendererParseTime",
+                             parsed_time_ - start_time_);
+  // TODO(darin): Perhaps this should be a routed message (frame level).
+  content::RenderThread::Get()->Send(new PrerenderHostMsg_PrefetchFinished());
 }
 
 }  // namespace prerender

chrome/renderer/prerender/prerender_helper.h

@@ -7,11 +7,16 @@
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "base/time/time.h"
 #include "chrome/common/prerender_types.h"
 #include "content/public/renderer/render_frame_observer.h"
 #include "content/public/renderer/render_frame_observer_tracker.h"
 
+namespace blink {
+class URLLoaderThrottle;
+}  // namespace blink
+
 namespace prerender {
 class PrerenderURLLoaderThrottle;
 
@@ -27,33 +32,46 @@ class PrerenderHelper
 
   ~PrerenderHelper() override;
 
-  // Called when a PrerenderURLLoaderThrottle is created for a resource for
-  // this frame.
-  void AddThrottle(const base::WeakPtr<PrerenderURLLoaderThrottle>& throttle);
-
-  PrerenderMode prerender_mode() const { return prerender_mode_; }
-  std::string histogram_prefix() const { return histogram_prefix_; }
+  // Configures and returns a new PrerenderURLLoaderThrottle instance if the
+  // indicated frame has an associated PrerenderHelper.
+  static std::unique_ptr<blink::URLLoaderThrottle> MaybeCreateThrottle(
+      int render_frame_id);
 
-  // Returns true if |render_view| is currently prerendering.
+  // Returns true if |render_frame| is currently prerendering.
   static bool IsPrerendering(const content::RenderFrame* render_frame);
 
   static PrerenderMode GetPrerenderMode(
       const content::RenderFrame* render_frame);
 
+  PrerenderMode prerender_mode() const { return prerender_mode_; }
+  std::string histogram_prefix() const { return histogram_prefix_; }
+
  private:
   // RenderFrameObserver implementation.
+  void DidFinishDocumentLoad() override;
   bool OnMessageReceived(const IPC::Message& message) override;
   void OnDestruct() override;
 
   void OnSetIsPrerendering(PrerenderMode mode,
                            const std::string& histogram_prefix);
 
+  void AddThrottle(const base::WeakPtr<PrerenderURLLoaderThrottle>& throttle);
+  void OnThrottleDestroyed();
+  void SendPrefetchFinished();
+
   PrerenderMode prerender_mode_;
   std::string histogram_prefix_;
 
   // Pending requests for this frame..
   std::vector<base::WeakPtr<PrerenderURLLoaderThrottle>> throttles_;
 
+  int prefetch_count_ = 0;
+  bool prefetch_finished_ = false;
+  base::TimeTicks start_time_;
+  base::TimeTicks parsed_time_;
+
+  base::WeakPtrFactory<PrerenderHelper> weak_factory_{this};
+
   DISALLOW_COPY_AND_ASSIGN(PrerenderHelper);
 };
 

chrome/renderer/url_loader_throttle_provider_impl.cc

@@ -11,11 +11,8 @@
 #include "base/feature_list.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/common/google_url_loader_throttle.h"
-#include "chrome/common/prerender.mojom.h"
-#include "chrome/common/prerender_url_loader_throttle.h"
 #include "chrome/renderer/chrome_content_renderer_client.h"
 #include "chrome/renderer/chrome_render_thread_observer.h"
-#include "chrome/renderer/prerender/prerender_dispatcher.h"
 #include "chrome/renderer/prerender/prerender_helper.h"
 #include "chrome/renderer/subresource_redirect/subresource_redirect_params.h"
 #include "chrome/renderer/subresource_redirect/subresource_redirect_url_loader_throttle.h"
@@ -44,14 +41,6 @@
 
 namespace {
 
-mojo::PendingRemote<chrome::mojom::PrerenderCanceler> GetPrerenderCanceler(
-    content::RenderFrame* render_frame) {
-  mojo::PendingRemote<chrome::mojom::PrerenderCanceler> canceler;
-  render_frame->GetBrowserInterfaceBroker()->GetInterface(
-      canceler.InitWithNewPipeAndPassReceiver());
-  return canceler;
-}
-
 #if BUILDFLAG(ENABLE_EXTENSIONS)
 std::unique_ptr<extensions::ExtensionThrottleManager>
 CreateExtensionThrottleManager() {
@@ -181,28 +170,10 @@ URLLoaderThrottleProviderImpl::CreateThrottles(
 
   if (type_ == content::URLLoaderThrottleProviderType::kFrame &&
       !is_frame_resource) {
-    content::RenderFrame* render_frame =
-        content::RenderFrame::FromRoutingID(render_frame_id);
-    auto* prerender_helper =
-        render_frame ? prerender::PrerenderHelper::Get(
-                           render_frame->GetRenderView()->GetMainRenderFrame())
-                     : nullptr;
-    if (prerender_helper) {
-      auto throttle = std::make_unique<prerender::PrerenderURLLoaderThrottle>(
-          prerender_helper->prerender_mode(),
-          prerender_helper->histogram_prefix(),
-          GetPrerenderCanceler(render_frame));
-      prerender_helper->AddThrottle(throttle->AsWeakPtr());
-      if (prerender_helper->prerender_mode() == prerender::PREFETCH_ONLY) {
-        auto* prerender_dispatcher =
-            chrome_content_renderer_client_->prerender_dispatcher();
-        prerender_dispatcher->IncrementPrefetchCount();
-        throttle->set_destruction_closure(base::BindOnce(
-            &prerender::PrerenderDispatcher::DecrementPrefetchCount,
-            base::Unretained(prerender_dispatcher)));
-      }
+    auto throttle =
+        prerender::PrerenderHelper::MaybeCreateThrottle(render_frame_id);
+    if (throttle)
       throttles.push_back(std::move(throttle));
-    }
   }
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)

third_party/blink/public/platform/web_prerendering_support.h

@@ -55,11 +55,6 @@ class WebPrerenderingSupport {
   // it after, for instance, a short redirect chain.
   virtual void Abandon(const WebPrerender&) = 0;
 
-  // Called when the current page has finished requesting early discoverable
-  // resources for prefetch. In prefetch mode link elements do not initiate any
-  // prerenders.
-  virtual void PrefetchFinished() = 0;
-
  protected:
   WebPrerenderingSupport() = default;
   virtual ~WebPrerenderingSupport() = default;

third_party/blink/renderer/core/dom/document.cc

@@ -6406,9 +6406,6 @@ void Document::FinishedParsing() {
 
   // Parser should have picked up all preloads by now
   fetcher_->ClearPreloads(ResourceFetcher::kClearSpeculativeMarkupPreloads);
-
-  if (IsPrefetchOnly())
-    WebPrerenderingSupport::Current()->PrefetchFinished();
 }
 
 void Document::ElementDataCacheClearTimerFired(TimerBase*) {

third_party/blink/renderer/core/exported/prerendering_test.cc

@@ -152,8 +152,6 @@ class TestPrerenderingSupport : public WebPrerenderingSupport {
     abandoned_prerenders_.push_back(prerender);
   }
 
-  void PrefetchFinished() override {}
-
   Vector<WebPrerender> added_prerenders_;
   Vector<WebPrerender> canceled_prerenders_;
   Vector<WebPrerender> abandoned_prerenders_;

third_party/blink/renderer/core/html/parser/html_document_parser_test.cc

@@ -37,12 +37,6 @@ class MockWebPrerenderingSupport : public WebPrerenderingSupport {
   void Add(const WebPrerender&) override {}
   void Cancel(const WebPrerender&) override {}
   void Abandon(const WebPrerender&) override {}
-  void PrefetchFinished() override { prefetch_finished_ = true; }
-
-  bool IsPrefetchFinished() const { return prefetch_finished_; }
-
- private:
-  bool prefetch_finished_ = false;
 };
 
 class HTMLDocumentParserTest : public PageTestBase {
@@ -61,10 +55,6 @@ class HTMLDocumentParserTest : public PageTestBase {
     return parser;
   }
 
-  bool PrefetchFinishedCleanly() {
-    return prerendering_support_.IsPrefetchFinished();
-  }
-
  private:
   MockWebPrerenderingSupport prerendering_support_;
 };
@@ -90,7 +80,6 @@ TEST_F(HTMLDocumentParserTest, AppendPrefetch) {
   // DCHECK).
   static_cast<DocumentParser*>(parser)->Finish();
   EXPECT_EQ(HTMLTokenizer::kDataState, parser->Tokenizer()->GetState());
-  EXPECT_TRUE(PrefetchFinishedCleanly());
 }
 
 TEST_F(HTMLDocumentParserTest, AppendNoPrefetch) {