Refactor prerender pending swap logic.

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 416 matching lines @@
 
 void PrerenderManager::CancelAllPrerenders() {
   DCHECK(CalledOnValidThread());
   while (!active_prerenders_.empty()) {
     PrerenderContents* prerender_contents =
         active_prerenders_.front()->contents();
     prerender_contents->Destroy(FINAL_STATUS_CANCELLED);
   }
 }
 
-void PrerenderManager::ProcessMergeResult(
-    PrerenderData* prerender_data,
-    bool timed_out,
-    content::SessionStorageNamespace::MergeResult result) {
-  PendingSwap* pending_swap = prerender_data->pending_swap();
-  DCHECK(pending_swap);
-  // No pending_swap should never happen. If it does anyways (in a retail
-  // build), log this and bail.
-  if (!pending_swap) {
-    RecordEvent(prerender_data->contents(),
-                PRERENDER_EVENT_MERGE_RESULT_NO_PENDING_SWAPIN);
-    return;
-  }
-  if (timed_out) {
-    RecordEvent(prerender_data->contents(),
-                PRERENDER_EVENT_MERGE_RESULT_TIMEOUT_CB);
-  } else {
-    RecordEvent(prerender_data->contents(),
-                PRERENDER_EVENT_MERGE_RESULT_RESULT_CB);
-    UMA_HISTOGRAM_TIMES("Prerender.SessionStorageNamespaceMergeTime",
-                        pending_swap->GetElapsedTime());
-  }
-
-  // Any return here must call ClearPendingSwap on |prerender_data| before
-  // returning, with one exception: when the prerender was ultimately swapped
-  // in. In that case, SwapInternal will take care of deleting
-  // |prerender_data| and sending the appropriate notifications to the tracker.
-  if (timed_out) {
-    RecordEvent(prerender_data->contents(),
-                PRERENDER_EVENT_MERGE_RESULT_TIMED_OUT);
-    prerender_data->ClearPendingSwap();
-    return;
-  }
-
-  RecordEvent(prerender_data->contents(),
-              PRERENDER_EVENT_MERGE_RESULT_MERGE_DONE);
-
-  // Log the exact merge result in a histogram.
-  switch (result) {
-    case content::SessionStorageNamespace::MERGE_RESULT_NAMESPACE_NOT_FOUND:
-      RecordEvent(prerender_data->contents(),
-                  PRERENDER_EVENT_MERGE_RESULT_RESULT_NAMESPACE_NOT_FOUND);
-      break;
-    case content::SessionStorageNamespace::MERGE_RESULT_NAMESPACE_NOT_ALIAS:
-      RecordEvent(prerender_data->contents(),
-                  PRERENDER_EVENT_MERGE_RESULT_RESULT_NAMESPACE_NOT_ALIAS);
-      break;
-    case content::SessionStorageNamespace::MERGE_RESULT_NOT_LOGGING:
-      RecordEvent(prerender_data->contents(),
-                  PRERENDER_EVENT_MERGE_RESULT_RESULT_NOT_LOGGING);
-      break;
-    case content::SessionStorageNamespace::MERGE_RESULT_NO_TRANSACTIONS:
-      RecordEvent(prerender_data->contents(),
-                  PRERENDER_EVENT_MERGE_RESULT_RESULT_NO_TRANSACTIONS);
-      break;
-    case content::SessionStorageNamespace::MERGE_RESULT_TOO_MANY_TRANSACTIONS:
-      RecordEvent(prerender_data->contents(),
-                  PRERENDER_EVENT_MERGE_RESULT_RESULT_TOO_MANY_TRANSACTIONS);
-      break;
-    case content::SessionStorageNamespace::MERGE_RESULT_NOT_MERGEABLE:
-      RecordEvent(prerender_data->contents(),
-                  PRERENDER_EVENT_MERGE_RESULT_RESULT_NOT_MERGEABLE);
-      break;
-    case content::SessionStorageNamespace::MERGE_RESULT_MERGEABLE:
-      RecordEvent(prerender_data->contents(),
-                  PRERENDER_EVENT_MERGE_RESULT_RESULT_MERGEABLE);
-      break;
-    default:
-      NOTREACHED();
-  }
-
-  if (result != content::SessionStorageNamespace::MERGE_RESULT_MERGEABLE &&
-      result !=
-      content::SessionStorageNamespace::MERGE_RESULT_NO_TRANSACTIONS) {
-    RecordEvent(prerender_data->contents(),
-                PRERENDER_EVENT_MERGE_RESULT_MERGE_FAILED);
-    prerender_data->ClearPendingSwap();
-    return;
-  }
-
-  RecordEvent(prerender_data->contents(),
-              PRERENDER_EVENT_MERGE_RESULT_SWAPPING_IN);
-  // Notice that SwapInInternal, on success, will delete |prerender_data|
-  // and |pending_swap|. Therefore, we have to pass a new GURL object rather
-  // than a reference to the one in |pending_swap|.
-  content::WebContents* new_web_contents =
-      SwapInternal(GURL(pending_swap->url()),
-                   pending_swap->target_contents(),
-                   prerender_data);
-  if (!new_web_contents) {
-    RecordEvent(prerender_data->contents(),
-                PRERENDER_EVENT_MERGE_RESULT_SWAPIN_FAILED);
-    prerender_data->ClearPendingSwap();
-  }
-}
-
 bool PrerenderManager::MaybeUsePrerenderedPage(const GURL& url,
                                                chrome::NavigateParams* params) {
   DCHECK(CalledOnValidThread());
 
   content::WebContents* web_contents = params->target_contents;
   DCHECK(!IsWebContentsPrerendering(web_contents, NULL));
 
   // Don't prerender if the navigation involves some special parameters.
   if (params->uses_post || !params->extra_headers.empty())
     return false;
 
-  content::WebContents* new_web_contents = SwapInternal(url, web_contents,
-                                                          NULL);
-  if (!new_web_contents)
-    return false;
-
-  // Record the new target_contents for the callers.
-  params->target_contents = new_web_contents;
-  return true;
-}
-
-content::WebContents* PrerenderManager::SwapInternal(
-    const GURL& url,
-    content::WebContents* web_contents,
-    PrerenderData* swap_candidate) {
-  DCHECK(CalledOnValidThread());
-  DCHECK(!IsWebContentsPrerendering(web_contents, NULL));
-
   DeleteOldEntries();
   to_delete_prerenders_.clear();
-  // TODO(ajwong): This doesn't handle isolated apps correctly.
-
-  // Only if this WebContents is used in a tabstrip may be swap.
-  // We check this by examining whether its CoreTabHelper has a delegate.
-  CoreTabHelper* core_tab_helper = CoreTabHelper::FromWebContents(web_contents);
-  if (!core_tab_helper || !core_tab_helper->delegate()) {
-    RecordEvent(NULL, PRERENDER_EVENT_SWAPIN_NO_DELEGATE);
-    return NULL;
-  }
 
   // First, try to find prerender data with the correct session storage
   // namespace.
+  // TODO(ajwong): This doesn't handle isolated apps correctly.
   PrerenderData* prerender_data = FindPrerenderData(
       url,
       web_contents->GetController().GetDefaultSessionStorageNamespace());
 
   // If this failed, we may still find a prerender for the same URL, but a
   // different session storage namespace. If we do, we might have to perform
   // a merge.
   if (!prerender_data) {
     prerender_data = FindPrerenderData(url, NULL);
   } else {
     RecordEvent(prerender_data->contents(),
                 PRERENDER_EVENT_SWAPIN_CANDIDATE_NAMESPACE_MATCHES);
   }
 
   if (!prerender_data)
-    return NULL;
+    return false;
   RecordEvent(prerender_data->contents(), PRERENDER_EVENT_SWAPIN_CANDIDATE);
   DCHECK(prerender_data->contents());
 
   // If there is currently a merge pending for this prerender data,
   // or this webcontents, do not swap in, but give the merge a chance to
   // finish and swap into the intended target webcontents.
-  if (prerender_data != swap_candidate && prerender_data->pending_swap()) {
-    return NULL;
-  }
+  if (prerender_data->pending_swap())
+    return false;

[mmenke, 2013/12/09 21:04:44]

Random comment:  We could still theoretically have another usable prerender with
a pending swap in this case.  I don't think it's worth worrying about, but I do
think it's worth being aware of funky cases like that.

 
   RecordEvent(prerender_data->contents(),
               PRERENDER_EVENT_SWAPIN_NO_MERGE_PENDING);
   SessionStorageNamespace* target_namespace =
       web_contents->GetController().GetDefaultSessionStorageNamespace();
   SessionStorageNamespace* prerender_namespace =
       prerender_data->contents()->GetSessionStorageNamespace();
   // Only when actually prerendering is session storage namespace merging an
   // issue. For the control group, it will be assumed that the merge succeeded.
   if (prerender_namespace && prerender_namespace != target_namespace &&
       !prerender_namespace->IsAliasOf(target_namespace)) {
     if (!ShouldMergeSessionStorageNamespaces()) {
       RecordEvent(prerender_data->contents(),
                   PRERENDER_EVENT_SWAPIN_MERGING_DISABLED);
-      return NULL;
+      return false;
     }
     RecordEvent(prerender_data->contents(),
                 PRERENDER_EVENT_SWAPIN_ISSUING_MERGE);
-    // There should never be a |pending_swap| if we get to here:
-    // Per check above, |pending_swap| may only be != NULL when
-    // processing a successful merge, as indicated by |swap_candidate|
-    // != NULL. But in that case, there should be no need for yet another merge.
-    DCHECK(!prerender_data->pending_swap());
-    if (prerender_data->pending_swap()) {
-      // In retail builds, log this error and bail.
-      RecordEvent(prerender_data->contents(),
-                  PRERENDER_EVENT_MERGE_FOR_SWAPIN_CANDIDATE);
-      return NULL;
-    }
-    PendingSwap* pending_swap = new PendingSwap(
-        prerender_tracker_,
-        web_contents,
-        prerender_data,
-        url,
-        base::Bind(&PrerenderManager::ProcessMergeResult,
-                   AsWeakPtr(),
-                   prerender_data,
-                   true,
-                   SessionStorageNamespace::MERGE_RESULT_NOT_MERGEABLE),
-        base::Bind(&PrerenderManager::ProcessMergeResult,
-                   AsWeakPtr(),
-                   prerender_data,
-                   false));
-    prerender_data->set_pending_swap(pending_swap);
-    prerender_namespace->Merge(
-        true,
-        prerender_data->contents()->child_id(),
-        target_namespace,
-        pending_swap->GetMergeResultCallback());
-    base::MessageLoop::current()->PostDelayedTask(
-        FROM_HERE,
-        pending_swap->GetTimeoutCallback(),
-        base::TimeDelta::FromMilliseconds(
-            kSessionStorageNamespaceMergeTimeoutMs));
+    prerender_data->set_pending_swap(new PendingSwap(
+        this, web_contents, prerender_data, url));
+    prerender_data->pending_swap()->BeginSwap();
+    return false;

[mmenke, 2013/12/09 21:04:44]

optional:  Suggest a comment that we'll defer the main frame load until the
pending swap completes, despite returning false.

[davidben, 2013/12/09 23:08:55]

Done.

+  }
+
+  // No need to merge; swap synchronously.
+  WebContents* new_web_contents = SwapInternal(url, web_contents,
+                                               prerender_data);
+  if (!new_web_contents)
+    return false;
+
+  // Record the new target_contents for the callers.
+  params->target_contents = new_web_contents;
+  return true;
+}
+
+WebContents* PrerenderManager::SwapInternal(
+    const GURL& url,
+    WebContents* web_contents,
+    PrerenderData* prerender_data) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(!IsWebContentsPrerendering(web_contents, NULL));
+
+  // Only if this WebContents is used in a tabstrip may be swap.

[mmenke, 2013/12/09 21:04:44]

nit:  Know this was an old comment, but could you fix the grammar here?

[davidben, 2013/12/09 23:08:55]

Done.

+  // We check this by examining whether its CoreTabHelper has a delegate.
+  CoreTabHelper* core_tab_helper = CoreTabHelper::FromWebContents(web_contents);
+  if (!core_tab_helper || !core_tab_helper->delegate()) {
+    RecordEvent(prerender_data->contents(), PRERENDER_EVENT_SWAPIN_NO_DELEGATE);
     return NULL;
   }
 
   if (IsNoSwapInExperiment(prerender_data->contents()->experiment_id()))
     return NULL;
 
   if (WebContents* new_web_contents =
       prerender_data->contents()->prerender_contents()) {
     if (web_contents == new_web_contents)
       return NULL;  // Do not swap in to ourself.
@@ skipping 180 matching lines @@
       NeedMatchCompleteDummyForFinalStatus(final_status) &&
       ActuallyPrerendering()) {
     // TODO(tburkard): I'd like to DCHECK that we are actually prerendering.
     // However, what if new conditions are added and
     // NeedMatchCompleteDummyForFinalStatus is not being updated.  Not sure
     // what's the best thing to do here.  For now, I will just check whether
     // we are actually prerendering.
     (*it)->MakeIntoMatchCompleteReplacement();
   } else {
     to_delete_prerenders_.push_back(*it);
-    (*it)->ClearPendingSwap();
     active_prerenders_.weak_erase(it);
   }
 
   // Destroy the old WebContents relatively promptly to reduce resource usage.
   PostCleanupTask();
 }
 
 // static
 void PrerenderManager::RecordPerceivedPageLoadTime(
     base::TimeDelta perceived_page_load_time,
@@ skipping 410 matching lines @@
 
 void PrerenderManager::PrerenderData::ClearPendingSwap() {
   pending_swap_.reset(NULL);
 }
 
 PrerenderContents* PrerenderManager::PrerenderData::ReleaseContents() {
   return contents_.release();
 }
 
 PrerenderManager::PendingSwap::PendingSwap(
-    PrerenderTracker* prerender_tracker,
+    PrerenderManager* manager,
     content::WebContents* target_contents,
     PrerenderData* prerender_data,
-    const GURL& url,
-    const base::Closure& timeout_cb,
-    const SessionStorageNamespace::MergeResultCallback& merge_result_cb)
+    const GURL& url)
     : content::WebContentsObserver(target_contents),
-      prerender_tracker_(prerender_tracker),
+      manager_(manager),
       target_contents_(target_contents),
       prerender_data_(prerender_data),
       url_(url),
-      timeout_cb_(timeout_cb),
-      merge_result_cb_(merge_result_cb),
-      start_time_(base::TimeTicks::Now()) {
+      start_time_(base::TimeTicks::Now()),
+      weak_factory_(this) {
   RenderViewCreated(target_contents->GetRenderViewHost());
 }
 
 PrerenderManager::PendingSwap::~PendingSwap() {
-    timeout_cb_.Cancel();
-    merge_result_cb_.Cancel();
-    for (size_t i = 0; i < rvh_ids_.size(); i++) {
-      prerender_tracker_->RemovePrerenderPendingSwap(rvh_ids_[i], false);
-    }
+  for (size_t i = 0; i < rvh_ids_.size(); i++) {
+    manager_->prerender_tracker()->RemovePrerenderPendingSwap(
+        rvh_ids_[i], false);
+  }
 }
 
-const base::Closure& PrerenderManager::PendingSwap::GetTimeoutCallback() {
-  return timeout_cb_.callback();
-}
+void PrerenderManager::PendingSwap::BeginSwap() {
+  SessionStorageNamespace* target_namespace =
+      target_contents_->GetController().GetDefaultSessionStorageNamespace();
+  SessionStorageNamespace* prerender_namespace =
+      prerender_data_->contents()->GetSessionStorageNamespace();
 
-void PrerenderManager::PendingSwap::SwapSuccessful() {
-  for (size_t i = 0; i < rvh_ids_.size(); i++) {
-    prerender_tracker_->RemovePrerenderPendingSwap(rvh_ids_[i], true);
-  }
-  rvh_ids_.clear();
-}
+  prerender_namespace->Merge(
+      true, prerender_data_->contents()->child_id(),
+      target_namespace,
+      base::Bind(&PrerenderManager::PendingSwap::OnMergeCompleted,
+                 weak_factory_.GetWeakPtr()));
 
-const SessionStorageNamespace::MergeResultCallback&
-PrerenderManager::PendingSwap::GetMergeResultCallback() {
-  return merge_result_cb_.callback();
+  merge_timeout_.Start(
+      FROM_HERE,
+      base::TimeDelta::FromMilliseconds(
+          kSessionStorageNamespaceMergeTimeoutMs),
+      this, &PrerenderManager::PendingSwap::OnMergeTimeout);
 }
 
 void PrerenderManager::PendingSwap::ProvisionalChangeToMainFrameUrl(
         const GURL& url,
         content::RenderViewHost* render_view_host) {
   // We must only cancel the pending swap if the |url| navigated to is not
   // the URL being attempted to be swapped in. That's because in the normal
   // flow, a ProvisionalChangeToMainFrameUrl will happen for the URL attempted
   // to be swapped in immediately after the pending swap has issued its merge.
   if (url != url_)
     prerender_data_->ClearPendingSwap();
 }
 
 void PrerenderManager::PendingSwap::DidCommitProvisionalLoadForFrame(
         int64 frame_id,
         const string16& frame_unique_name,
         bool is_main_frame,
         const GURL& validated_url,
         content::PageTransition transition_type,
         content::RenderViewHost* render_view_host){
   if (!is_main_frame)
     return;
-  if (validated_url != url_)
-    prerender_data_->ClearPendingSwap();
+  prerender_data_->ClearPendingSwap();
 }
 
 void PrerenderManager::PendingSwap::RenderViewCreated(
     content::RenderViewHost* render_view_host) {
+  // Record the RVH id in the tracker to install throttles on MAIN_FRAME
+  // requests from that route.
   int child_id = render_view_host->GetProcess()->GetID();
   int route_id = render_view_host->GetRoutingID();
   PrerenderTracker::ChildRouteIdPair child_route_id_pair(child_id, route_id);
   rvh_ids_.push_back(child_route_id_pair);
-  prerender_tracker_->AddPrerenderPendingSwap(child_route_id_pair, url_);
+  manager_->prerender_tracker()->AddPrerenderPendingSwap(
+      child_route_id_pair, url_);
 }
 
 void PrerenderManager::PendingSwap::DidFailProvisionalLoad(
         int64 frame_id,
         const string16& frame_unique_name,
         bool is_main_frame,
         const GURL& validated_url,
         int error_code,
         const string16& error_description,
         content::RenderViewHost* render_view_host) {
+  if (!is_main_frame)
+    return;
   prerender_data_->ClearPendingSwap();
 }
 
 void PrerenderManager::PendingSwap::WebContentsDestroyed(
     content::WebContents* web_contents) {
   prerender_data_->ClearPendingSwap();
 }
 
-base::TimeDelta PrerenderManager::PendingSwap::GetElapsedTime() {
-  return base::TimeTicks::Now() - start_time_;
+void PrerenderManager::PendingSwap::SwapSuccessful() {

[mmenke, 2013/12/09 21:04:44]

Can we just delete the swap object and get rid of this function?

[davidben, 2013/12/09 23:08:55]

There's the parameter for whether the swap succeeded or not. I replaced it with
a swap_successful parameter.

+  for (size_t i = 0; i < rvh_ids_.size(); i++) {
+    manager_->prerender_tracker()->RemovePrerenderPendingSwap(
+        rvh_ids_[i], true);
+  }
+  rvh_ids_.clear();
+}
+
+void PrerenderManager::PendingSwap::RecordEvent(PrerenderEvent event) const {
+  manager_->RecordEvent(prerender_data_->contents(), event);
+}
+
+void PrerenderManager::PendingSwap::OnMergeCompleted(
+    SessionStorageNamespace::MergeResult result) {
+  UMA_HISTOGRAM_TIMES("Prerender.SessionStorageNamespaceMergeTime",
+                      base::TimeTicks::Now() - start_time_);
+  RecordEvent(PRERENDER_EVENT_MERGE_RESULT_MERGE_DONE);
+
+  // Log the exact merge result in a histogram.
+  switch (result) {
+    case SessionStorageNamespace::MERGE_RESULT_NAMESPACE_NOT_FOUND:
+      RecordEvent(PRERENDER_EVENT_MERGE_RESULT_RESULT_NAMESPACE_NOT_FOUND);
+      break;
+    case SessionStorageNamespace::MERGE_RESULT_NAMESPACE_NOT_ALIAS:
+      RecordEvent(PRERENDER_EVENT_MERGE_RESULT_RESULT_NAMESPACE_NOT_ALIAS);
+      break;
+    case SessionStorageNamespace::MERGE_RESULT_NOT_LOGGING:
+      RecordEvent(PRERENDER_EVENT_MERGE_RESULT_RESULT_NOT_LOGGING);
+      break;
+    case SessionStorageNamespace::MERGE_RESULT_NO_TRANSACTIONS:
+      RecordEvent(PRERENDER_EVENT_MERGE_RESULT_RESULT_NO_TRANSACTIONS);
+      break;
+    case SessionStorageNamespace::MERGE_RESULT_TOO_MANY_TRANSACTIONS:
+      RecordEvent(PRERENDER_EVENT_MERGE_RESULT_RESULT_TOO_MANY_TRANSACTIONS);
+      break;
+    case SessionStorageNamespace::MERGE_RESULT_NOT_MERGEABLE:
+      RecordEvent(PRERENDER_EVENT_MERGE_RESULT_RESULT_NOT_MERGEABLE);
+      break;
+    case SessionStorageNamespace::MERGE_RESULT_MERGEABLE:
+      RecordEvent(PRERENDER_EVENT_MERGE_RESULT_RESULT_MERGEABLE);
+      break;
+    default:
+      NOTREACHED();
+  }
+
+  if (result != SessionStorageNamespace::MERGE_RESULT_MERGEABLE &&
+      result != SessionStorageNamespace::MERGE_RESULT_NO_TRANSACTIONS) {
+    RecordEvent(PRERENDER_EVENT_MERGE_RESULT_MERGE_FAILED);
+    prerender_data_->ClearPendingSwap();
+    return;
+  }
+
+  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_|.
+  WebContents* new_web_contents =
+      manager_->SwapInternal(GURL(url_), target_contents_, prerender_data_);
+  if (!new_web_contents) {
+    RecordEvent(PRERENDER_EVENT_MERGE_RESULT_SWAPIN_FAILED);
+    prerender_data_->ClearPendingSwap();
+  }
+}
+
+void PrerenderManager::PendingSwap::OnMergeTimeout() {
+  RecordEvent(PRERENDER_EVENT_MERGE_RESULT_TIMED_OUT);
+  prerender_data_->ClearPendingSwap();
 }
 
 void PrerenderManager::SetPrerenderContentsFactory(
     PrerenderContents::Factory* prerender_contents_factory) {
   DCHECK(CalledOnValidThread());
   prerender_contents_factory_.reset(prerender_contents_factory);
 }
 
 
 void PrerenderManager::StartPendingPrerenders(
@@ skipping 557 matching lines @@
 void PrerenderManager::RecordEvent(PrerenderContents* contents,
                                    PrerenderEvent event) const {
   if (!contents)
     histograms_->RecordEvent(ORIGIN_NONE, kNoExperiment, event);
   else
     histograms_->RecordEvent(contents->origin(), contents->experiment_id(),
                              event);
 }
 
 }  // namespace prerender