Handle interface to prerenders.

chrome/browser/ui/omnibox/omnibox_edit_model.cc

Index: chrome/browser/ui/omnibox/omnibox_edit_model.cc
diff --git a/chrome/browser/ui/omnibox/omnibox_edit_model.cc b/chrome/browser/ui/omnibox/omnibox_edit_model.cc
index 0b80d5769852abf5d88f4f35bf13370b1392cc58..5bbd5a70951d9af3fb3c6cc637bee75abf837b2d 100644
--- a/chrome/browser/ui/omnibox/omnibox_edit_model.cc
+++ b/chrome/browser/ui/omnibox/omnibox_edit_model.cc
@@ -1110,21 +1110,19 @@ void OmniboxEditModel::DoPrerender(const AutocompleteMatch& match) {
   // 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) {