[Chrome OS] Make PrerenderConditionNetwork use NetworkStateHandler

chrome/browser/prerender/prerender_manager_factory.cc

Index: chrome/browser/prerender/prerender_manager_factory.cc
diff --git a/chrome/browser/prerender/prerender_manager_factory.cc b/chrome/browser/prerender/prerender_manager_factory.cc
index 5845dc43ef7f5641228bcd430b7e33f0406cf398..08b17ad7bd8e452142de916447a1a905c6b1bf14 100644
--- a/chrome/browser/prerender/prerender_manager_factory.cc
+++ b/chrome/browser/prerender/prerender_manager_factory.cc
@@ -15,8 +15,8 @@
 #include "components/browser_context_keyed_service/browser_context_dependency_manager.h"
 
 #if defined(OS_CHROMEOS)
-#include "chrome/browser/chromeos/cros/cros_library.h"
 #include "chrome/browser/chromeos/prerender_condition_network.h"
+#include "chromeos/network/network_handler.h"
 #endif
 
 namespace prerender {
@@ -55,11 +55,8 @@ BrowserContextKeyedService* PrerenderManagerFactory::BuildServiceInstanceFor(
   PrerenderManager* prerender_manager = new PrerenderManager(
       static_cast<Profile*>(profile), g_browser_process->prerender_tracker());
 #if defined(OS_CHROMEOS)
-  if (chromeos::CrosLibrary::Get()) {
-    prerender_manager->AddCondition(
-        new chromeos::PrerenderConditionNetwork(
-            chromeos::CrosLibrary::Get()->GetNetworkLibrary()));
-  }
+  if (chromeos::NetworkHandler::IsInitialized())

[cbentzel, 2013/06/03 16:51:16]

What's the initialization order here? I'm worried that if
PrerenderManagerFactory is initialized prior to NetworkHandler that prerender
requests for the entire browser session will go over cellular/metered
connections.

[stevenjb, 2013/06/03 17:14:54]

That's a good point. We really should:
a) Make it a CHECK instead of an if()
b) Move the CHECK to PrerenderConditionNetwork where we actually access
NetworkHandler.

If that breaks any tests, we should fix the tests to initialize NetworkHandler.

[gauravsh, 2013/06/03 17:20:49]

NetworkHandler is initialized at the same time as NetworkLibrary (as part of
DBus service initialization), and this happens before any profile-specific
initialization. Since PrerenderManagerFactory is a keyed service, it should be
getting initialized after NetworkHandler.

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...

[cbentzel, 2013/06/03 17:48:26]

Should this be a CHECK then?

[gauravsh, 2013/06/03 19:28:24]

I'd prefer to do this as a separate CL.

This particular pattern - if (SingletonClass::IsInitialized()) - is fairly
common in our codebase. The flipside to replacing conditionals is that now tests
need to have an additional dependency on NetworkHandler. I do think that a CHECK
makes more sense since it lets us catch initialization issues in production code
(currently, that results in silent failures).

I filed crbug.com/246309 so we can first reach consensus on the preferred way
(and under what specific conditions) of doing these checks. I can then make the
change across (at least for NetworkHandler) the entire codebase.

+    prerender_manager->AddCondition(new chromeos::PrerenderConditionNetwork);
 #endif
   return prerender_manager;
 }