Precache tracking database

chrome/browser/net/chrome_network_delegate.cc

Index: chrome/browser/net/chrome_network_delegate.cc
diff --git a/chrome/browser/net/chrome_network_delegate.cc b/chrome/browser/net/chrome_network_delegate.cc
index 230b40b3fcfd613ffa05ba5603efa781be3e0df1..e8c5c6e414134d7d5f51d3655f8d0faeb1579486 100644
--- a/chrome/browser/net/chrome_network_delegate.cc
+++ b/chrome/browser/net/chrome_network_delegate.cc
@@ -38,6 +38,9 @@
 #include "chrome/browser/task_manager/task_manager.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
+#include "components/precache/content/precache_manager.h"
+#include "components/precache/content/precache_manager_factory.h"
+#include "components/precache/core/precache_database.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/resource_request_info.h"
@@ -299,6 +302,34 @@ void RecordContentLengthHistograms(
 #endif  // defined(OS_ANDROID)
 }
 
+void RecordPrecacheStatsOnUIThread(void* profile_id, const GURL& url,

[bengr, 2013/10/23 19:03:36]

Why is this a void*? Can it be a const Profile*?

[bengr, 2013/10/23 19:03:36]

To be explicit, I'd wrap this function in #if defined(OS_ANDROID), in addition
to wrapping the caller.

[sclittle, 2013/10/24 22:11:38]

It's a void* because that's what the ChromeNetworkDelegate has. Profiles should
only be used on the UI thread, so we wait until we're on the UI thread before we
cast it into a Profile*. The function NotifyEPMRequestStatus in this file does
it the same way.

We can't make it a const pointer since BrowserContext::GetRequestContext is a
non-const method, and the PrecacheManager needs to call that on the profile when
it constructs the PrecacheFetcher.

[sclittle, 2013/10/24 22:11:38]

Done.

+                                   const base::Time& fetch_time,
+                                   int64 received_content_length,
+                                   bool was_cached) {
+  if (!received_content_length || url.is_empty())
+    return;
+  Profile* profile = reinterpret_cast<Profile*>(profile_id);
+  if (!g_browser_process->profile_manager()->IsValidProfile(profile))
+    return;
+  precache::PrecacheManager* precache_manager =
+      precache::PrecacheManagerFactory::GetForBrowserContext(profile);
+  if (!precache_manager)

[bengr, 2013/10/23 19:03:36]

Can this ever happen? How?

[sclittle, 2013/10/24 22:11:38]

This will happen if the profile is incognito.

+    return;
+  scoped_refptr<precache::PrecacheDatabase> precache_database =
+      precache_manager->precache_database();
+  if (!precache_database)

[bengr, 2013/10/23 19:03:36]

Likewise.

[sclittle, 2013/10/24 22:11:38]

This can never happen. Removed if statement.

+    return;
+
+  bool is_cellular = net::NetworkChangeNotifier::IsConnectionCellular(

[bengr, 2013/10/23 19:03:36]

I wonder what fraction is labeled as CONNECTION_UNKNOWN. 

[sclittle, 2013/10/24 22:11:38]

Check out the NCN histograms, including NCN.CM.TimeOnUnknown, NCN.CM.TimeOn2G,
NCN.CM.TimeOn3G, and NCN.CM.TimeOn4G.

+      net::NetworkChangeNotifier::GetConnectionType());
+
+  BrowserThread::PostTask(
+      BrowserThread::DB, FROM_HERE,
+      base::Bind(&precache::PrecacheDatabase::RecordURLFetched,
+                 precache_database, url, fetch_time, received_content_length,
+                 was_cached, precache_manager->is_precaching(), is_cellular));
+}
+
 }  // namespace
 
 ChromeNetworkDelegate::ChromeNetworkDelegate(
@@ -515,6 +546,17 @@ void ChromeNetworkDelegate::OnCompleted(net::URLRequest* request,
     // or missing, as is the case with chunked encoding.
     int64 received_content_length = request->received_response_content_length();
 
+#if defined(OS_ANDROID)
+    if (precache::PrecacheManager::IsPrecachingEnabled()) {
+      // Record precache statistics for the fetch.
+      BrowserThread::PostTask(
+          BrowserThread::UI, FROM_HERE,
+          base::Bind(&RecordPrecacheStatsOnUIThread, profile_, request->url(),
+                     request->response_info().response_time,
+                     received_content_length, request->was_cached()));

[bengr, 2013/10/23 19:03:36]

In what cases does was_cached() return true? Is it true if revalidated and not
changed? Etc.

[sclittle, 2013/10/24 22:11:38]

was_cached() returns true if the response was served from the cache, including
304s. I've verified this manually.

+    }
+#endif  // defined(OS_ANDROID)
+
     // Only record for http or https urls.
     bool is_http = request->url().SchemeIs("http");
     bool is_https = request->url().SchemeIs("https");