Handle interface to prerenders.

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 c12e6cce61b5123ad9fa5e93e3cbfcb75c6b9b3b..53bc98ebee8b80bba13bb303f3b2e26da6af0530 100644
--- a/chrome/browser/prerender/prerender_link_manager.cc
+++ b/chrome/browser/prerender/prerender_link_manager.cc
@@ -9,6 +9,7 @@
 #include <utility>
 
 #include "chrome/browser/prerender/prerender_contents.h"
+#include "chrome/browser/prerender/prerender_handle.h"
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/prerender/prerender_manager_factory.h"
 #include "chrome/browser/profiles/profile.h"
@@ -29,6 +30,13 @@ PrerenderLinkManager::PrerenderLinkManager(PrerenderManager* manager)
 }
 
 PrerenderLinkManager::~PrerenderLinkManager() {
+  for (IdPairToPrerenderHandleMap::iterator it = ids_to_handle_map_.begin();
+       it != ids_to_handle_map_.end();
+       ++it) {
+    PrerenderHandle* prerender_handle = it->second;
+    prerender_handle->OnNavigateAway();

[dominich, 2012/07/02 20:43:03]

Why OnNavigateAway? If you delete (as you do below) it should call OnCancel
which is more correct, i think.

[gavinp, 2012/07/03 16:41:02]

I removed the delete.

[dominich, 2012/07/03 17:08:39]

But that's the right thing to do :) We're not navigating away, we're shutting
down. 

[gavinp, 2012/07/03 18:45:40]

Done.

+    delete prerender_handle;

[dominich, 2012/07/02 20:43:03]

So the caller of AddPrerender* owns the returned handle?

[gavinp, 2012/07/03 16:41:02]

Yes.

+  }
 }
 
 bool PrerenderLinkManager::OnAddPrerender(int child_id,
@@ -50,14 +58,16 @@ bool PrerenderLinkManager::OnAddPrerender(int child_id,
   replacements.ClearRef();
   const GURL url = orig_url.ReplaceComponents(replacements);
 
-  if (!manager_->AddPrerenderFromLinkRelPrerender(
-          child_id, render_view_route_id, url, referrer, size)) {
-    return false;
+  if (PrerenderHandle* prerender_handle =
+          manager_->AddPrerenderFromLinkRelPrerender(
+              child_id, render_view_route_id, url, referrer, size)) {
+    ChildAndPrerenderIdPair child_and_prerender_id(child_id, prerender_id);
+    DCHECK_EQ(0U, ids_to_handle_map_.count(child_and_prerender_id));
+    ids_to_handle_map_.insert(
+        std::make_pair(child_and_prerender_id, prerender_handle));
+    return true;
   }
-  const ChildAndPrerenderIdPair child_and_prerender_id(child_id, prerender_id);
-  DCHECK_EQ(0U, ids_to_url_map_.count(child_and_prerender_id));
-  ids_to_url_map_.insert(std::make_pair(child_and_prerender_id, url));
-  return true;
+  return false;
 }
 
 // TODO(gavinp): Once an observer interface is provided down to the WebKit
@@ -69,24 +79,30 @@ void PrerenderLinkManager::OnCancelPrerender(int child_id, int prerender_id) {
   DVLOG(2) << "OnCancelPrerender, child_id = " << child_id
            << ", prerender_id = " << prerender_id;
   const ChildAndPrerenderIdPair child_and_prerender_id(child_id, prerender_id);
-  IdPairToUrlMap::iterator id_url_iter =
-      ids_to_url_map_.find(child_and_prerender_id);
-  if (id_url_iter == ids_to_url_map_.end()) {
+  IdPairToPrerenderHandleMap::iterator id_handle_iter =
+      ids_to_handle_map_.find(child_and_prerender_id);
+  if (id_handle_iter == ids_to_handle_map_.end()) {
     DVLOG(5) << "... canceling a prerender that doesn't exist.";
     return;
   }
-  const GURL url = id_url_iter->second;
-  ids_to_url_map_.erase(id_url_iter);
-  manager_->MaybeCancelPrerender(url);
+  PrerenderHandle* prerender_handle = id_handle_iter->second;
+  DCHECK(prerender_handle);
+  delete prerender_handle;
+  ids_to_handle_map_.erase(id_handle_iter);
 }
 
 void PrerenderLinkManager::OnAbandonPrerender(int child_id, int prerender_id) {
   DVLOG(2) << "OnAbandonPrerender, child_id = " << child_id
            << ", prerender_id = " << prerender_id;
-  // TODO(gavinp,cbentzel): Implement reasonable behaviour for
-  // navigation away from launcher.
   const ChildAndPrerenderIdPair child_and_prerender_id(child_id, prerender_id);
-  ids_to_url_map_.erase(child_and_prerender_id);
+  IdPairToPrerenderHandleMap::iterator id_handle_iter =
+      ids_to_handle_map_.find(child_and_prerender_id);
+  if (id_handle_iter == ids_to_handle_map_.end())
+    return;
+  PrerenderHandle* prerender_handle = id_handle_iter->second;
+  prerender_handle->OnNavigateAway();

[dominich, 2012/07/02 20:43:03]

This will call both OnNavigateAway and OnCancel. It's not clear that this is a
good idea from the API. Once we hook this up to the link element getting events,
I'm not sure it will be clear what the expected behaviour is.  

[gavinp, 2012/07/03 16:41:02]

Both OnNavigateAway and OnCancel invalidate the handle. Now documented.

[dominich, 2012/07/03 17:08:39]

I'm tempted to suggest that OnCancel should delete(this), but there be dragons.

[gavinp, 2012/07/03 18:45:40]

I was tempted to write it that way too, but came to the same conclusion.

+  delete prerender_handle;
+  ids_to_handle_map_.erase(id_handle_iter);
 }
 
 void PrerenderLinkManager::OnChannelClosing(int child_id) {
@@ -96,9 +112,9 @@ void PrerenderLinkManager::OnChannelClosing(int child_id) {
   const ChildAndPrerenderIdPair child_and_maximum_prerender_id(
       child_id, std::numeric_limits<int>::max());
   std::queue<int> prerender_ids_to_abandon;
-  for (IdPairToUrlMap::iterator
-           i = ids_to_url_map_.lower_bound(child_and_minimum_prerender_id),
-           e = ids_to_url_map_.upper_bound(child_and_maximum_prerender_id);
+  for (IdPairToPrerenderHandleMap::iterator
+           i = ids_to_handle_map_.lower_bound(child_and_minimum_prerender_id),
+           e = ids_to_handle_map_.upper_bound(child_and_maximum_prerender_id);
        i != e; ++i) {
     prerender_ids_to_abandon.push(i->first.second);
   }
@@ -111,7 +127,7 @@ void PrerenderLinkManager::OnChannelClosing(int child_id) {
 }
 
 bool PrerenderLinkManager::IsEmpty() const {
-  return ids_to_url_map_.empty();
+  return ids_to_handle_map_.empty();
 }
 
 }  // namespace prerender