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 205 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 552 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::OnNewHandle(PrerenderHandle* handle) {
   ++handle_count_;
+  if (contents_)

[mmenke, 2012/12/06 22:22:15]

contents_ can no longer be null.

[gavinp, 2012/12/10 17:55:10]

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::OnNavigateAwayByHandle(
+    PrerenderHandle* handle) {
+  DCHECK_LT(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::OnCancelByHandle() {
+void PrerenderManager::PrerenderData::OnCancelByHandle(
+    PrerenderHandle* handle) {

[mmenke, 2012/12/06 22:22:15]

Why do we need the handle here?  Just for consistency?

[gavinp, 2012/12/10 17:55:10]

Just for consistency.

   DCHECK_LE(1, handle_count_);
-  DCHECK(contents_ || handle_count_ == 1);
+  DCHECK(contents_ || handle_count_ == 1)
+      << "Pending prerenders should never have multiple prerenders";
 
   if (--handle_count_ == 0) {
     if (contents_) {

[mmenke, 2012/12/06 22:22:15]

contents_ can't be null.

[gavinp, 2012/12/10 17:55:10]

Done.

       // 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())
       continue;
 
-    DCHECK(existing_prerender_handle->IsPending());
+    DCHECK(!existing_prerender_handle->IsValid());
     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));

[mmenke, 2012/12/07 16:46:08]

optional (Or feel free to think about in a later CL):  May make life simpler to
have AddPrerender just return a PrerenderContents*, and have the calling
function wrap it into the PrerenderHandle.

Think creating a handle only to destroy it makes things overly complicated here,
and creating the handle in a couple places isn't too terribly ugly.

[gavinp, 2012/12/10 17:55:10]

Definitely a good idea, and I'm definitely punting to a later CL.

-    if (swap_prerender_handle.get()) {
+    if (new_prerender_handle.get()) {

[mmenke, 2012/12/07 16:46:08]

nit:  Can remove the .get() while you're here.

[gavinp, 2012/12/10 17:55:10]

enne is Awesome! Done.

       // 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;
     }

[mmenke, 2012/12/07 16:46:08]

Random comment for the future - if a prerender is cancelled without really being
started...Should there be some notification of that?

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

I'm going to go with no. The launcher has no promises about which ones will be
started...

-
-    // 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 354 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