Handle interface to prerenders.

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 5d0409120bd9b15b85f9f769da5bb525017f5737..673f0a67f72e175bc9165dc570cbced3c41a8d48 100644
--- a/chrome/browser/prerender/prerender_manager.cc
+++ b/chrome/browser/prerender/prerender_manager.cc
@@ -4,6 +4,8 @@
 
 #include "chrome/browser/prerender/prerender_manager.h"
 
+#include <algorithm>
+#include <functional>
 #include <set>
 #include <string>

[mmenke, 2012/07/09 18:06:57]

While you're here, set doesn't look to be used in this file, and string is
included in the header file.

[gavinp, 2012/07/11 17:04:00]

Done.

 #include <vector>
@@ -106,10 +108,8 @@ bool NeedMatchCompleteDummyForFinalStatus(FinalStatus final_status) {
       final_status != FINAL_STATUS_MANAGER_SHUTDOWN &&
       final_status != FINAL_STATUS_APP_TERMINATING &&
       final_status != FINAL_STATUS_WINDOW_OPENER &&
-      final_status != FINAL_STATUS_FRAGMENT_MISMATCH &&
       final_status != FINAL_STATUS_CACHE_OR_HISTORY_CLEARED &&
       final_status != FINAL_STATUS_CANCELLED &&
-      final_status != FINAL_STATUS_SESSION_STORAGE_NAMESPACE_MISMATCH &&
       final_status != FINAL_STATUS_DEVTOOLS_ATTACHED &&
       final_status != FINAL_STATUS_CROSS_SITE_NAVIGATION_PENDING;
 }
@@ -171,18 +171,6 @@ int PrerenderManager::prerenders_per_session_count_ = 0;
 PrerenderManager::PrerenderManagerMode PrerenderManager::mode_ =
     PRERENDER_MODE_ENABLED;
 
-struct PrerenderManager::PrerenderContentsData {
-  PrerenderContents* contents_;
-  base::Time start_time_;
-  int active_count_;
-  PrerenderContentsData(PrerenderContents* contents, base::Time start_time)
-      : contents_(contents),
-        start_time_(start_time),
-        active_count_(1) {
-    CHECK(contents);
-  }
-};
-
 struct PrerenderManager::NavigationRecord {
   GURL url_;
   base::TimeTicks time_;
@@ -217,7 +205,7 @@ void PrerenderManager::Shutdown() {
   DoShutdown();
 }
 
-bool PrerenderManager::AddPrerenderFromLinkRelPrerender(
+PrerenderHandle* PrerenderManager::AddPrerenderFromLinkRelPrerender(
     int process_id,
     int route_id,
     const GURL& url,
@@ -228,14 +216,19 @@ bool PrerenderManager::AddPrerenderFromLinkRelPrerender(
   //                link-prerender and enable omnibox-prerender only.
   return false;
 #else
-  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()) {
+  if (PrerenderHandle::PrerenderData* parent_prerender_data =
+          FindPrerenderDataForChildAndRoute(process_id, route_id)) {
     // 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, size);
-    return true;
+    if (PrerenderContents* contents = parent_prerender_data->contents) {
+      pending_prerender_list_.push_back(PrerenderHandle::PrerenderData(this));
+      PrerenderHandle* prerender_handle = CreatePrerenderHandleForPrerenderData(
+          &pending_prerender_list_.back());
+      contents->AddPendingPrerender(
+          prerender_handle->weak_ptr_factory_.GetWeakPtr(),
+          url, referrer, size);
+      return prerender_handle;
+    }
   }
 
   // Unit tests pass in a process_id == -1.
@@ -244,7 +237,7 @@ bool PrerenderManager::AddPrerenderFromLinkRelPrerender(
     RenderViewHost* source_render_view_host =
         RenderViewHost::FromID(process_id, route_id);
     if (!source_render_view_host || !source_render_view_host->GetView())
-      return false;
+      return NULL;
     session_storage_namespace =
         source_render_view_host->GetSessionStorageNamespace();
 
@@ -266,44 +259,68 @@ bool PrerenderManager::AddPrerenderFromLinkRelPrerender(
 #endif
 }
 
-bool PrerenderManager::AddPrerenderFromOmnibox(
+PrerenderHandle* PrerenderManager::AddPrerenderFromOmnibox(
     const GURL& url,
     SessionStorageNamespace* session_storage_namespace,
     gfx::Size size) {
   if (!IsOmniboxEnabled(profile_))
-    return false;
-  return AddPrerender(ORIGIN_OMNIBOX, -1, url,
-                      content::Referrer(), size,
+    return NULL;
+  return AddPrerender(ORIGIN_OMNIBOX, -1, url, content::Referrer(), size,
                       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.active_count_ == 0)
-    prerender_contents_data.contents_->Destroy(FINAL_STATUS_CANCELLED);
+void PrerenderManager::StartPendingPrerender(
+    PrerenderHandle* existing_prerender_handle,
+    Origin origin,
+    int process_id,
+    const GURL& url,
+    const content::Referrer& referrer,
+    const gfx::Size& size,
+    content::SessionStorageNamespace* session_storage_namespace) {
+  DCHECK(existing_prerender_handle);
+  DCHECK(existing_prerender_handle->IsValid());
+  DCHECK(existing_prerender_handle->IsPending());
+
+  DCHECK(process_id == -1 || session_storage_namespace);
+
+  scoped_ptr<PrerenderHandle>
+      new_prerender_handle(AddPrerender(origin, process_id, url, referrer,
+                                        size, session_storage_namespace));

[mmenke, 2012/07/10 18:01:13]

IMPORTANT:  AddPerender() can return NULL.  Neither this function nor
SwapPrerenderDataWith() has a NULL check.  Fix that.  Also probably a good idea
to have a unit test for it.

[mmenke, 2012/07/10 18:01:13]

While this code is pretty straight forward, think this line is worthy of a
comment due to its importance.  Maybe something along the lines of "If |url| is
already being prerendered, this will result creating another handle for the
existing PrerenderData.  If it's not being prerendered, this will start a new
prerender (Or return NULL)."

[gavinp, 2012/07/11 17:04:00]

Done. Very good catch, I'm glad you're reviewing this.

+
+  existing_prerender_handle->SwapPrerenderDataWith(new_prerender_handle.get());
+}
+
+void PrerenderManager::DestroyPendingPrerenderData(
+    PrerenderHandle::PrerenderData* pending_prerender_data) {
+  for (std::list<PrerenderHandle::PrerenderData>::iterator
+           it = pending_prerender_list_.begin();
+       it != pending_prerender_list_.end();
+       ++it) {

[mmenke, 2012/07/10 18:01:13]

std::find would be a little simpler.

[gavinp, 2012/07/11 17:04:00]

It would. If only we had address_of().... But we don't. See
http://codereview.chromium.org/10261004/

I don't believe we can use std::find without this function.

Related rant: if we had std::compose(), I could replace about half of my loops
with find and for_each. Alas. I also wouldn't mind nonstd::member_of(). However,
it's probably best for all of us that we don't have these...

[mmenke, 2012/07/11 18:40:05]

Right.  For some reason, I was thinking the list was of PrerenderData*'s rather
than PrerenderData's.

[mmenke, 2012/07/11 18:48:37]

optional:  May actually be a little cleaner to make it a non-copiable class and
use pointers / delete, rather than make it POD.

+    if (&(*it) == pending_prerender_data) {
+      DCHECK_EQ(0, pending_prerender_data->instance_count);
+      InvalidatePrerenderHandlesForPrerenderData(*it);
+      pending_prerender_list_.erase(it);
+      return;
+    }
+  }
+  NOTREACHED();
 }
 
 void PrerenderManager::DestroyPrerenderForRenderView(
     int process_id, int view_id, FinalStatus final_status) {
   DCHECK(CalledOnValidThread());
-  PrerenderContentsDataList::iterator it =
-      FindPrerenderContentsForChildRouteIdPair(
-          std::make_pair(process_id, view_id));
-  if (it != prerender_list_.end()) {
-    PrerenderContents* prerender_contents = it->contents_;
-    prerender_contents->Destroy(final_status);
+  if (PrerenderHandle::PrerenderData* prerender_data =
+          FindPrerenderDataForChildAndRoute(process_id, view_id)) {
+    prerender_data->contents->Destroy(final_status);
   }
 }
 
 void PrerenderManager::CancelAllPrerenders() {
   DCHECK(CalledOnValidThread());
-  while (!prerender_list_.empty()) {
-    PrerenderContentsData data = prerender_list_.front();
-    DCHECK(data.contents_);
-    data.contents_->Destroy(FINAL_STATUS_CANCELLED);
+  while (!active_prerender_list_.empty()) {
+    PrerenderContents* prerender_contents =
+        active_prerender_list_.front().contents;
+    prerender_contents->Destroy(FINAL_STATUS_CANCELLED);
   }
 }
 
@@ -312,27 +329,39 @@ bool PrerenderManager::MaybeUsePrerenderedPage(WebContents* web_contents,
   DCHECK(CalledOnValidThread());
   DCHECK(!IsWebContentsPrerendering(web_contents));
 
-  scoped_ptr<PrerenderContents> prerender_contents(
-      GetEntryButNotSpecifiedWC(url, web_contents));
-  if (prerender_contents.get() == NULL)
+  RenderViewHost* old_render_view_host = web_contents->GetRenderViewHost();
+
+  DeleteOldEntries();
+  DeletePendingDeleteEntries();
+  PrerenderHandle::PrerenderData* prerender_data =
+      FindPrerenderData(url,
+                        old_render_view_host->GetSessionStorageNamespace());
+  if (!prerender_data)
+    return false;
+  DCHECK(prerender_data->contents);
+  if (IsNoSwapInExperiment(prerender_data->contents->experiment_id()))
     return false;
 
+  if (TabContents* new_tab_contents =
+          prerender_data->contents->prerender_contents()) {
+    if (web_contents == new_tab_contents->web_contents())
+      return false;  // Do not swap in to ourself.
+  }
+
+  scoped_ptr<PrerenderContents> prerender_contents(prerender_data->contents);
+  std::list<PrerenderHandle::PrerenderData>::iterator to_erase =
+      FindIteratorForPrerenderContents(prerender_contents.get());
+  DCHECK(active_prerender_list_.end() != to_erase);
+  DCHECK(&(*to_erase) == prerender_data);
+  InvalidatePrerenderHandlesForPrerenderData(*prerender_data);
+  active_prerender_list_.erase(to_erase);
+
   // Do not use the prerendered version if there is an opener object.
   if (web_contents->HasOpener()) {
     prerender_contents.release()->Destroy(FINAL_STATUS_WINDOW_OPENER);
     return false;
   }
 
-  // Even if we match, the location.hash might be different. Record this as a
-  // separate final status.
-  GURL matching_url;
-  bool url_matches = prerender_contents->MatchesURL(url, &matching_url);
-  DCHECK(url_matches);
-  if (url_matches && url.ref() != matching_url.ref()) {
-    prerender_contents.release()->Destroy(FINAL_STATUS_FRAGMENT_MISMATCH);
-    return false;
-  }
-
   // If we are just in the control group (which can be detected by noticing
   // that prerendering hasn't even started yet), record that |web_contents| now
   // would be showing a prerendered contents, but otherwise, don't do anything.
@@ -359,22 +388,6 @@ bool PrerenderManager::MaybeUsePrerenderedPage(WebContents* web_contents,
     return false;
   }
 
-  // If the session storage namespaces don't match, cancel the prerender.
-  RenderViewHost* old_render_view_host = web_contents->GetRenderViewHost();
-  RenderViewHost* new_render_view_host =
-      prerender_contents->prerender_contents()->web_contents()->
-          GetRenderViewHost();
-  DCHECK(old_render_view_host);
-  DCHECK(new_render_view_host);
-  if (old_render_view_host->GetSessionStorageNamespace() !=
-      new_render_view_host->GetSessionStorageNamespace()) {
-    DestroyAndMarkMatchCompleteAsUsed(
-        prerender_contents.release(),
-        FINAL_STATUS_SESSION_STORAGE_NAMESPACE_MISMATCH);
-    return false;
-  }
-
-  // If we don't want to use prerenders at all, we are done.
   // For bookkeeping purposes, we need to mark this WebContents to
   // reflect that it would have been prerendered.
   if (GetMode() == PRERENDER_MODE_EXPERIMENT_NO_USE_GROUP) {
@@ -393,6 +406,8 @@ bool PrerenderManager::MaybeUsePrerenderedPage(WebContents* web_contents,
   if (!prerender_tracker_->TryUse(child_id, route_id))
     return false;
 
+  // At this point, we've determined that we will use the prerender.
+
   if (!prerender_contents->load_start_time().is_null()) {
     histograms_->RecordTimeUntilUsed(GetCurrentTimeTicks() -
                                      prerender_contents->load_start_time(),
@@ -403,10 +418,17 @@ bool PrerenderManager::MaybeUsePrerenderedPage(WebContents* web_contents,
   histograms_->RecordUsedPrerender(prerender_contents->origin());
   prerender_contents->set_final_status(FINAL_STATUS_USED);
 
+  RenderViewHost* new_render_view_host =
+      prerender_contents->prerender_contents()->web_contents()->
+          GetRenderViewHost();
   new_render_view_host->Send(
       new PrerenderMsg_SetIsPrerendering(new_render_view_host->GetRoutingID(),
                                          false));
 
+  // Start pending prerender requests from the PrerenderContents, if there are
+  // any.
+  prerender_contents->StartPendingPrerenders();
+
   TabContents* new_tab_contents =
       prerender_contents->ReleasePrerenderContents();
   TabContents* old_tab_contents = TabContents::FromWebContents(web_contents);
@@ -439,10 +461,6 @@ bool PrerenderManager::MaybeUsePrerenderedPage(WebContents* web_contents,
   DCHECK(prerender_tab_helper != NULL);
   prerender_tab_helper->PrerenderSwappedIn();
 
-  // Start pending prerender requests from the PrerenderContents, if there are
-  // any.
-  prerender_contents->StartPendingPrerenders();
-
   if (old_tab_contents->web_contents()->NeedToFireBeforeUnload()) {
     // Schedule the delete to occur after the tab has run its unload handlers.
     on_close_tab_contents_deleters_.push_back(
@@ -469,50 +487,47 @@ void PrerenderManager::MoveEntryToPendingDelete(PrerenderContents* entry,
   DCHECK_EQ(0, std::count(pending_delete_list_.begin(),
                           pending_delete_list_.end(), entry));
 
-  for (PrerenderContentsDataList::iterator it = prerender_list_.begin();
-       it != prerender_list_.end();
-       ++it) {
-    if (it->contents_ == entry) {
-      bool swapped_in_dummy_replacement = false;
-
-      // If this PrerenderContents is being deleted due to a cancellation,
-      // we need to create a dummy replacement for PPLT accounting purposes
-      // for the Match Complete group.
-      // This is the case if the cancellation is for any reason that would not
-      // occur in the control group case.
-      if (entry->match_complete_status() ==
-          PrerenderContents::MATCH_COMPLETE_DEFAULT &&
-          NeedMatchCompleteDummyForFinalStatus(final_status) &&
-          ActuallyPrerendering()) {
-        // TODO(tburkard): I'd like to DCHECK that we are actually prerendering.
-        // However, what if new conditions are added and
-        // NeedMatchCompleteDummyForFinalStatus, is not being updated.  Not sure
-        // what's the best thing to do here.  For now, I will just check whether
-        // we are actually prerendering.
-        entry->set_match_complete_status(
-            PrerenderContents::MATCH_COMPLETE_REPLACED);
-        if (PrerenderContents* dummy_replacement_prerender_contents =
-                CreatePrerenderContents(entry->prerender_url(),
-                                        entry->referrer(),
-                                        entry->origin(),
-                                        entry->experiment_id())) {
-          dummy_replacement_prerender_contents->set_match_complete_status(
-              PrerenderContents::MATCH_COMPLETE_REPLACEMENT_PENDING);
-          if (!dummy_replacement_prerender_contents->Init())
-            break;
-          dummy_replacement_prerender_contents->
-              AddAliasURLsFromOtherPrerenderContents(entry);
-          dummy_replacement_prerender_contents->set_match_complete_status(
-              PrerenderContents::MATCH_COMPLETE_REPLACEMENT);
-          it->contents_ = dummy_replacement_prerender_contents;
-          swapped_in_dummy_replacement = true;
-        }
-      }
-      if (!swapped_in_dummy_replacement)
-        prerender_list_.erase(it);
-      break;
+  std::list<PrerenderHandle::PrerenderData>::iterator it =
+      FindIteratorForPrerenderContents(entry);
+
+  // If this PrerenderContents is being deleted due to a cancellation,
+  // we need to create a dummy replacement for PPLT accounting purposes
+  // for the Match Complete group.
+  // This is the case if the cancellation is for any reason that would not
+  // occur in the control group case.
+  if (it != active_prerender_list_.end()) {
+    if (entry->match_complete_status() ==
+            PrerenderContents::MATCH_COMPLETE_DEFAULT &&
+        NeedMatchCompleteDummyForFinalStatus(final_status) &&
+        ActuallyPrerendering()) {
+      // TODO(tburkard): I'd like to DCHECK that we are actually prerendering.
+      // However, what if new conditions are added and
+      // NeedMatchCompleteDummyForFinalStatus, is not being updated.  Not sure
+      // what's the best thing to do here.  For now, I will just check whether
+      // we are actually prerendering.
+      entry->set_match_complete_status(
+          PrerenderContents::MATCH_COMPLETE_REPLACED);
+      PrerenderContents* dummy_replacement_prerender_contents =
+          CreatePrerenderContents(entry->prerender_url(), entry->referrer(),
+                                  entry->origin(), entry->experiment_id());
+      dummy_replacement_prerender_contents->MakeIntoDummyReplacementOf(entry);
+      DCHECK(dummy_replacement_prerender_contents);
+
+      dummy_replacement_prerender_contents->set_match_complete_status(
+          PrerenderContents::MATCH_COMPLETE_REPLACEMENT_PENDING);
+      DCHECK(dummy_replacement_prerender_contents->Init());
+      dummy_replacement_prerender_contents->
+          AddAliasURLsFromOtherPrerenderContents(entry);
+      dummy_replacement_prerender_contents->set_match_complete_status(
+          PrerenderContents::MATCH_COMPLETE_REPLACEMENT);
+
+      it->contents = dummy_replacement_prerender_contents;
+    } else {
+      InvalidatePrerenderHandlesForPrerenderData(*it);
+      active_prerender_list_.erase(it);

[mmenke, 2012/07/10 18:01:13]

I'd suggest replacing InvalidatePrerenderHandlesForPrerenderData(*it) with
DeletePrerenderData(list_, *it), and have it do the erasing as well.  I figure
if two operations must always be done together or we could end up with a
difficult to detect leak, it's best to couple them in a function, if possible.

[gavinp, 2012/07/11 17:04:00]

Done, but mooted if we go with the ptr in the struct (see below).

     }
   }
+
   AddToHistory(entry);
   pending_delete_list_.push_back(entry);
 
@@ -623,10 +638,11 @@ bool PrerenderManager::IsNoUseGroup() {
 bool PrerenderManager::IsWebContentsPrerendering(
     WebContents* web_contents) const {
   DCHECK(CalledOnValidThread());
-  for (PrerenderContentsDataList::const_iterator it = prerender_list_.begin();
-       it != prerender_list_.end();
+  for (std::list<PrerenderHandle::PrerenderData>::const_iterator it =
+           active_prerender_list_.begin();
+       it != active_prerender_list_.end();
        ++it) {
-    TabContents* prerender_tab_contents = it->contents_->prerender_contents();
+    TabContents* prerender_tab_contents = it->contents->prerender_contents();
     if (prerender_tab_contents &&
         prerender_tab_contents->web_contents() == web_contents) {
       return true;
@@ -647,12 +663,6 @@ bool PrerenderManager::IsWebContentsPrerendering(
   return false;
 }
 
-bool PrerenderManager::DidPrerenderFinishLoading(const GURL& url) const {
-  DCHECK(CalledOnValidThread());
-  PrerenderContents* contents = FindEntry(url);
-  return contents ? contents->has_finished_loading() : false;
-}
-
 void PrerenderManager::MarkWebContentsAsPrerendered(WebContents* web_contents) {
   DCHECK(CalledOnValidThread());
   prerendered_tab_contents_set_.insert(web_contents);
@@ -768,22 +778,6 @@ void PrerenderManager::AddCondition(const PrerenderCondition* condition) {
   prerender_conditions_.push_back(condition);
 }
 
-bool PrerenderManager::IsPendingEntry(const GURL& url) const {
-  DCHECK(CalledOnValidThread());
-  for (PrerenderContentsDataList::const_iterator it = prerender_list_.begin();
-       it != prerender_list_.end();
-       ++it) {
-    if (it->contents_->IsPendingEntry(url))
-      return true;
-  }
-  return false;
-}
-
-bool PrerenderManager::IsPrerendering(const GURL& url) const {
-  DCHECK(CalledOnValidThread());
-  return (FindEntry(url) != NULL);
-}
-
 void PrerenderManager::RecordNavigation(const GURL& url) {
   DCHECK(CalledOnValidThread());
 
@@ -803,10 +797,14 @@ void PrerenderManager::DoShutdown() {
   STLDeleteElements(&prerender_conditions_);
   on_close_tab_contents_deleters_.reset();
   profile_ = NULL;
+
+  DCHECK(active_prerender_list_.empty());
+  DCHECK(pending_prerender_list_.empty());
+  DCHECK(prerender_weak_ptr_factory_map_.empty());
 }
 
 // private
-bool PrerenderManager::AddPrerender(
+PrerenderHandle* PrerenderManager::AddPrerender(
     Origin origin,
     int process_id,
     const GURL& url_arg,
@@ -816,7 +814,7 @@ bool PrerenderManager::AddPrerender(
   DCHECK(CalledOnValidThread());
 
   if (!IsEnabled())
-    return false;
+    return NULL;
 
   if (origin == ORIGIN_LINK_REL_PRERENDER &&
       IsGoogleSearchResultURL(referrer.url)) {
@@ -840,10 +838,11 @@ bool PrerenderManager::AddPrerender(
   // histogram tracking.
   histograms_->RecordPrerender(origin, url_arg);
 
-  if (PrerenderContentsData* prerender_contents_data = FindEntryData(url)) {
-    ++prerender_contents_data->active_count_;
+  if (PrerenderHandle::PrerenderData* preexisting_prerender_data =
+          FindPrerenderData(url, session_storage_namespace)) {
     RecordFinalStatus(origin, experiment, FINAL_STATUS_DUPLICATE);
-    return true;
+    ++preexisting_prerender_data->instance_count;
+    return CreatePrerenderHandleForPrerenderData(preexisting_prerender_data);
   }
 
   // Do not prerender if there are too many render processes, and we would
@@ -861,7 +860,7 @@ bool PrerenderManager::AddPrerender(
           profile_, url) &&
       !content::RenderProcessHost::run_renderer_in_process()) {
     RecordFinalStatus(origin, experiment, FINAL_STATUS_TOO_MANY_PROCESSES);
-    return false;
+    return NULL;
   }
 #endif
 
@@ -871,64 +870,40 @@ bool PrerenderManager::AddPrerender(
     // this doesn't make sense as the next prerender request will be triggered
     // by a navigation and is unlikely to be the same site.
     RecordFinalStatus(origin, experiment, FINAL_STATUS_RATE_LIMIT_EXCEEDED);
-    return false;
+    return NULL;
   }
 
   PrerenderContents* prerender_contents = CreatePrerenderContents(
       url, referrer, origin, experiment);
   if (!prerender_contents || !prerender_contents->Init())
-    return false;
+    return NULL;
 
   histograms_->RecordPrerenderStarted(origin);
 
   // TODO(cbentzel): Move invalid checks here instead of PrerenderContents?
-  PrerenderContentsData data(prerender_contents, GetCurrentTime());
+  active_prerender_list_.push_back(
+      PrerenderHandle::PrerenderData(this, prerender_contents));
 
-  prerender_list_.push_back(data);
+  PrerenderHandle* prerender_handle =
+      CreatePrerenderHandleForPrerenderData(&active_prerender_list_.back());
 
   last_prerender_start_time_ = GetCurrentTimeTicks();
 
   if (size.IsEmpty())
     size = config_.default_tab_bounds.size();
 
-  data.contents_->StartPrerendering(process_id, size, session_storage_namespace,
-                                    control_group_behavior);
+  prerender_contents->StartPrerendering(process_id, size,
+                                        session_storage_namespace,
+                                        control_group_behavior);
 
-  while (prerender_list_.size() > config_.max_elements) {
-    data = prerender_list_.front();
-    prerender_list_.pop_front();
-    data.contents_->Destroy(FINAL_STATUS_EVICTED);
+  while (active_prerender_list_.size() > config_.max_elements) {
+    prerender_contents = active_prerender_list_.front().contents;
+    DCHECK(prerender_contents);
+    prerender_contents->Destroy(FINAL_STATUS_EVICTED);
   }
-  StartSchedulingPeriodicCleanups();
-  return true;
-}
 
-PrerenderContents* PrerenderManager::GetEntry(const GURL& url) {
-  return GetEntryButNotSpecifiedWC(url, NULL);
-}
-
-PrerenderContents* PrerenderManager::GetEntryButNotSpecifiedWC(
-    const GURL& url,
-    WebContents* wc) {
-  DCHECK(CalledOnValidThread());
-  DeleteOldEntries();
-  DeletePendingDeleteEntries();
-  for (PrerenderContentsDataList::iterator it = prerender_list_.begin();
-       it != prerender_list_.end();
-       ++it) {
-    PrerenderContents* prerender_contents = it->contents_;
-    if (prerender_contents->MatchesURL(url, NULL) &&
-        !IsNoSwapInExperiment(prerender_contents->experiment_id())) {
-      if (!prerender_contents->prerender_contents() ||
-          !wc ||
-          prerender_contents->prerender_contents()->web_contents() != wc) {
-        prerender_list_.erase(it);
-        return prerender_contents;
-      }
-    }
-  }
-  // Entry not found.
-  return NULL;
+  StartSchedulingPeriodicCleanups();
+  return prerender_handle;
 }
 
 void PrerenderManager::StartSchedulingPeriodicCleanups() {
@@ -941,10 +916,7 @@ void PrerenderManager::StartSchedulingPeriodicCleanups() {
       &PrerenderManager::PeriodicCleanup);
 }
 
-void PrerenderManager::MaybeStopSchedulingPeriodicCleanups() {
-  if (!prerender_list_.empty())
-    return;
-
+void PrerenderManager::StopSchedulingPeriodicCleanups() {
   DCHECK(CalledOnValidThread());
   repeating_timer_.Stop();
 }
@@ -953,22 +925,22 @@ void PrerenderManager::PeriodicCleanup() {
   DCHECK(CalledOnValidThread());
   DeleteOldTabContents();
   DeleteOldEntries();
+  if (active_prerender_list_.empty())
+    StopSchedulingPeriodicCleanups();
 
   // Grab a copy of the current PrerenderContents pointers, so that we
   // will not interfere with potential deletions of the list.
   std::vector<PrerenderContents*> prerender_contents;
-  for (PrerenderContentsDataList::iterator it = prerender_list_.begin();
-       it != prerender_list_.end();
-       ++it) {
-    DCHECK(it->contents_);
-    prerender_contents.push_back(it->contents_);
-  }
-  for (std::vector<PrerenderContents*>::iterator it =
-           prerender_contents.begin();
-       it != prerender_contents.end();
-       ++it) {
-    (*it)->DestroyWhenUsingTooManyResources();
+  prerender_contents.reserve(active_prerender_list_.size());
+  for (std::list<PrerenderHandle::PrerenderData>::iterator
+          it = active_prerender_list_.begin();
+      it != active_prerender_list_.end();
+      ++it) {
+    prerender_contents.push_back(it->contents);
   }
+  std::for_each(prerender_contents.begin(), prerender_contents.end(),
+                std::mem_fun(
+                    &PrerenderContents::DestroyWhenUsingTooManyResources));
 
   DeletePendingDeleteEntries();
 }
@@ -986,21 +958,20 @@ base::TimeDelta PrerenderManager::GetMaxAge() const {
       base::TimeDelta::FromSeconds(300) : config_.max_age);
 }
 
-bool PrerenderManager::IsPrerenderElementFresh(const base::Time start) const {
+bool PrerenderManager::IsPrerenderFresh(const base::TimeTicks start) const {
   DCHECK(CalledOnValidThread());
-  base::Time now = GetCurrentTime();
-  return (now - start < GetMaxAge());
+  return GetCurrentTimeTicks() - start < GetMaxAge();
 }
 
 void PrerenderManager::DeleteOldEntries() {
   DCHECK(CalledOnValidThread());
-  while (!prerender_list_.empty()) {
-    PrerenderContentsData data = prerender_list_.front();
-    if (IsPrerenderElementFresh(data.start_time_))
+  while (!active_prerender_list_.empty()) {
+    PrerenderContents* contents = active_prerender_list_.front().contents;
+    DCHECK(contents);
+    if (IsPrerenderFresh(contents->load_start_time()))
       return;
-    data.contents_->Destroy(FINAL_STATUS_TIMED_OUT);
+    contents->Destroy(FINAL_STATUS_TIMED_OUT);
   }
-  MaybeStopSchedulingPeriodicCleanups();
 }
 
 base::Time PrerenderManager::GetCurrentTime() const {
@@ -1030,58 +1001,78 @@ void PrerenderManager::DeletePendingDeleteEntries() {
   }
 }
 
-PrerenderManager::PrerenderContentsData* PrerenderManager::FindEntryData(
-    const GURL& url) {
-  DCHECK(CalledOnValidThread());
-  PrerenderContentsDataList::iterator it = FindPrerenderContentsForURL(url);
-  if (it == prerender_list_.end())
-    return NULL;
-  PrerenderContentsData& prerender_contents_data = *it;
-  return &prerender_contents_data;
+PrerenderHandle* PrerenderManager::CreatePrerenderHandleForPrerenderData(
+    PrerenderHandle::PrerenderData* prerender_data) {
+  PrerenderWeakPtrFactoryMap::value_type map_value(prerender_data, NULL);
+  std::pair<PrerenderWeakPtrFactoryMap::iterator, bool>
+      insert_result = prerender_weak_ptr_factory_map_.insert(map_value);
+  if (insert_result.second)
+    insert_result.first->second =
+        new base::WeakPtrFactory<PrerenderHandle::PrerenderData>(
+            prerender_data);
+  DCHECK(insert_result.first->second);
+  base::WeakPtr<PrerenderHandle::PrerenderData> weak_prerender_data =
+      insert_result.first->second->GetWeakPtr();
+  return new PrerenderHandle(weak_prerender_data);
+}
+
+void PrerenderManager::InvalidatePrerenderHandlesForPrerenderData(

[mmenke, 2012/07/10 18:01:13]

I think that this is pretty ugly.  My suggestion (And I'm completely open to
push back/discussion on this) would be to make PrerenderData have a pointer to
the factory.  I'd also switch PrerenderData to being an inner class of
PrerenderManager at the same time, since proper use of PrerenderData would then
be more dependent on PrerenderManager than PrerenderHandle.

My reasoning is basically just that the fewer redundant stl containers we have,
the simpler things are.

[gavinp, 2012/07/11 17:04:00]

Done. Please look it over and let me know what you think about the struct and
its podness.

+    const PrerenderHandle::PrerenderData& prerender_data) {
+  PrerenderWeakPtrFactoryMap::iterator it =
+      prerender_weak_ptr_factory_map_.find(&prerender_data);
+  if (it != prerender_weak_ptr_factory_map_.end()) {
+    delete it->second;
+    prerender_weak_ptr_factory_map_.erase(it);
+  }
 }
 
-PrerenderContents* PrerenderManager::FindEntry(const GURL& url) const {
-  DCHECK(CalledOnValidThread());
-  for (PrerenderContentsDataList::const_iterator it = prerender_list_.begin();
-       it != prerender_list_.end();
+PrerenderHandle::PrerenderData* PrerenderManager::FindPrerenderData(
+    const GURL& url,
+    const SessionStorageNamespace* session_storage_namespace) {
+  for (std::list<PrerenderHandle::PrerenderData>::iterator
+           it = active_prerender_list_.begin();
+       it != active_prerender_list_.end();
        ++it) {
-    if (it->contents_->MatchesURL(url, NULL))
-      return it->contents_;
+    PrerenderContents* prerender_contents = it->contents;
+    if (prerender_contents->Matches(url, session_storage_namespace))
+      return &(*it);
   }
-  // Entry not found.
   return NULL;
 }
 
-PrerenderManager::PrerenderContentsDataList::iterator
-    PrerenderManager::FindPrerenderContentsForChildRouteIdPair(
-        const std::pair<int, int>& child_route_id_pair) {
-  PrerenderContentsDataList::iterator it = prerender_list_.begin();
-  for (; it != prerender_list_.end(); ++it) {
-    PrerenderContents* prerender_contents = it->contents_;
+PrerenderHandle::PrerenderData*
+PrerenderManager::FindPrerenderDataForChildAndRoute(
+    const int child_id, const int route_id) {
+  for (std::list<PrerenderHandle::PrerenderData>::iterator
+           it = active_prerender_list_.begin();
+       it != active_prerender_list_.end();
+       ++it) {
+    PrerenderContents* prerender_contents = it->contents;
 
-    int child_id;
-    int route_id;
-    bool has_child_id = prerender_contents->GetChildId(&child_id);
-    bool has_route_id = has_child_id &&
-                        prerender_contents->GetRouteId(&route_id);
+    int contents_child_id;
+    if (!prerender_contents->GetChildId(&contents_child_id))
+      continue;
+    int contents_route_id;
+    if (!prerender_contents->GetRouteId(&contents_route_id))
+      continue;
 
-    if (has_child_id && has_route_id &&
-        child_id == child_route_id_pair.first &&
-        route_id == child_route_id_pair.second) {
-      break;
-    }
+    if (contents_child_id == child_id && contents_route_id == route_id)
+      return &(*it);
   }
-  return it;
+  return NULL;
 }
 
-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))
+std::list<PrerenderHandle::PrerenderData>::iterator
+PrerenderManager::FindIteratorForPrerenderContents(
+    PrerenderContents* prerender_contents) {
+  for (std::list<PrerenderHandle::PrerenderData>::iterator
+           it = active_prerender_list_.begin();
+       it != active_prerender_list_.end();
+       ++it) {
+    if (prerender_contents == it->contents)
       return it;
   }
-  return prerender_list_.end();
+  return active_prerender_list_.end();
 }
 
 bool PrerenderManager::DoesRateLimitAllowPrerender() const {
@@ -1143,23 +1134,21 @@ void PrerenderManager::AddToHistory(PrerenderContents* contents) {
 
 Value* PrerenderManager::GetActivePrerendersAsValue() const {
   ListValue* list_value = new ListValue();
-  for (PrerenderContentsDataList::const_iterator it = prerender_list_.begin();
-       it != prerender_list_.end();
+  for (std::list<PrerenderHandle::PrerenderData>::const_iterator it =
+           active_prerender_list_.begin();
+       it != active_prerender_list_.end();
        ++it) {
-    Value* prerender_value = it->contents_->GetAsValue();
-    if (!prerender_value)
-      continue;
-    list_value->Append(prerender_value);
+    if (Value* prerender_value = it->contents->GetAsValue())
+      list_value->Append(prerender_value);
   }
   return list_value;
 }
 
 void PrerenderManager::DestroyAllContents(FinalStatus final_status) {
   DeleteOldTabContents();
-  while (!prerender_list_.empty()) {
-    PrerenderContentsData data = prerender_list_.front();
-    prerender_list_.pop_front();
-    data.contents_->Destroy(final_status);
+  while (!active_prerender_list_.empty()) {
+    PrerenderContents* contents = active_prerender_list_.front().contents;
+    contents->Destroy(final_status);
   }
   DeletePendingDeleteEntries();
 }
@@ -1218,3 +1207,4 @@ PrerenderManager* FindPrerenderManagerUsingRenderProcessId(
 }
 
 }  // namespace prerender
+