Cleanup PrerenderTracker in response to comments in

http://codereview.chromium.org/7038012/.

BUG=83062
TEST=PrerenderStatusManagerTests, PrerenderBrowserTests

Review URL: http://codereview.chromium.org/7060012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@86342 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 59 insertions, 42 deletions

diff --git a/chrome/browser/prerender/prerender_tracker.cc b/chrome/browser/prerender/prerender_tracker.cc
index c82acd3..f191e8c 100644
--- a/chrome/browser/prerender/prerender_tracker.cc
+++ b/chrome/browser/prerender/prerender_tracker.cc
@@ -46,9 +46,7 @@ PrerenderTracker* PrerenderTracker::GetInstance() {
 bool PrerenderTracker::TryUse(int child_id, int route_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  FinalStatus final_status = SetFinalStatus(child_id, route_id,
-                                            FINAL_STATUS_USED);
-  return final_status == FINAL_STATUS_USED;
+  return SetFinalStatus(child_id, route_id, FINAL_STATUS_USED, NULL);
 }
 
 bool PrerenderTracker::TryCancel(
@@ -58,8 +56,10 @@ bool PrerenderTracker::TryCancel(
   DCHECK_NE(FINAL_STATUS_USED, final_status);
   DCHECK(final_status >= 0 && final_status < FINAL_STATUS_MAX);
 
-  final_status = SetFinalStatus(child_id, route_id, final_status);
-  return final_status != FINAL_STATUS_USED && final_status != FINAL_STATUS_MAX;
+  FinalStatus actual_final_status;
+  SetFinalStatus(child_id, route_id, final_status, &actual_final_status);
+  return actual_final_status != FINAL_STATUS_USED &&
+         actual_final_status != FINAL_STATUS_MAX;
 }
 
 bool PrerenderTracker::TryCancelOnIOThread(
@@ -118,6 +118,12 @@ void PrerenderTracker::OnPrerenderingStarted(
       std::make_pair(child_route_id_pair, RenderViewInfo(prerender_manager)));
 }
 
+PrerenderTracker::PrerenderTracker() {
+}
+
+PrerenderTracker::~PrerenderTracker() {
+}
+
 void PrerenderTracker::OnPrerenderingFinished(int child_id, int route_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK_GE(child_id, 0);
@@ -134,15 +140,11 @@ void PrerenderTracker::OnPrerenderingFinished(int child_id, int route_id) {
   DCHECK_EQ(1u, num_erased);
 }
 
-PrerenderTracker::PrerenderTracker() {
-}
-
-PrerenderTracker::~PrerenderTracker() {
-}
-
-FinalStatus PrerenderTracker::SetFinalStatus(int child_id, int route_id,
-                                             FinalStatus final_status) {
-  DCHECK(final_status >= FINAL_STATUS_USED && final_status < FINAL_STATUS_MAX);
+bool PrerenderTracker::SetFinalStatus(int child_id, int route_id,
+                                      FinalStatus desired_final_status,
+                                      FinalStatus* actual_final_status) {
+  DCHECK(desired_final_status >= FINAL_STATUS_USED &&
+         desired_final_status < FINAL_STATUS_MAX);
 
   ChildRouteIdPair child_route_id_pair(child_id, route_id);
 
@@ -151,22 +153,31 @@ FinalStatus PrerenderTracker::SetFinalStatus(int child_id, int route_id,
       final_status_map_.find(child_route_id_pair);
   if (final_status_it == final_status_map_.end()) {
     // The RenderView has already been either used or destroyed.
-    return FINAL_STATUS_MAX;
+    if (actual_final_status)
+      *actual_final_status = FINAL_STATUS_MAX;
+    return false;
   }
 
   if (final_status_it->second.final_status == FINAL_STATUS_MAX) {
-    final_status_it->second.final_status = final_status;
-    if (final_status != FINAL_STATUS_USED) {
+    final_status_it->second.final_status = desired_final_status;
+    if (desired_final_status != FINAL_STATUS_USED) {
       BrowserThread::PostTask(
           BrowserThread::UI, FROM_HERE,
           NewRunnableFunction(&DestroyPreloadForRenderView,
                               final_status_it->second.prerender_manager,
                               child_id,
                               route_id,
-                              final_status));
+                              desired_final_status));
     }
+
+    if (actual_final_status)
+      *actual_final_status = desired_final_status;
+    return true;
   }
-  return final_status_it->second.final_status;
+
+  if (actual_final_status)
+    *actual_final_status = final_status_it->second.final_status;
+  return false;
 }
 
 void PrerenderTracker::AddPrerenderOnIOThread(
diff --git a/chrome/browser/prerender/prerender_tracker.h b/chrome/browser/prerender/prerender_tracker.h
index f263052..acd8e8d 100644
--- a/chrome/browser/prerender/prerender_tracker.h
+++ b/chrome/browser/prerender/prerender_tracker.h
@@ -28,13 +28,13 @@ class PrerenderTracker {
 
   // Attempts to set the status of the specified RenderViewHost to
   // FINAL_STATUS_USED.  Returns true on success.  Returns false if it has
-  // already been cancelled for any reason, or is no longer prerendering.
+  // already been cancelled for any reason or is no longer prerendering.
   // Can only be called only on the IO thread.  This method will not call
   // PrerenderContents::set_final_status() on the corresponding
   // PrerenderContents.
   //
-  // If it returns true, all subsequent calls to TryCancel for the RenderView
-  // will return false.
+  // If it returns true, all subsequent calls to TryCancel and TryUse for the
+  // RenderView will return false.
   bool TryUse(int child_id, int route_id);
 
   // Attempts to cancel prerendering by the specified RenderView, setting the
@@ -65,7 +65,8 @@ class PrerenderTracker {
   bool GetFinalStatus(int child_id, int route_id,
                       FinalStatus* final_status) const;
 
- protected:
+ private:
+  friend struct DefaultSingletonTraits<PrerenderTracker>;
   friend class PrerenderContents;
 
   FRIEND_TEST_ALL_PREFIXES(PrerenderTrackerTest, PrerenderTrackerUsed);
@@ -74,19 +75,6 @@ class PrerenderTracker {
   FRIEND_TEST_ALL_PREFIXES(PrerenderTrackerTest, PrerenderTrackerCancelledFast);
   FRIEND_TEST_ALL_PREFIXES(PrerenderTrackerTest, PrerenderTrackerMultiple);
 
-  // Must be called when a RenderView starts prerendering, before the first
-  // navigation starts to avoid any races.
-  void OnPrerenderingStarted(int child_id, int route_id,
-                             PrerenderManager* prerender_manager);
-
-  // Must be called when a RenderView stops prerendering, either because the
-  // RenderView was used or prerendering was cancelled and it is being
-  // destroyed.
-  void OnPrerenderingFinished(int child_id, int route_id);
-
- private:
-  friend struct DefaultSingletonTraits<PrerenderTracker>;
-
   typedef std::pair<int, int> ChildRouteIdPair;
 
   // Map of child/route id pairs to final statuses.
@@ -97,13 +85,31 @@ class PrerenderTracker {
   PrerenderTracker();
   ~PrerenderTracker();
 
+  // Must be called when a RenderView starts prerendering, before the first
+  // navigation starts to avoid any races.
+  void OnPrerenderingStarted(int child_id, int route_id,
+                             PrerenderManager* prerender_manager);
+
+  // Must be called when a RenderView stops prerendering, either because the
+  // RenderView was used or prerendering was cancelled and it is being
+  // destroyed.
+  void OnPrerenderingFinished(int child_id, int route_id);
+
   // Attempts to set the FinalStatus of the specified RenderView to
-  // |final_status|.  If the FinalStatus of the RenderView has already been
-  // set, does nothing.  Returns the resulting FinalStatus of that RenderView,
-  // regardless of success or failure.  If the RenderView isn't currently
-  // prerendering, returns FINAL_STATUS_MAX.
-  FinalStatus SetFinalStatus(int child_id, int route_id,
-                             FinalStatus final_status);
+  // |desired_final_status|.  If non-NULL, |actual_final_status| is set to the
+  // FinalStatus of the RenderView.
+  //
+  // If the FinalStatus of the RenderView is successfully set, returns true and
+  // sets |actual_final_status| to |desired_final_status|.
+  //
+  // If the FinalStatus of the RenderView was already set, returns false and
+  // sets |actual_final_status| to the actual FinalStatus of the RenderView.
+  //
+  // If the RenderView is not a prerendering RenderView, returns false and sets
+  // |actual_final_status| to FINAL_STATUS_MAX.
+  bool SetFinalStatus(int child_id, int route_id,
+                      FinalStatus desired_final_status,
+                      FinalStatus* actual_final_status);
 
   // Add/remove the specified pair to |possibly_prerendering_io_thread_set_| on
   // the IO Thread.