Handle interface to prerenders.

chrome/browser/ui/omnibox/omnibox_edit_model.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/ui/omnibox/omnibox_edit_model.h"
 
 #include <string>
 
 #include "base/metrics/histogram.h"
 #include "base/string_util.h"
@@ skipping 1092 matching lines @@
   return false;
 }
 
 void OmniboxEditModel::DoPrerender(const AutocompleteMatch& match) {
   // Do not prerender if the destination URL is the same as the current URL.
   if (match.destination_url == PermanentURL())
     return;
   // It's possible the tab strip does not have an active tab contents, for
   // instance if the tab has been closed or on return from a sleep state
   // (http://crbug.com/105689)
-  TabContents* tab = controller_->GetTabContents();
-  if (!tab)
-    return;
-  prerender::PrerenderManager* prerender_manager =
-      prerender::PrerenderManagerFactory::GetForProfile(tab->profile());
-  if (!prerender_manager)
-    return;
 
-  content::RenderViewHost* current_host =
-      tab->web_contents()->GetRenderViewHost();
-  gfx::Rect container_bounds;
-  tab->web_contents()->GetView()->GetContainerBounds(&container_bounds);
-  prerender_manager->AddPrerenderFromOmnibox(
-      match.destination_url, current_host->GetSessionStorageNamespace(),
-      container_bounds.size());
+  if (TabContents* tab = controller_->GetTabContents()) {

[Peter Kasting, 2012/07/13 19:35:09]

Nit: Don't combine set + test like this.  The old code that first set |tab|,
then checked it, was the right wayu to go.  (I also slightly preferred the early
return of the old code.)

[gavinp, 2012/07/13 19:59:10]

I can definitely change this if there's consensus, but I think
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Local_...
suggests this is a slightly preferred formulation for our style. I know I've had
conversations with brettw and jar about its usage before.

Should we resolve this or let it go by?

[Peter Kasting, 2012/07/13 20:56:41]

I haven't seen this used in Chromium code and I really dislike it.  I think the
Google style guide example in the while() loop is borderline, but arguably OK in
that this is effectively a re-written for() loop, where having a loop-scoped
variable is common.  Doing it in an if() statement on the other hand just asks
to be misread.

If I saw this in one of my readability reviews I'd ask the author to fix it.

+    gfx::Rect container_bounds;
+    tab->web_contents()->GetView()->GetContainerBounds(&container_bounds);
+    content::RenderViewHost* current_host =

[Peter Kasting, 2012/07/13 19:35:09]

Nit: Could just inline these next two temps into the following statement... not
sure having them separate buys a ton

[gavinp, 2012/07/13 19:59:10]

Done.

+        tab->web_contents()->GetRenderViewHost();
+    AutocompleteActionPredictor* action_predictor =
+        AutocompleteActionPredictorFactory::GetForProfile(profile_);
+

[Peter Kasting, 2012/07/13 19:35:09]

Nit: Unnecessary blank line

[gavinp, 2012/07/13 19:59:10]

Done.

+    action_predictor->StartPrerendering(
+        match.destination_url, current_host->GetSessionStorageNamespace(),
+        container_bounds.size());
+  }
 }
 
 void OmniboxEditModel::DoPreconnect(const AutocompleteMatch& match) {
   if (!match.destination_url.SchemeIs(chrome::kExtensionScheme)) {
     // Warm up DNS Prefetch cache, or preconnect to a search service.
     UMA_HISTOGRAM_ENUMERATION("Autocomplete.MatchType", match.type,
                               AutocompleteMatch::NUM_TYPES);
     if (profile_->GetNetworkPredictor()) {
       profile_->GetNetworkPredictor()->AnticipateOmniboxUrl(
           match.destination_url,
@@ skipping 31 matching lines @@
 }
 
 void OmniboxEditModel::ClassifyStringForPasteAndGo(
     const string16& text,
     AutocompleteMatch* match,
     GURL* alternate_nav_url) const {
   DCHECK(match);
   AutocompleteClassifierFactory::GetForProfile(profile_)->Classify(text,
       string16(), false, false, match, alternate_nav_url);
 }