Update prerender policy for custom tabs

chrome/browser/prerender/prerender_manager.cc

Index: chrome/browser/prerender/prerender_manager.cc
diff --git a/chrome/browser/prerender/prerender_manager.cc b/chrome/browser/prerender/prerender_manager.cc
index 4bb07b23169a97432e1a9cb06d1239f8751ae0d1..d36e43a46937b3cc4c9c2c461a079abf1e512227 100644
--- a/chrome/browser/prerender/prerender_manager.cc
+++ b/chrome/browser/prerender/prerender_manager.cc
@@ -297,9 +297,12 @@ PrerenderHandle* PrerenderManager::AddPrerenderFromExternalRequest(
     const GURL& url,
     const content::Referrer& referrer,
     SessionStorageNamespace* session_storage_namespace,
-    const gfx::Size& size) {
-  return AddPrerender(ORIGIN_EXTERNAL_REQUEST, url, referrer, size,
-                      session_storage_namespace);
+    const gfx::Size& size,
+    bool prerender_on_mobile) {

[mmenke, 2016/03/14 20:38:01]

This is weird, for a couple reasons:

* This method is only used on mobile.
* Presumably we know at build time if we're on mobile.
* It can be false on mobile.

I also am not a fan of bool arguments passed in by value - "false" just sin't
too meaningful.

I suggest either we make this an enum class within PrerenderManager (And rename
it - prerender_on_cellular?) or make a new method
AddPrerenderOnCellularFromExternalRequest (Or somesuch)

[Yusuf, 2016/03/14 23:08:19]

Done. Added AddPrerenderOnCellularFromExternalRequest and renamed all relevant
calls and variables to say cellular instead of mobile. Removed custom tabs
mentions.

+  Origin origin = prerender_on_mobile
+      ? ORIGIN_EXTERNAL_REQUEST_FORCED_CUSTOMTABS

[mmenke, 2016/03/14 20:38:01]

Mapping "prerender_on_mobile == true" to
"ORIGIN_EXTERNAL_REQUEST_FORCED_CUSTOMTABS" seems weird - PrerenderManager
shouldn't know that only "CUSTOMTABS" (?) requests prerender on cellular
connections.

[Yusuf, 2016/03/14 23:08:20]

Done.

+      : ORIGIN_EXTERNAL_REQUEST;
+  return AddPrerender(origin, url, referrer, size, session_storage_namespace);
 }
 
 PrerenderHandle* PrerenderManager::AddPrerenderForInstant(
@@ -934,7 +937,8 @@ PrerenderHandle* PrerenderManager::AddPrerender(
   // histogram tracking.
   histograms_->RecordPrerender(origin, url_arg);
 
-  NetworkPredictionStatus prerendering_status = GetPredictionStatus();
+  NetworkPredictionStatus prerendering_status =
+      GetPredictionStatusForOrigin(origin);
   if (prerendering_status != NetworkPredictionStatus::ENABLED) {
     FinalStatus final_status =
         prerendering_status == NetworkPredictionStatus::DISABLED_DUE_TO_NETWORK
@@ -1285,6 +1289,16 @@ NetworkPredictionStatus PrerenderManager::GetPredictionStatus() const {
   return CanPrefetchAndPrerenderUI(profile_->GetPrefs());
 }
 
+NetworkPredictionStatus PrerenderManager::GetPredictionStatusForOrigin(
+    Origin origin) const {
+  NetworkPredictionStatus prediction_status = GetPredictionStatus();
+  if (prediction_status == NetworkPredictionStatus::DISABLED_DUE_TO_NETWORK
+      && origin == ORIGIN_EXTERNAL_REQUEST_FORCED_CUSTOMTABS) {

[mmenke, 2016/03/14 20:38:01]

Do we really want to do this?  How large are AMP pages?  I'm concerned both
about the performance cost and the bandwidth, particularly in emerging markets,
where people are often on 2G connections, and data expenses make up a
significant portion of people's income.

[Yusuf, 2016/03/14 23:08:20]

AMP in general is pretty cheap.
On our end, we never call the external request call on low end phones and also
heavily throttle calls that doesn't end up following with a corresponding intent
launch and ban apps from using this codepath that repeatedly get caught doing
this. 

+    prediction_status = NetworkPredictionStatus::ENABLED;
+  }
+  return prediction_status;
+}
+
 void PrerenderManager::AddProfileNetworkBytesIfEnabled(int64_t bytes) {
   DCHECK_GE(bytes, 0);
   if (GetPredictionStatus() == NetworkPredictionStatus::ENABLED &&