Move pending prerender logic into PrerenderLinkManager.

chrome/browser/prerender/prerender_link_manager.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/prerender/prerender_link_manager.h"
 
 #include <limits>
 #include <set>
 #include <utility>
 
@@ skipping 25 matching lines @@
   RenderProcessHost* render_process_host = RenderProcessHost::FromID(child_id);
   if (!render_process_host)
     return;
   render_process_host->Send(own_message.release());
 }
 
 }  // namespace
 
 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() {
   for (std::list<LinkPrerender>::iterator i = prerenders_.begin();
        i != prerenders_.end(); ++i) {
     if (i->handle) {
       DCHECK(!i->handle->IsPrerendering())
           << "All running prerenders should stop at the same time as the "
           << "PrerenderManager.";
       delete i->handle;
@@ skipping 11 matching lines @@
   DCHECK_EQ(static_cast<LinkPrerender*>(NULL),
             FindByLauncherChildIdAndPrerenderId(launcher_child_id,
                                                 prerender_id));
   content::RenderProcessHost* rph =
       content::RenderProcessHost::FromID(launcher_child_id);
   // Guests inside <webview> do not support cross-process navigation and so we
   // do not allow guests to prerender content.
   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) {
   LinkPrerender* prerender = FindByLauncherChildIdAndPrerenderId(child_id,
                                                                  prerender_id);
   if (!prerender)
     return;
 
   CancelPrerender(prerender);
   StartPrerenders();
@@ skipping 36 matching lines @@
   }
 }
 
 PrerenderLinkManager::LinkPrerender::LinkPrerender(
     int launcher_child_id,
     int prerender_id,
     const GURL& url,
     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() {
   DCHECK_EQ(static_cast<PrerenderHandle*>(NULL), handle)
       << "The PrerenderHandle should be destroyed before its Prerender.";
 }
 
 bool PrerenderLinkManager::IsEmpty() const {
   return prerenders_.empty();
 }
@@ skipping 16 matching lines @@
   std::list<LinkPrerender*> abandoned_prerenders;
   std::list<std::list<LinkPrerender>::iterator> pending_prerenders;
   std::multiset<std::pair<int, int> >
       running_launcher_and_render_view_routes;
 
   // Scan the list, counting how many prerenders have handles (and so were added
   // to the PrerenderManager). The count is done for the system as a whole, and
   // 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 {
       ++total_started_prerender_count;
       if (i->has_been_abandoned) {
         abandoned_prerenders.push_back(&(*i));
       } else {
         // We do not count abandoned prerenders towards their launcher, since it
         // has already navigated on to another page.
         std::pair<int, int> launcher_and_render_view_route(
@@ skipping 116 matching lines @@
       i->handle = NULL;
       prerenders_.erase(i);
       if (own_handle)
         own_handle->OnCancel();
       return;
     }
   }
   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;
 }
 
 // In practice, this is always called from either
 // PrerenderLinkManager::OnAddPrerender in the regular case, or in the pending
 // prerender case, from PrerenderHandle::AdoptPrerenderDataFrom.
 void PrerenderLinkManager::OnPrerenderStart(
     PrerenderHandle* prerender_handle) {
   LinkPrerender* prerender = FindByPrerenderHandle(prerender_handle);
@@ skipping 38 matching lines @@
   DCHECK(!prerender->is_match_complete_replacement);
   prerender->is_match_complete_replacement = true;
   Send(prerender->launcher_child_id,
        new PrerenderMsg_OnPrerenderStop(prerender->prerender_id));
   // Do not call RemovePrerender here. The replacement needs to stay connected
   // to the HTMLLinkElement in the renderer so it notices renderer-triggered
   // cancelations.
 }
 
 }  // namespace prerender