Move pending prerender logic into PrerenderLinkManager.

chrome/browser/prerender/prerender_link_manager.cc

Index: chrome/browser/prerender/prerender_link_manager.cc
diff --git a/chrome/browser/prerender/prerender_link_manager.cc b/chrome/browser/prerender/prerender_link_manager.cc
index 7726016c203f430ee05bb727dc56b28e7e8a7a64..296400b192cb5a600b6293949df2aff9fce7706c 100644
--- a/chrome/browser/prerender/prerender_link_manager.cc
+++ b/chrome/browser/prerender/prerender_link_manager.cc
@@ -43,9 +43,55 @@ void Send(int child_id, IPC::Message* raw_message) {
 
 namespace prerender {
 
+// Helper class to implement PrerenderContents::Observer and watch prerenders
+// which launch other prerenders.
+class PrerenderLinkManager::PendingPrerenderManager
+    : public PrerenderContents::Observer {
+ public:
+  explicit PendingPrerenderManager(PrerenderLinkManager* link_manager)
+      : link_manager_(link_manager) {
+  }

[gavinp, 2014/01/30 19:58:49]

This can move up a line too.

[davidben, 2014/01/30 21:01:51]

Done.

+
+  virtual ~PendingPrerenderManager() {
+    DCHECK(observed_launchers_.empty());
+    for (std::set<PrerenderContents*>::iterator i = observed_launchers_.begin();
+         i != observed_launchers_.end(); ++i) {
+      (*i)->RemoveObserver(this);
+    }
+  }
+
+  void ObserveLauncher(PrerenderContents* launcher) {
+    DCHECK_EQ(FINAL_STATUS_MAX, launcher->final_status());
+    if (observed_launchers_.find(launcher) != observed_launchers_.end())
+      return;
+    observed_launchers_.insert(launcher);
+    launcher->AddObserver(this);
+  }
+
+  virtual void OnPrerenderStart(PrerenderContents* launcher) OVERRIDE {
+  }

[gavinp, 2014/01/30 19:58:49]

Nit: Move this to the previous line.

Also, add a blank line between methods.

[davidben, 2014/01/30 21:01:51]

Done.

+  virtual void OnPrerenderStop(PrerenderContents* launcher) OVERRIDE {
+    observed_launchers_.erase(launcher);
+    if (launcher->final_status() == FINAL_STATUS_USED) {
+      link_manager_->StartPendingPrerendersForLauncher(launcher);
+    } else {
+      link_manager_->CancelPendingPrerendersForLauncher(launcher);
+    }
+  }
+
+ private:
+  // A pointer to the parent PrerenderLinkManager.
+  PrerenderLinkManager* link_manager_;
+
+  // The set of PrerenderContentses being observed. Lifetimes are managed by
+  // OnPrerenderStop.
+  std::set<PrerenderContents*> observed_launchers_;
+};
+
 PrerenderLinkManager::PrerenderLinkManager(PrerenderManager* manager)
     : has_shutdown_(false),
-      manager_(manager) {
+      manager_(manager),
+      pending_prerender_manager_(new PendingPrerenderManager(this)) {

[gavinp, 2014/01/30 19:58:49]

Ditto.

[davidben, 2014/01/30 21:01:51]

Done.

 }
 
 PrerenderLinkManager::~PrerenderLinkManager() {
@@ -77,11 +123,28 @@ void PrerenderLinkManager::OnAddPrerender(int launcher_child_id,
   if (rph && rph->IsGuest())
     return;
 
+  // Check if the launcher is itself an unswapped prerender.
+  PrerenderContents* prerender_contents =
+      manager_->GetPrerenderContentsForRoute(launcher_child_id,
+                                             render_view_route_id);
+  if (prerender_contents &&
+      prerender_contents->final_status() != FINAL_STATUS_MAX) {
+    // The launcher is a prerender about to be destroyed asynchronously, but
+    // its AddLinkRelPrerender message raced with shutdown. Ignore it.
+    DCHECK_NE(FINAL_STATUS_USED, prerender_contents->final_status());
+    return;
+  }
+
   LinkPrerender
       prerender(launcher_child_id, prerender_id, url, referrer, size,
-                render_view_route_id, manager_->GetCurrentTimeTicks());
+                render_view_route_id, manager_->GetCurrentTimeTicks(),
+                prerender_contents);
   prerenders_.push_back(prerender);
-  StartPrerenders();
+  if (prerender_contents) {

[gavinp, 2014/01/30 19:58:49]

Nit: the {} on this are not necessary. I wouldn't use them.

[davidben, 2014/01/30 21:01:51]

Done.

+    pending_prerender_manager_->ObserveLauncher(prerender_contents);
+  } else {
+    StartPrerenders();
+  }
 }
 
 void PrerenderLinkManager::OnCancelPrerender(int child_id, int prerender_id) {
@@ -138,16 +201,19 @@ PrerenderLinkManager::LinkPrerender::LinkPrerender(
     const content::Referrer& referrer,
     const gfx::Size& size,
     int render_view_route_id,
-    TimeTicks creation_time) : launcher_child_id(launcher_child_id),
-                               prerender_id(prerender_id),
-                               url(url),
-                               referrer(referrer),
-                               size(size),
-                               render_view_route_id(render_view_route_id),
-                               creation_time(creation_time),
-                               handle(NULL),
-                               is_match_complete_replacement(false),
-                               has_been_abandoned(false) {
+    TimeTicks creation_time,
+    PrerenderContents* deferred_launcher)
+    : launcher_child_id(launcher_child_id),
+      prerender_id(prerender_id),
+      url(url),
+      referrer(referrer),
+      size(size),
+      render_view_route_id(render_view_route_id),
+      creation_time(creation_time),
+      deferred_launcher(deferred_launcher),
+      handle(NULL),
+      is_match_complete_replacement(false),
+      has_been_abandoned(false) {
 }
 
 PrerenderLinkManager::LinkPrerender::~LinkPrerender() {
@@ -184,6 +250,9 @@ void PrerenderLinkManager::StartPrerenders() {
   // also per launcher.
   for (std::list<LinkPrerender>::iterator i = prerenders_.begin();
        i != prerenders_.end(); ++i) {
+    // Skip prerenders launched by a prerender.
+    if (i->deferred_launcher)
+      continue;
     if (!i->handle) {
       pending_prerenders.push_back(i);
     } else {
@@ -320,6 +389,30 @@ void PrerenderLinkManager::CancelPrerender(LinkPrerender* prerender) {
   NOTREACHED();
 }
 
+void PrerenderLinkManager::StartPendingPrerendersForLauncher(
+    PrerenderContents* launcher) {
+  for (std::list<LinkPrerender>::iterator i = prerenders_.begin();
+       i != prerenders_.end(); ++i) {
+    if (i->deferred_launcher == launcher)
+      i->deferred_launcher = NULL;
+  }
+  StartPrerenders();
+}
+
+void PrerenderLinkManager::CancelPendingPrerendersForLauncher(
+    PrerenderContents* launcher) {
+  // Remove all pending prerenders for this launcher.
+  std::list<LinkPrerender>::iterator iter = prerenders_.begin();
+  while (iter != prerenders_.end()) {
+    // Increment iterator first so it isn't invalidated.
+    std::list<LinkPrerender>::iterator current = iter++;

[gavinp, 2014/01/30 19:58:49]

This looks correct.

However, just for absolute safety when iterating and mutating a data structure,
I'm an advocate of just constructing a vector of iterators to_erase, and then
calling:

std::for_each(to_erase.begin(), to_erase.end(),
                     std::bind1st(std::mem_fun(&std::list<LinkPrerender>::erase,
prerenders_));

at the end. It's safer and a bit less scary.

[davidben, 2014/01/30 21:01:51]

Done.

+    if (current->deferred_launcher == launcher) {
+      DCHECK(!current->handle);
+      prerenders_.erase(current);
+    }
+  }
+}
+
 void PrerenderLinkManager::Shutdown() {
   has_shutdown_ = true;
 }