Precache should cancel when there is user traffic

chrome/browser/precache/precache_util.cc

Index: chrome/browser/precache/precache_util.cc
diff --git a/chrome/browser/precache/precache_util.cc b/chrome/browser/precache/precache_util.cc
new file mode 100644
index 0000000000000000000000000000000000000000..9036e1af26b32211cfee33abcd28c1b3b220fbe5
--- /dev/null
+++ b/chrome/browser/precache/precache_util.cc
@@ -0,0 +1,62 @@
+// Copyright (c) 2016 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/precache/precache_util.h"
+
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/precache/precache_manager_factory.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "components/data_use_measurement/content/data_use_measurement.h"
+#include "components/precache/content/precache_manager.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace precache {
+
+void PrecacheUtil::UpdatePrecacheMetricsAndState(const net::URLRequest* request,

[bengr, 2016/06/20 20:21:14]

There should be unit tests for these methods.

[Raj, 2016/06/30 06:03:08]

I am trying to add tests. But looks little complicated to create a test profile
and test this.
BrowserTests have a way of testing profiles. But I believe, browsertests are not
supported in android. And testing this precache code in non-android does not
make sense.
I could add unittest for this separately from this CL. WDYT.

The functions used by this api have unittests, and this function is just a
wrapper.
 * RecordStatsForFetch() in precache_manager_unittest.cc
 * IsUserTraffic() in data_use_measurement_unittest.cc

[bengr, 2016/07/08 16:54:22]

If the functions this uses have unit tests, then just add a TODO to add more
test coverage.

[Raj, 2016/07/08 20:05:49]

Done.

+                                                 void* profile_id) {
+  // For better accuracy, we use the actual bytes read instead of the length
+  // specified with the Content-Length header, which may be inaccurate,
+  // or missing, as is the case with chunked encoding.
+  int64_t received_content_length = request->received_response_content_length();
+  base::TimeDelta latency = base::TimeTicks::Now() - request->creation_time();
+
+  // Record precache metrics when a fetch is completed successfully, if
+  // precaching is allowed.
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(
+          &PrecacheUtil::UpdatePrecacheMetricsAndStateOnUIThread,
+          request->url(), GURL(request->referrer()), latency, base::Time::Now(),
+          received_content_length, request->was_cached(),
+          data_use_measurement::DataUseMeasurement::IsUserTraffic(request),
+          profile_id));
+}
+
+void PrecacheUtil::UpdatePrecacheMetricsAndStateOnUIThread(

[bengr, 2016/06/20 20:21:14]

Can this method be in an anonymous namespace in this file?

[Raj, 2016/06/30 06:03:08]

Done.

+    const GURL& url,
+    const GURL& referrer,
+    base::TimeDelta latency,
+    const base::Time& fetch_time,
+    int64_t size,
+    bool was_cached,
+    bool is_user_traffic,
+    void* profile_id) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  if (!g_browser_process->profile_manager()->IsValidProfile(profile_id))
+    return;
+  Profile* profile = reinterpret_cast<Profile*>(profile_id);
+
+  PrecacheManager* precache_manager =
+      precache::PrecacheManagerFactory::GetForBrowserContext(profile);
+  // |precache_manager| could be NULL if the profile is off the record.
+  if (!precache_manager || !precache_manager->IsPrecachingAllowed())
+    return;
+
+  precache_manager->UpdatePrecacheMetricsAndState(
+      url, referrer, latency, fetch_time, size, was_cached, is_user_traffic);
+}
+
+}  // namespace precache