New link rel=prerender api, using WebKit::WebPrerenderingPlatform

chrome/browser/prerender/prerender_manager.cc

Index: chrome/browser/prerender/prerender_manager.cc
diff --git a/chrome/browser/prerender/prerender_manager.cc b/chrome/browser/prerender/prerender_manager.cc
index 6d91e39e09daeb5a650dc5ab0f78043594eb3e92..24ab71dd54000baa0f2ccb2d573f9989d3a9539b 100644
--- a/chrome/browser/prerender/prerender_manager.cc
+++ b/chrome/browser/prerender/prerender_manager.cc
@@ -4,7 +4,9 @@
 
 #include "chrome/browser/prerender/prerender_manager.h"
 
+#include <set>
 #include <string>
+#include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ -175,9 +177,11 @@ PrerenderManager::PrerenderManagerMode PrerenderManager::mode_ =
 struct PrerenderManager::PrerenderContentsData {
   PrerenderContents* contents_;
   base::Time start_time_;
+  int add_count_;

[dominich, 2012/04/29 18:52:55]

active_count_?

[gavinp, 2012/04/30 11:43:16]

Done.

   PrerenderContentsData(PrerenderContents* contents, base::Time start_time)
       : contents_(contents),
-        start_time_(start_time) {
+        start_time_(start_time),
+        add_count_(1) {
     CHECK(contents);
   }
 };
@@ -274,17 +278,26 @@ bool PrerenderManager::AddPrerenderFromLinkRelPrerender(
     int process_id,
     int route_id,
     const GURL& url,
-    const content::Referrer& referrer) {
+    const content::Referrer& referrer,
+    const gfx::Size& size,
+    SessionStorageNamespace* session_storage_namespace) {
 #if defined(OS_ANDROID)
   // TODO(jcivelli): http://crbug.com/113322 We should have an option to disable
   //                link-prerender and enable omnibox-prerender only.
   return false;
 #else
-  std::pair<int, int> child_route_id_pair = std::make_pair(process_id,
-                                                           route_id);
-
-  return AddPrerender(ORIGIN_LINK_REL_PRERENDER, child_route_id_pair,
-                      url, referrer, NULL);
+  std::pair<int, int> child_route_id_pair(process_id, route_id);
+  PrerenderContentsDataList::iterator it =
+      FindPrerenderContentsForChildRouteIdPair(child_route_id_pair);
+  if (it != prerender_list_.end()) {
+    // Instead of prerendering from inside of a running prerender, we will defer
+    // this request until its launcher is made visible.
+    it->contents_->AddPendingPrerender(url, referrer);
+    return true;
+  }
+  return AddPrerender(ORIGIN_LINK_REL_PRERENDER,
+                      process_id, url, referrer, size,
+                      session_storage_namespace);
 #endif
 }
 
@@ -293,8 +306,19 @@ bool PrerenderManager::AddPrerenderFromOmnibox(
     SessionStorageNamespace* session_storage_namespace) {
   if (!IsOmniboxEnabled(profile_))
     return false;
-  return AddPrerender(ORIGIN_OMNIBOX, std::make_pair(-1, -1), url,
-                      content::Referrer(), session_storage_namespace);
+  return AddPrerender(ORIGIN_OMNIBOX, -1, url,
+                      content::Referrer(), gfx::Size(0, 0),
+                      session_storage_namespace);
+}
+
+void PrerenderManager::MaybeCancelPrerender(const GURL& url) {
+  PrerenderContentsDataList::iterator it = FindPrerenderContentsForURL(url);
+  if (it == prerender_list_.end())
+    return;
+  PrerenderContentsData& prerender_contents_data = *it;
+  if (--prerender_contents_data.add_count_ > 0)

[dominich, 2012/04/29 18:52:55]

why not:

if (--prerender_contents_data.add_count_ == 0)
  prerender_contents_data.contents_->Destroy(FINAL_STATUS_CANCELLED);

[gavinp, 2012/04/30 11:43:16]

Done.

+    return;
+  prerender_contents_data.contents_->Destroy(FINAL_STATUS_CANCELLED);
 }
 
 void PrerenderManager::DestroyPrerenderForRenderView(
@@ -823,9 +847,10 @@ void PrerenderManager::DoShutdown() {
 // private
 bool PrerenderManager::AddPrerender(
     Origin origin,
-    const std::pair<int, int>& child_route_id_pair,
+    int process_id,
     const GURL& url_arg,
     const content::Referrer& referrer,
+    const gfx::Size& size,
     SessionStorageNamespace* session_storage_namespace) {
   DCHECK(CalledOnValidThread());
 
@@ -837,15 +862,6 @@ bool PrerenderManager::AddPrerender(
     origin = ORIGIN_GWS_PRERENDER;
   }
 
-  // If the referring page is prerendering, defer the prerender.
-  PrerenderContentsDataList::iterator source_prerender =
-      FindPrerenderContentsForChildRouteIdPair(child_route_id_pair);
-  if (source_prerender != prerender_list_.end()) {
-    source_prerender->contents_->AddPendingPrerender(
-        origin, url_arg, referrer);
-    return true;
-  }
-
   DeleteOldEntries();
   DeletePendingDeleteEntries();
 
@@ -860,9 +876,10 @@ bool PrerenderManager::AddPrerender(
 
   uint8 experiment = GetQueryStringBasedExperiment(url_arg);
 
-  if (FindEntry(url)) {
+  if (PrerenderContentsData* prerender_contents_data = FindEntryData(url)) {
+    ++prerender_contents_data->add_count_;
     RecordFinalStatus(origin, experiment, FINAL_STATUS_DUPLICATE);

[dominich, 2012/04/29 18:52:55]

this is going to mess with the stats. If the prerender is eventually used, it
wouldn't be captured as USED. Maybe we shouldn't record a final status for
these?

[gavinp, 2012/04/30 11:43:16]

Isn't this already exactly the case on pages that launch two prerenders of the
same target?  One is OK, one fails FINAL_STATUS_DUPLICATE, and on navigation,
one gets USED?  Maybe the behaviour is broken, but I don't think this change
breaks it.

[dominich, 2012/04/30 15:52:05]

You're correct, but we do have to be careful with the refcounting here.
Currently, Omnibox calls a specific method on PrerenderManager to cancel all
Omnibox prerenders. We might want to be more surgical should we start multiple
prerenders from the Omnibox and cancel a given URL (PrerenderHandle, whatever).
When that happens, it will be tempting to minimize the API surface and remove
the Omnibox-specific cancellation method and instead use MaybeCancelPrerender.
However this will check the active count before cancelling but, in the case of
the Omnibox, this is incorrect as we can request multiple prerenders for the
same URL but from the same source, unlike the case of <link>. In that case, we
want to cancel the prerender no matter what the active count. In a way, the
active_count should actually be the number of independent sources that have
requested a particular URL.

It's fine for this CL, but would be a very simple mistake for someone (me, when
I will have forgotten about this code review in a month or two)  to make. I
wonder if we can protect against it somehow, either by specifically not
incrementing active_count for Omnibox origin, or something smarter.

-    return false;
+    return true;
   }
 
   // Do not prerender if there are too many render processes, and we would
@@ -893,25 +910,6 @@ bool PrerenderManager::AddPrerender(
     return false;
   }
 
-  RenderViewHost* source_render_view_host = NULL;
-  if (child_route_id_pair.first != -1) {
-    source_render_view_host =
-        RenderViewHost::FromID(child_route_id_pair.first,
-                               child_route_id_pair.second);
-    // Don't prerender page if parent RenderViewHost no longer exists, or it has
-    // no view.  The latter should only happen when the RenderView has closed.
-    if (!source_render_view_host || !source_render_view_host->GetView()) {
-      RecordFinalStatus(origin, experiment,
-                        FINAL_STATUS_SOURCE_RENDER_VIEW_CLOSED);
-      return false;
-    }
-  }
-
-  if (!session_storage_namespace && source_render_view_host) {
-    session_storage_namespace =
-        source_render_view_host->GetSessionStorageNamespace();
-  }
-
   PrerenderContents* prerender_contents = CreatePrerenderContents(
       url, referrer, origin, experiment);
   if (!prerender_contents || !prerender_contents->Init())
@@ -927,8 +925,8 @@ bool PrerenderManager::AddPrerender(
   last_prerender_start_time_ = GetCurrentTimeTicks();
 
   if (!IsControlGroup()) {
-    data.contents_->StartPrerendering(source_render_view_host,
-                                      session_storage_namespace);
+    data.contents_->StartPrerendering(process_id,
+                                      size, session_storage_namespace);
   }
   while (prerender_list_.size() > config_.max_elements) {
     data = prerender_list_.front();
@@ -1079,6 +1077,15 @@ void PrerenderManager::DeletePendingDeleteEntries() {
   }
 }
 
+PrerenderManager::PrerenderContentsData* PrerenderManager::FindEntryData(
+    const GURL& url) {
+  DCHECK(CalledOnValidThread());
+  PrerenderContentsDataList::iterator it = FindPrerenderContentsForURL(url);
+  if (it == prerender_list_.end())
+    return NULL;
+  return address_of(*it);
+}
+
 PrerenderContents* PrerenderManager::FindEntry(const GURL& url) const {
   DCHECK(CalledOnValidThread());
   for (PrerenderContentsDataList::const_iterator it = prerender_list_.begin();
@@ -1091,7 +1098,7 @@ PrerenderContents* PrerenderManager::FindEntry(const GURL& url) const {
   return NULL;
 }
 
-std::list<PrerenderManager::PrerenderContentsData>::iterator
+PrerenderManager::PrerenderContentsDataList::iterator
     PrerenderManager::FindPrerenderContentsForChildRouteIdPair(
         const std::pair<int, int>& child_route_id_pair) {
   PrerenderContentsDataList::iterator it = prerender_list_.begin();
@@ -1113,6 +1120,16 @@ std::list<PrerenderManager::PrerenderContentsData>::iterator
   return it;
 }
 
+PrerenderManager::PrerenderContentsDataList::iterator
+    PrerenderManager::FindPrerenderContentsForURL(const GURL& url) {
+  for (PrerenderContentsDataList::iterator it = prerender_list_.begin();
+       it != prerender_list_.end(); ++it) {
+    if (it->contents_->MatchesURL(url, NULL))
+      return it;
+  }
+  return prerender_list_.end();
+}
+
 bool PrerenderManager::DoesRateLimitAllowPrerender() const {
   DCHECK(CalledOnValidThread());
   base::TimeDelta elapsed_time =