Handle interface to prerenders.

chrome/browser/prerender/prerender_manager.h

Index: chrome/browser/prerender/prerender_manager.h
diff --git a/chrome/browser/prerender/prerender_manager.h b/chrome/browser/prerender/prerender_manager.h
index 3fa9591e9629923ea8f0ab379447502f6629ee98..ff65a1a7437d1946d41528e986bd5616fb180f77 100644
--- a/chrome/browser/prerender/prerender_manager.h
+++ b/chrome/browser/prerender/prerender_manager.h
@@ -21,6 +21,7 @@
 #include "chrome/browser/prerender/prerender_config.h"
 #include "chrome/browser/prerender/prerender_contents.h"
 #include "chrome/browser/prerender/prerender_final_status.h"
+#include "chrome/browser/prerender/prerender_handle.h"
 #include "chrome/browser/prerender/prerender_origin.h"
 #include "chrome/browser/profiles/profile_keyed_service.h"
 #include "googleurl/src/gurl.h"
@@ -107,7 +108,7 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>,
   // Returns true if the URL was added, false if it was not.
   // If the launching RenderView is itself prerendering, the prerender is added
   // as a pending prerender.
-  bool AddPrerenderFromLinkRelPrerender(
+  PrerenderHandle AddPrerenderFromLinkRelPrerender(
       int process_id,
       int route_id,
       const GURL& url,
@@ -119,15 +120,10 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>,
   // child or route id, or a referrer. This method uses sensible values for
   // those. The |session_storage_namespace| matches the namespace of the active
   // tab at the time the prerender is generated from the omnibox.
-  bool AddPrerenderFromOmnibox(
+  PrerenderHandle AddPrerenderFromOmnibox(
       const GURL& url,
       content::SessionStorageNamespace* session_storage_namespace);
 
-  // Request cancelation of a previously added prerender. If the |active_count_|
-  // of the prerender is one, it will be canceled.  Otherwise, |active_count_|
-  // will be decremented by one.
-  void MaybeCancelPrerender(const GURL& url);

[dominich, 2012/06/18 15:32:44]

explicit cancellation is the bomb. Why would you remove it? If nothing in this
class takes a PrerenderHandle as a parameter, then why do you return one from
the public interface?

-
   // Destroy all prerenders for the given child route id pair and assign a final
   // status to them.
   virtual void DestroyPrerenderForRenderView(int process_id,
@@ -137,9 +133,6 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>,
   // Cancels all active prerenders.
   void CancelAllPrerenders();
 
-  // Cancels all active prerenders with the ORIGIN_OMNIBOX origin.
-  void CancelOmniboxPrerenders();
-
   // If |url| matches a valid prerendered page, try to swap it into
   // |web_contents| and merge browsing histories. Returns |true| if a
   // prerendered page is swapped in, |false| otherwise.
@@ -148,7 +141,7 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>,
 
   // Moves a PrerenderContents to the pending delete list from the list of
   // active prerenders when prerendering should be cancelled.
-  void MoveEntryToPendingDelete(PrerenderContents* entry,
+  void MoveEntryToPendingDelete(WeakPrerenderHandle prerender,
                                 FinalStatus final_status);
 
   // Records the perceived page load time for a page - effectively the time from
@@ -186,10 +179,6 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>,
   // is prerendering a page.
   bool IsWebContentsPrerendering(content::WebContents* web_contents) const;
 
-  // Returns true if there is a prerendered page for the given URL and it has
-  // finished loading. Only valid if called before MaybeUsePrerenderedPage.
-  bool DidPrerenderFinishLoading(const GURL& url) const;
-
   // Maintaining and querying the set of WebContents belonging to this
   // PrerenderManager that are currently showing prerendered pages.
   void MarkWebContentsAsPrerendered(content::WebContents* web_contents);
@@ -261,9 +250,17 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>,
   // tests.
   void DoShutdown();
 
+  const PrerenderContents* FindPrerender(const GURL& url) const;

[dominich, 2012/06/18 15:32:44]

Should this still be needed? I thought the point was to get rid of interfaces
like this. If it is, it should take a SessionStorageNamespace at least.

+
+  PrerenderContents* ClaimPrerender(
+      content::WebContents* web_contents,
+      const GURL& url,
+      content::SessionStorageNamespace* session_storage_namespace);
+
  private:
   // Test that needs needs access to internal functions.
   friend class PrerenderBrowserTest;
+  friend class PrerenderTest;
   FRIEND_TEST_ALL_PREFIXES(PrerenderTest, AliasURLTest);
   FRIEND_TEST_ALL_PREFIXES(PrerenderTest, CancelAllTest);
   FRIEND_TEST_ALL_PREFIXES(PrerenderTest,
@@ -299,12 +296,12 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>,
   FRIEND_TEST_ALL_PREFIXES(PrerenderTest, SourceRenderViewClosed);
   FRIEND_TEST_ALL_PREFIXES(PrerenderTest, TwoElementPrerenderTest);
 
-  struct PrerenderContentsData;
-  struct NavigationRecord;
-
   class OnCloseTabContentsDeleter;
+  class PrerenderContentsData;
+  struct NavigationRecord;
 
-  typedef std::list<PrerenderContentsData> PrerenderContentsDataList;
+  typedef std::list<base::WeakPtr<PrerenderContentsData> >

[dominich, 2012/06/18 15:32:44]

Shouldn't this now be a list of PrerenderHandles from which the
PrerenderContentsData can be determined?

[gavinp, 2012/06/18 16:40:48]

I was really hoping to avoid a lot of "IsCanceled" logic. So that's why the
WeakPtrs.

But now I'm thinking an Observer interface with OnDestroy is the way to go;
Observers can do things like take themselves out of this list.

So yes, explicit cancellation, and sure, PrerenderHandles here, but they will
remove themselves from the list when they get OnDestroy

+      PrerenderContentsDataList;
   typedef base::hash_map<content::WebContents*, bool> WouldBePrerenderedMap;
 
   // Time window for which we record old navigations, in milliseconds.
@@ -316,7 +313,7 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>,
   // PrerenderContents::StartPrerendering will instead use the size of the
   // currently active tab. If the current active tab size cannot be found, it
   // then uses a default from PrerenderConfig.
-  bool AddPrerender(
+  PrerenderHandle AddPrerender(
       Origin origin,
       int child_id,
       const GURL& url,
@@ -324,19 +321,6 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>,
       const gfx::Size& size,
       content::SessionStorageNamespace* session_storage_namespace);
 
-  // Retrieves the PrerenderContents object for the specified URL, if it
-  // has been prerendered.  The caller will then have ownership of the
-  // PrerenderContents object and is responsible for freeing it.
-  // Returns NULL if the specified URL has not been prerendered.
-  PrerenderContents* GetEntry(const GURL& url);
-
-  // Identical to GetEntry, with one exception:
-  // The WebContents specified indicates the WC in which to swap the
-  // prerendering into.  If the WebContents specified is the one
-  // to doing the prerendered itself, will return NULL.
-  PrerenderContents* GetEntryButNotSpecifiedWC(const GURL& url,
-                                               content::WebContents* wc);
-
   // Starts scheduling periodic cleanups.
   void StartSchedulingPeriodicCleanups();
   // Stops scheduling periodic cleanups if they're no longer needed.
@@ -354,7 +338,6 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>,
   void PostCleanupTask();
 
   base::TimeDelta GetMaxAge() const;
-  bool IsPrerenderElementFresh(const base::Time start) const;
   void DeleteOldEntries();
   virtual base::Time GetCurrentTime() const;
   virtual base::TimeTicks GetCurrentTimeTicks() const;
@@ -368,20 +351,11 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>,
   // list.
   void DeletePendingDeleteEntries();
 
-  // Finds the specified PrerenderContentsData/PrerenderContents and returns it,
-  // if it exists. Returns NULL otherwise.  Unlike GetEntry, the
-  // PrerenderManager maintains ownership of the PrerenderContents.
-  PrerenderContentsData* FindEntryData(const GURL& url);
-  PrerenderContents* FindEntry(const GURL& url) const;
-
-  // Returns the iterator to the PrerenderContentsData entry that is being
-  // prerendered from the given child route id pair.
-  PrerenderContentsDataList::iterator
-      FindPrerenderContentsForChildRouteIdPair(
-          const std::pair<int, int>& child_route_id_pair);
+  const PrerenderContentsData* FindContentsData(PrerenderHandle handle) const;
+  PrerenderContentsData* FindContentsDataMutable(PrerenderHandle handle);
 
-  PrerenderContentsDataList::iterator
-      FindPrerenderContentsForURL(const GURL& url);
+  PrerenderContentsData*
+      FindContentsDataForChildAndRouteId(int child_id, int route_id);
 
   bool DoesRateLimitAllowPrerender() const;