Make PrerenderHandle an observer of PrerenderContents.

chrome/browser/prerender/prerender_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_manager.h"
 
 #include <algorithm>
 #include <functional>
 #include <string>
 #include <vector>
@@ skipping 206 matching lines @@
       break;
     default:
       break;
   }
 }
 
 PrerenderManager::~PrerenderManager() {
   // The earlier call to ProfileKeyedService::Shutdown() should have emptied
   // these vectors already.
   DCHECK(active_prerenders_.empty());
-  DCHECK(pending_prerenders_.empty());
   DCHECK(to_delete_prerenders_.empty());
 }
 
 void PrerenderManager::Shutdown() {
   DestroyAllContents(FINAL_STATUS_MANAGER_SHUTDOWN);
   STLDeleteElements(&prerender_conditions_);
   on_close_tab_contents_deleters_.clear();
   // Must happen before |profile_| is set to NULL as
   // |local_predictor_| accesses it.
   if (local_predictor_)
     local_predictor_->Shutdown();
   profile_ = NULL;
 
   DCHECK(active_prerenders_.empty());
-  pending_prerenders_.clear();
 }
 
 PrerenderHandle* PrerenderManager::AddPrerenderFromLinkRelPrerender(
     int process_id,
     int route_id,
     const GURL& url,
     const content::Referrer& referrer,
     const gfx::Size& size) {
 #if defined(OS_ANDROID)
   // TODO(jcivelli): http://crbug.com/113322 We should have an option to disable
@@ skipping 19 matching lines @@
     session_storage_namespace =
         source_web_contents->GetController()
             .GetDefaultSessionStorageNamespace();
   }
 
   if (PrerenderData* parent_prerender_data =
           FindPrerenderDataForChildAndRoute(process_id, route_id)) {
     // Instead of prerendering from inside of a running prerender, we will defer
     // this request until its launcher is made visible.
     if (PrerenderContents* contents = parent_prerender_data->contents()) {
-      pending_prerenders_.push_back(new PrerenderData(this));
       PrerenderHandle* prerender_handle =
-          new PrerenderHandle(pending_prerenders_.back());
-      DCHECK(prerender_handle->IsPending());
-
+          new PrerenderHandle(static_cast<PrerenderData*>(NULL));
       scoped_ptr<PrerenderContents::PendingPrerenderInfo>
           pending_prerender_info(new PrerenderContents::PendingPrerenderInfo(
               prerender_handle->weak_ptr_factory_.GetWeakPtr(),
               origin, url, referrer, size));
 
       contents->AddPendingPrerender(pending_prerender_info.Pass());
       return prerender_handle;
     }
   }
 
@@ skipping 127 matching lines @@
 
   RenderViewHost* new_render_view_host =
       prerender_contents->prerender_contents()->web_contents()->
           GetRenderViewHost();
   new_render_view_host->Send(
       new PrerenderMsg_SetIsPrerendering(new_render_view_host->GetRoutingID(),
                                          false));
 
   // Start pending prerender requests from the PrerenderContents, if there are
   // any.
-  prerender_contents->StartPendingPrerenders();
+  prerender_contents->PrepareForUse();
 
   TabContents* new_tab_contents =
       prerender_contents->ReleasePrerenderContents();
   TabContents* old_tab_contents = TabContents::FromWebContents(web_contents);
   DCHECK(new_tab_contents);
   DCHECK(old_tab_contents);
 
   MarkWebContentsAsPrerendered(new_tab_contents->web_contents(),
                                prerender_contents->origin());
 
@@ skipping 405 matching lines @@
   CleanUpOldNavigations();
 }
 
 // protected
 struct PrerenderManager::PrerenderData::OrderByExpiryTime {
   bool operator()(const PrerenderData* a, const PrerenderData* b) const {
     return a->expiry_time() < b->expiry_time();
   }
 };
 
-PrerenderManager::PrerenderData::PrerenderData(PrerenderManager* manager)
-    : manager_(manager), contents_(NULL), handle_count_(0) {
-}
-
 PrerenderManager::PrerenderData::PrerenderData(PrerenderManager* manager,
                                                PrerenderContents* contents,
                                                base::TimeTicks expiry_time)
     : manager_(manager),
       contents_(contents),
       handle_count_(0),
       expiry_time_(expiry_time) {
 }
 
 PrerenderManager::PrerenderData::~PrerenderData() {
 }
 
 void PrerenderManager::PrerenderData::MakeIntoMatchCompleteReplacement() {
   DCHECK(contents_);
   contents_->set_match_complete_status(
       PrerenderContents::MATCH_COMPLETE_REPLACED);
   PrerenderData* to_delete = new PrerenderData(manager_, contents_.release(),
                                                expiry_time_);
   contents_.reset(to_delete->contents_->CreateMatchCompleteReplacement());
   manager_->to_delete_prerenders_.push_back(to_delete);
 }
 
-void PrerenderManager::PrerenderData::OnNewHandle() {
-  DCHECK(contents_ || handle_count_ == 0) <<
-      "Cannot create multiple handles to a pending prerender.";
+void PrerenderManager::PrerenderData::OnHandleCreated(PrerenderHandle* handle) {
   ++handle_count_;
+  DCHECK_NE(static_cast<PrerenderContents*>(NULL), contents_);

[mmenke, 2012/12/12 16:54:12]

nit:  DCHECKs are generally put at the start of functions.

[mmenke, 2012/12/12 16:54:12]

May want to put one of these in the constructor as well.

[gavinp, 2012/12/13 13:38:03]

Done & Done.

+  contents_->AddObserver(handle);
 }
 
-void PrerenderManager::PrerenderData::OnNavigateAwayByHandle() {
-  if (!contents_) {
-    DCHECK_EQ(1, handle_count_);
-    // Pending prerenders are not maintained in the active_prerenders_, so they
-    // will not get normal expiry. Since this prerender hasn't even been
-    // launched yet, and it's held by a page that is being prerendered, we will
-    // just delete it.
-    manager_->DestroyPendingPrerenderData(this);
-  } else {
-    DCHECK_LE(0, handle_count_);
-    // We intentionally don't decrement the handle count here, so that the
-    // prerender won't be canceled until it times out.
-    manager_->SourceNavigatedAway(this);
+void PrerenderManager::PrerenderData::OnHandleNavigatedAway(
+    PrerenderHandle* handle) {
+  DCHECK_LT(0, handle_count_);
+  DCHECK_NE(static_cast<PrerenderContents*>(NULL), contents_);
+  // We intentionally don't decrement the handle count here, so that the
+  // prerender won't be canceled until it times out.
+  manager_->SourceNavigatedAway(this);
+}
+
+void PrerenderManager::PrerenderData::OnHandleCanceled(
+    PrerenderHandle* handle) {
+  DCHECK_LE(1, handle_count_);
+  DCHECK_NE(static_cast<PrerenderContents*>(NULL), contents_);
+
+  if (--handle_count_ == 0) {
+    // This will eventually remove this object from active_prerenders_.
+    contents_->Destroy(FINAL_STATUS_CANCELLED);
   }
 }
 
-void PrerenderManager::PrerenderData::OnCancelByHandle() {
-  DCHECK_LE(1, handle_count_);
-  DCHECK(contents_ || handle_count_ == 1);
-
-  if (--handle_count_ == 0) {
-    if (contents_) {
-      // This will eventually remove this object from active_prerenders_.
-      contents_->Destroy(FINAL_STATUS_CANCELLED);
-    } else {
-      manager_->DestroyPendingPrerenderData(this);
-    }
-  }
-}
-
 PrerenderContents* PrerenderManager::PrerenderData::ReleaseContents() {
   return contents_.release();
 }
 
 void PrerenderManager::SetPrerenderContentsFactory(
     PrerenderContents::Factory* prerender_contents_factory) {
   DCHECK(CalledOnValidThread());
   prerender_contents_factory_.reset(prerender_contents_factory);
 }
 
 void PrerenderManager::StartPendingPrerenders(
     const int process_id,
     ScopedVector<PrerenderContents::PendingPrerenderInfo>* pending_prerenders,
     content::SessionStorageNamespace* session_storage_namespace) {
   for (ScopedVector<PrerenderContents::PendingPrerenderInfo>::iterator
            it = pending_prerenders->begin();
        it != pending_prerenders->end(); ++it) {
     PrerenderContents::PendingPrerenderInfo* info = *it;
     PrerenderHandle* existing_prerender_handle =
         info->weak_prerender_handle.get();
-    if (!existing_prerender_handle || !existing_prerender_handle->IsValid())
+    if (!existing_prerender_handle)
+      continue;
+    if (existing_prerender_handle->has_been_canceled())

[mmenke, 2012/12/12 16:54:12]

Since we delete handles when cancelled, and don't start prerenders on cancel, is
this check even needed?

I think we can get rid of has_been_canceled() and the underlying flag entirely.

Fine to keep this as-is if you disagree, or feel it may be needed in the future.

[gavinp, 2012/12/13 13:38:03]

I think that's a good idea. It simplifies the handle, I've moved cancelation
logic into the destructor.

       continue;
 
-    DCHECK(existing_prerender_handle->IsPending());
+    DCHECK(!existing_prerender_handle->IsPrerendering());
     DCHECK(process_id == -1 || session_storage_namespace);
 
-    scoped_ptr<PrerenderHandle> swap_prerender_handle(AddPrerender(
+    scoped_ptr<PrerenderHandle> new_prerender_handle(AddPrerender(
         info->origin, process_id,
         info->url, info->referrer, info->size,
         session_storage_namespace));
-    if (swap_prerender_handle.get()) {
+    if (new_prerender_handle) {
       // AddPrerender has returned a new prerender handle to us. We want to make
-      // |existing_prerender_handle| active, so swap the underlying
-      // PrerenderData between the two handles, and delete our old handle (which
-      // will release our entry in the pending_prerender_list_).
-      existing_prerender_handle->SwapPrerenderDataWith(
-          swap_prerender_handle.get());
-      swap_prerender_handle->OnCancel();
+      // |existing_prerender_handle| active, so move the underlying
+      // PrerenderData to our new handle.
+      existing_prerender_handle->AdoptPrerenderDataFrom(
+          new_prerender_handle.get());
       continue;
     }
-
-    // We could not start our Prerender. Canceling the existing handle will make
-    // it return false for PrerenderHandle::IsPending(), and will release the
-    // PrerenderData from pending_prerender_list_.
-    existing_prerender_handle->OnCancel();
   }
 }
 
 void PrerenderManager::SourceNavigatedAway(PrerenderData* prerender_data) {
   // The expiry time of our prerender data will likely change because of
   // this navigation. This requires a resort of active_prerenders_.
   ScopedVector<PrerenderData>::iterator it =
       std::find(active_prerenders_.begin(), active_prerenders_.end(),
                 prerender_data);
   if (it == active_prerenders_.end())
     return;
 
   (*it)->set_expiry_time(
       std::min((*it)->expiry_time(),
                GetExpiryTimeForNavigatedAwayPrerender()));
   SortActivePrerenders();
 }
 
-void PrerenderManager::DestroyPendingPrerenderData(
-    PrerenderData* pending_prerender_data) {
-  ScopedVector<PrerenderData>::iterator it =
-      std::find(pending_prerenders_.begin(), pending_prerenders_.end(),
-                pending_prerender_data);
-  if (it == pending_prerenders_.end())
-    return;
-  pending_prerenders_.erase(it);
-}
-
 // private
 PrerenderHandle* PrerenderManager::AddPrerender(
     Origin origin,
     int process_id,
     const GURL& url_arg,
     const content::Referrer& referrer,
     const gfx::Size& size,
     SessionStorageNamespace* session_storage_namespace) {
   DCHECK(CalledOnValidThread());
 
@@ skipping 352 matching lines @@
   if (!render_process_host || !render_process_host->GetBrowserContext())
     return NULL;
   Profile* profile = Profile::FromBrowserContext(
       render_process_host->GetBrowserContext());
   if (!profile)
     return NULL;
   return PrerenderManagerFactory::GetInstance()->GetForProfile(profile);
 }
 
 }  // namespace prerender