Remove PrerenderTracker::TryCancel* methods and associated ones, since...

Remove PrerenderTracker::TryCancel* methods and associated ones, since destruction of prerender only happens on the UI thread through PrerenderContents now.

BUG=304341
R=mmenke@chromium.org, tburkard@chromium.org

Review URL: https://codereview.chromium.org/132613002

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

chrome/browser/prerender/prerender_contents.cc

@@ -666,22 +666,6 @@ void PrerenderContents::Destroy(FinalStatus final_status) {
   if (prerendering_has_been_cancelled_)
     return;
 
-  if (child_id_ != -1 && route_id_ != -1) {
-    // Cancel the prerender in the PrerenderTracker.  This is needed
-    // because destroy may be called directly from the UI thread without calling
-    // TryCancel().  This is difficult to completely avoid, since prerendering
-    // can be cancelled before a RenderView is created.
-    bool is_cancelled = prerender_manager()->prerender_tracker()->TryCancel(
-        child_id_, route_id_, final_status);
-    CHECK(is_cancelled);
-
-    // A different final status may have been set already from another thread.
-    // If so, use it instead.
-    if (!prerender_manager()->prerender_tracker()->
-            GetFinalStatus(child_id_, route_id_, &final_status)) {
-      NOTREACHED();
-    }
-  }
   SetFinalStatus(final_status);
 
   prerendering_has_been_cancelled_ = true;

chrome/browser/prerender/prerender_manager.cc

@@ -413,15 +413,6 @@ PrerenderHandle* PrerenderManager::AddPrerenderForInstant(
                       session_storage_namespace);
 }
 
-void PrerenderManager::DestroyPrerenderForRenderView(
-    int process_id, int view_id, FinalStatus final_status) {
-  DCHECK(CalledOnValidThread());
-  if (PrerenderData* prerender_data =
-          FindPrerenderDataForChildAndRoute(process_id, view_id)) {
-    prerender_data->contents()->Destroy(final_status);
-  }
-}
-
 void PrerenderManager::CancelAllPrerenders() {
   DCHECK(CalledOnValidThread());
   while (!active_prerenders_.empty()) {
@@ -600,12 +591,6 @@ WebContents* PrerenderManager::SwapInternal(
   CHECK(prerender_data->contents()->GetChildId(&child_id));
   CHECK(prerender_data->contents()->GetRouteId(&route_id));
 
-  // Try to set the prerendered page as used, so any subsequent attempts to
-  // cancel on other threads will fail.  If this fails because the prerender
-  // was already cancelled, possibly on another thread, fail.
-  if (!prerender_tracker_->TryUse(child_id, route_id))
-    return NULL;
-
   // At this point, we've determined that we will use the prerender.
   if (prerender_data->pending_swap())
     prerender_data->pending_swap()->set_swap_successful(true);
@@ -1281,14 +1266,21 @@ void PrerenderManager::PendingSwap::OnMergeCompleted(
   }
 
   RecordEvent(PRERENDER_EVENT_MERGE_RESULT_SWAPPING_IN);
-  // Note that SwapInternal, on success, will delete |prerender_data_| and
-  // |this|. Pass in a new GURL object rather than a reference to |url_|.
-  //
-  // TODO(davidben): See about deleting PrerenderData asynchronously so this
-  // behavior is more reasonable.
-  WebContents* new_web_contents =
-      manager_->SwapInternal(GURL(url_), target_contents_, prerender_data_,
-                             should_replace_current_entry_);
+
+  WebContents* new_web_contents = NULL;
+  // Ensure that the prerendering hasn't been destroyed in the meantime.
+  if (prerender_data_->contents()->final_status() == FINAL_STATUS_MAX) {
+    // Note that SwapInternal, on success, will delete |prerender_data_| and
+    // |this|. Pass in a new GURL object rather than a reference to |url_|.
+    //
+    // TODO(davidben): See about deleting PrerenderData asynchronously so this
+    // behavior is more reasonable.
+
+    new_web_contents =  manager_->SwapInternal(
+        GURL(url_), target_contents_, prerender_data_,
+        should_replace_current_entry_);
+  }
+
   if (!new_web_contents) {
     RecordEvent(PRERENDER_EVENT_MERGE_RESULT_SWAPIN_FAILED);
     prerender_data_->ClearPendingSwap();

chrome/browser/prerender/prerender_manager.h

@@ -170,12 +170,6 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>,
       content::SessionStorageNamespace* session_storage_namespace,
       const gfx::Size& size);
 
-  // If |process_id| and |view_id| refer to a running prerender, destroy
-  // it with |final_status|.
-  virtual void DestroyPrerenderForRenderView(int process_id,
-                                             int view_id,
-                                             FinalStatus final_status);
-
   // Cancels all active prerenders.
   void CancelAllPrerenders();
 

chrome/browser/prerender/prerender_tracker.cc

@@ -18,24 +18,6 @@ using content::BrowserThread;
 
 namespace prerender {
 
-namespace {
-
-void DestroyPrerenderForRenderViewOnUI(
-    const base::WeakPtr<PrerenderManager>& prerender_manager_weak_ptr,
-    int render_process_id,
-    int render_view_id,
-    FinalStatus final_status) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  PrerenderManager* prerender_manager = prerender_manager_weak_ptr.get();
-  if (!prerender_manager)
-    return;
-
-  prerender_manager->DestroyPrerenderForRenderView(
-      render_process_id, render_view_id, final_status);
-}
-
-}  // namespace
-
 struct RenderViewInfo {
   explicit RenderViewInfo(PrerenderManager* prerender_manager)
       : final_status(FINAL_STATUS_MAX),
@@ -52,51 +34,6 @@ PrerenderTracker::PrerenderTracker() {
 }
 
 PrerenderTracker::~PrerenderTracker() {
-  DCHECK(final_status_map_.empty());
-}
-
-bool PrerenderTracker::TryUse(int child_id, int route_id) {
-  DCHECK(CalledOnValidThread());
-  return SetFinalStatus(child_id, route_id, FINAL_STATUS_USED, NULL);
-}
-
-bool PrerenderTracker::TryCancel(
-    int child_id,
-    int route_id,
-    FinalStatus final_status) {
-  DCHECK_NE(FINAL_STATUS_USED, final_status);
-  DCHECK(final_status >= 0 && 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(
-    int child_id,
-    int route_id,
-    FinalStatus final_status) {
-  DCHECK_NE(FINAL_STATUS_USED, final_status);
-  DCHECK_LE(0, final_status);
-  DCHECK_GT(FINAL_STATUS_MAX, final_status);
-
-  if (!IsPrerenderingOnIOThread(child_id, route_id))
-    return false;
-  return TryCancel(child_id, route_id, final_status);
-}
-
-bool PrerenderTracker::GetFinalStatus(int child_id, int route_id,
-                                      FinalStatus* final_status) const {
-  ChildRouteIdPair child_route_id_pair(child_id, route_id);
-
-  base::AutoLock lock(final_status_map_lock_);
-  FinalStatusMap::const_iterator final_status_it =
-      final_status_map_.find(child_route_id_pair);
-  if (final_status_it == final_status_map_.end())
-    return false;
-  *final_status = final_status_it->second.final_status;
-  return true;
 }
 
 void PrerenderTracker::OnPrerenderStart(
@@ -113,14 +50,6 @@ void PrerenderTracker::OnPrerenderStart(
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::Bind(&AddPrerenderOnIOThreadTask, child_route_id_pair));
-
-  base::AutoLock lock(final_status_map_lock_);
-  // The RenderView should not already be prerendering.
-  DCHECK_EQ(0u, final_status_map_.count(child_route_id_pair));
-
-  final_status_map_.insert(
-      std::make_pair(child_route_id_pair,
-                     RenderViewInfo(prerender_contents->prerender_manager())));
 }
 
 void PrerenderTracker::OnPrerenderStop(
@@ -139,48 +68,6 @@ void PrerenderTracker::OnPrerenderStop(
       BrowserThread::IO, FROM_HERE,
       base::Bind(&RemovePrerenderOnIOThreadTask, child_route_id_pair,
                  prerender_contents->final_status()));
-
-  base::AutoLock lock(final_status_map_lock_);
-  size_t num_erased = final_status_map_.erase(child_route_id_pair);
-  DCHECK_EQ(1u, num_erased);
-}
-
-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);
-
-  base::AutoLock lock(final_status_map_lock_);
-  FinalStatusMap::iterator final_status_it =
-      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.
-    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 = desired_final_status;
-    if (desired_final_status != FINAL_STATUS_USED) {
-      BrowserThread::PostTask(
-          BrowserThread::UI, FROM_HERE,
-          base::Bind(&DestroyPrerenderForRenderViewOnUI,
-                     final_status_it->second.prerender_manager, child_id,
-                     route_id, desired_final_status));
-    }
-
-    if (actual_final_status)
-      *actual_final_status = desired_final_status;
-    return true;
-  }
-
-  if (actual_final_status)
-    *actual_final_status = final_status_it->second.final_status;
-  return false;
 }
 
 bool PrerenderTracker::IsPrerenderingOnIOThread(int child_id,

chrome/browser/prerender/prerender_tracker.h

@@ -26,8 +26,7 @@ class PrerenderResourceThrottle;
 struct RenderViewInfo;
 
 // PrerenderTracker is responsible for keeping track of all prerendering
-// RenderViews and their statuses.  Its list is guaranteed to be up to date
-// and can be modified on any thread.
+// RenderViews.
 class PrerenderTracker : public base::NonThreadSafe,
                          public PrerenderContents::Observer {
  public:
@@ -36,38 +35,6 @@ class PrerenderTracker : public base::NonThreadSafe,
   PrerenderTracker();
   virtual ~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.
-  // Can only be called only on the UI thread.  This method will not call
-  // PrerenderContents::SetFinalStatus() on the corresponding PrerenderContents.
-  //
-  // 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
-  // FinalStatus to |final_status|.  Returns true if the specified prerender has
-  // been cancelled, either as a result of this call or for any other reason.
-  // If the call results in cancelling a PrerenderContents, a task to destroy
-  // it is also posted to the UI thread.
-  //
-  // When true is returned, it is guaranteed that the RenderView will never
-  // be displayed.  When false is returned, the RenderView has either been
-  // swapped into a tab or has already been destroyed.
-  bool TryCancel(int child_id, int route_id, FinalStatus final_status);
-
-  // Same as above, but can only called on the IO Thread.  Does not acquire a
-  // lock when the RenderView is not being prerendered.
-  bool TryCancelOnIOThread(int child_id, int route_id,
-                           FinalStatus final_status);
-
-  // Gets the FinalStatus of the specified prerendered RenderView.  Returns
-  // |true| and sets |final_status| to the status of the RenderView if it
-  // is found, returns false otherwise.
-  bool GetFinalStatus(int child_id, int route_id,
-                      FinalStatus* final_status) const;
-
   // Returns whether or not a RenderView is prerendering.  Can only be called on
   // the IO thread.  Does not acquire a lock, so may claim a RenderView that has
   // been displayed or destroyed is still prerendering.
@@ -103,12 +70,6 @@ class PrerenderTracker : public base::NonThreadSafe,
 
  private:
   friend class PrerenderContents;
-  FRIEND_TEST_ALL_PREFIXES(PrerenderTrackerTest, PrerenderTrackerNull);
-  FRIEND_TEST_ALL_PREFIXES(PrerenderTrackerTest, PrerenderTrackerUsed);
-  FRIEND_TEST_ALL_PREFIXES(PrerenderTrackerTest, PrerenderTrackerCancelled);
-  FRIEND_TEST_ALL_PREFIXES(PrerenderTrackerTest, PrerenderTrackerCancelledOnIO);
-  FRIEND_TEST_ALL_PREFIXES(PrerenderTrackerTest, PrerenderTrackerCancelledFast);
-  FRIEND_TEST_ALL_PREFIXES(PrerenderTrackerTest, PrerenderTrackerMultiple);
 
   // Map of child/route id pairs to final statuses.
   typedef std::map<ChildRouteIdPair, RenderViewInfo> FinalStatusMap;
@@ -131,22 +92,6 @@ class PrerenderTracker : public base::NonThreadSafe,
   virtual void OnPrerenderStart(PrerenderContents* prerender_contents) OVERRIDE;
   virtual void OnPrerenderStop(PrerenderContents* prerender_contents) OVERRIDE;
 
-  // Attempts to set the FinalStatus of the specified RenderView to
-  // |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.
   void AddPrerenderOnIOThread(const ChildRouteIdPair& child_route_id_pair);
@@ -174,13 +119,6 @@ class PrerenderTracker : public base::NonThreadSafe,
 
   static PrerenderTracker* GetDefault();
 
-  // |final_status_map_lock_| protects access to |final_status_map_|.
-  mutable base::Lock final_status_map_lock_;
-  // Map containing child/route id pairs and their final statuses.  Must only be
-  // accessed while the lock is held.  Values are always accurate and up to
-  // date.
-  FinalStatusMap final_status_map_;
-
   // Resources that are throttled, pending a prerender use.  The keys are a
   // superset of child/route id pairs that are prerendering.  Can only access on
   // the IO thread.  May contain entries that have since been displayed.  Used