Send the list of used and unused resources for precache

components/precache/core/precache_fetcher.cc

 // Copyright 2013 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 "components/precache/core/precache_fetcher.h"
 
 #include <algorithm>
 #include <limits>
-#include <string>
 #include <utility>
 #include <vector>
 
+#include "base/base64.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/containers/hash_tables.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/sha1.h"
+#include "base/strings/string_piece.h"
+#include "base/task_runner_util.h"
+#include "components/precache/core/precache_database.h"
 #include "components/precache/core/precache_switches.h"
 #include "components/precache/core/proto/precache.pb.h"
 #include "components/precache/core/proto/unfinished_work.pb.h"
 #include "net/base/completion_callback.h"
 #include "net/base/escape.h"
 #include "net/base/io_buffer.h"
 #include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
+#include "net/base/url_util.h"
 #include "net/http/http_response_headers.h"
 #include "net/url_request/url_fetcher_response_writer.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "net/url_request/url_request_status.h"
 
 namespace precache {
 
 // The following flags are for privacy reasons. For example, if a user clears
 // their cookies, but a tracking beacon is prefetched and the beacon specifies
 // its source URL in a URL param, the beacon site would be able to rebuild a
@@ skipping 41 matching lines @@
 
 #if defined(PRECACHE_MANIFEST_URL_PREFIX)
   return PRECACHE_MANIFEST_URL_PREFIX;
 #else
   // The precache manifest URL prefix could not be determined, so return an
   // empty string.
   return std::string();
 #endif
 }
 
-// Construct the URL of the precache manifest for the given name (either host or
-// URL). The server is expecting a request for a URL consisting of the manifest
-// URL prefix followed by the doubly escaped name.
-std::string ConstructManifestURL(const std::string& prefix,
-                                 const std::string& name) {
-  return prefix + net::EscapeQueryParamValue(
-                      net::EscapeQueryParamValue(name, false), false);
-}
-
 // Attempts to parse a protobuf message from the response string of a
 // URLFetcher. If parsing is successful, the message parameter will contain the
 // parsed protobuf and this function will return true. Otherwise, returns false.
 bool ParseProtoFromFetchResponse(const net::URLFetcher& source,
                                  ::google::protobuf::MessageLite* message) {
   std::string response_string;
 
   if (!source.GetStatus().is_success()) {
     DLOG(WARNING) << "Fetch failed: " << source.GetOriginalURL().spec();
     return false;
@@ skipping 40 matching lines @@
             int num_bytes,
             const net::CompletionCallback& callback) override {
     return num_bytes;
   }
 
   int Finish(const net::CompletionCallback& callback) override {
     return net::OK;
   }
 };
 
-void AppendManifestURLIfValidAndNew(
-    const std::string& prefix,
-    const std::string& name,
-    base::hash_set<std::string>* seen_manifest_urls,
-    std::list<GURL>* unique_manifest_urls) {
-  const std::string manifest_url = ConstructManifestURL(prefix, name);
-  bool first_seen = seen_manifest_urls->insert(manifest_url).second;
-  if (first_seen) {
-    GURL url(manifest_url);
-    if (url.is_valid())
-      unique_manifest_urls->push_back(url);
+// Returns the base64 encoded resource URL hashes. The resource URLs are hashed
+// individually, and 8 bytes of each hash is appended together, which is then
+// encoded to base64.
+std::string GetResourceURLBase64Hash(const std::vector<GURL>& urls) {
+  // Each resource hash uses 8 bytes.
+  const int hash_bytes_size = 8;

[sclittle, 2016/08/15 20:13:09]

nit: Change this to be a size_t instead of an int, since it represents some
number of bytes in memory, and name this like a constant, e.g. kHashBytesSize.

[Raj, 2016/08/16 17:54:37]

Done.

+  std::string hashes;
+  hashes.reserve(urls.size() * hash_bytes_size);
+
+  for (const auto& url : urls) {
+    const std::string& url_spec = url.spec();
+    unsigned char sha1_hash[base::kSHA1Length];
+    base::SHA1HashBytes(
+        reinterpret_cast<const unsigned char*>(url_spec.c_str()),
+        url_spec.size(), sha1_hash);
+    hashes.append(reinterpret_cast<const char*>(sha1_hash), hash_bytes_size);
   }
+  base::Base64Encode(hashes, &hashes);
+  return hashes;
 }
 
 }  // namespace
 
 PrecacheFetcher::Fetcher::Fetcher(
     net::URLRequestContextGetter* request_context,
     const GURL& url,
+    const std::string& referrer,
     const base::Callback<void(const Fetcher&)>& callback,
     bool is_resource_request,
     size_t max_bytes)
     : request_context_(request_context),
       url_(url),
+      referrer_(referrer),
       callback_(callback),
       is_resource_request_(is_resource_request),
       max_bytes_(max_bytes),
       response_bytes_(0),
-      network_response_bytes_(0) {
+      network_response_bytes_(0),
+      was_cached_(false) {
+  DCHECK(url.is_valid());
   if (is_resource_request_)
     LoadFromCache();
   else
     LoadFromNetwork();
 }
 
 PrecacheFetcher::Fetcher::~Fetcher() {}
 
 void PrecacheFetcher::Fetcher::LoadFromCache() {
   fetch_stage_ = FetchStage::CACHE;
@@ skipping 32 matching lines @@
     const net::URLFetcher* source,
     int64_t current,
     int64_t total) {
   // If going over the per-resource download cap.
   if (fetch_stage_ == FetchStage::NETWORK &&
       // |current| is guaranteed to be non-negative, so this cast is safe.
       static_cast<size_t>(std::max(current, total)) > max_bytes_) {
     VLOG(1) << "Cancelling " << url_ << ": (" << current << "/" << total
             << ") is over " << max_bytes_;
 
-    // Cancel the download.
-    network_url_fetcher_.reset();
-
     // Call the completion callback, to attempt the next download, or to trigger
     // cleanup in precache_delegate_->OnDone().
     response_bytes_ = network_response_bytes_ = current;
+    was_cached_ = source->WasCached();
 
+    // Cancel the download.
+    network_url_fetcher_.reset();
     callback_.Run(*this);
   }
 }
 
 void PrecacheFetcher::Fetcher::OnURLFetchComplete(
     const net::URLFetcher* source) {
   CHECK(source);
   if (fetch_stage_ == FetchStage::CACHE &&
       (source->GetStatus().error() == net::ERR_CACHE_MISS ||
        (source->GetResponseHeaders() &&
         source->GetResponseHeaders()->HasValidators()))) {
     // If the resource was not found in the cache, request it from the
     // network.
     //
     // If the resource was found in the cache, but contains validators,
     // request a refresh. The presence of validators increases the chance that
     // we get a 304 response rather than a full one, thus allowing us to
     // refresh the cache with minimal network load.
     LoadFromNetwork();
     return;
   }
 
   // If any of:
   // - The request was for a config or manifest.
   // - The resource was a cache hit without validators.
   // - The response came from the network.
   // Then Fetcher is done with this URL and can return control to the caller.
   response_bytes_ = source->GetReceivedResponseContentLength();
   network_response_bytes_ = source->GetTotalReceivedBytes();
+  was_cached_ = source->WasCached();
   callback_.Run(*this);
 }
 
 // static
 void PrecacheFetcher::RecordCompletionStatistics(
     const PrecacheUnfinishedWork& unfinished_work,
     size_t remaining_manifest_urls_to_fetch,
     size_t remaining_resource_urls_to_fetch) {
   // These may be unset in tests.
   if (!unfinished_work.has_start_time())
@@ skipping 30 matching lines @@
                               1, kMaxResponseBytes,
                               100);
 }
 
 PrecacheFetcher::PrecacheFetcher(
     net::URLRequestContextGetter* request_context,
     const GURL& config_url,
     const std::string& manifest_url_prefix,
     std::unique_ptr<PrecacheUnfinishedWork> unfinished_work,
     uint32_t experiment_id,
+    const base::WeakPtr<PrecacheDatabase>& precache_database,
+    const scoped_refptr<base::SingleThreadTaskRunner>& db_task_runner,
     PrecacheFetcher::PrecacheDelegate* precache_delegate)
     : request_context_(request_context),
       config_url_(config_url),
       manifest_url_prefix_(manifest_url_prefix),
+      precache_database_(precache_database),
+      db_task_runner_(std::move(db_task_runner)),
       precache_delegate_(precache_delegate),
       pool_(kMaxParallelFetches),
       experiment_id_(experiment_id) {
   DCHECK(request_context_.get());  // Request context must be non-NULL.
   DCHECK(precache_delegate_);  // Precache delegate must be non-NULL.
 
   DCHECK_NE(GURL(), GetDefaultConfigURL())
       << "Could not determine the precache config settings URL.";
   DCHECK_NE(std::string(), GetDefaultManifestURLPrefix())
       << "Could not determine the default precache manifest URL prefix.";
   DCHECK(unfinished_work);
 
-  // Copy manifests and resources to member variables as a convenience.
-  // TODO(bengr): Consider accessing these directly from the proto.
-  for (const auto& manifest : unfinished_work->manifest()) {
-    if (manifest.has_url())
-      manifest_urls_to_fetch_.push_back(GURL(manifest.url()));
-  }
+  // Copy resources to member variable as a convenience.
+  // TODO(rajendrant): Consider accessing these directly from the proto, by
+  // keeping track of the current resource index.
   for (const auto& resource : unfinished_work->resource()) {
-    if (resource.has_url())
-      resource_urls_to_fetch_.push_back(GURL(resource.url()));
+    if (resource.has_url() && resource.has_top_host_name()) {
+      resources_to_fetch_.emplace_back(GURL(resource.url()),
+                                       resource.top_host_name());
+    }
   }
   unfinished_work_ = std::move(unfinished_work);
 }
 
 PrecacheFetcher::~PrecacheFetcher() {
 }
 
 std::unique_ptr<PrecacheUnfinishedWork> PrecacheFetcher::CancelPrecaching() {
   // This could get called multiple times, and it should be handled gracefully.
   if (!unfinished_work_)
     return nullptr;
 
-  unfinished_work_->clear_manifest();
   unfinished_work_->clear_resource();
-  for (const auto& manifest : manifest_urls_to_fetch_)
-    unfinished_work_->add_manifest()->set_url(manifest.spec());
-  for (const auto& resource : resource_urls_to_fetch_)
-    unfinished_work_->add_resource()->set_url(resource.spec());
+  if (top_hosts_to_fetch_) {
+    unfinished_work_->clear_top_host();
+    for (const auto& top_host : *top_hosts_to_fetch_) {
+      unfinished_work_->add_top_host()->set_hostname(top_host.hostname);
+    }
+  }
+  for (const auto& resource : resources_to_fetch_) {
+    auto new_resource = unfinished_work_->add_resource();
+    new_resource->set_url(resource.first.spec());
+    new_resource->set_top_host_name(resource.second);
+  }
   for (const auto& it : pool_.elements()) {
     const Fetcher* fetcher = it.first;
-    if (fetcher->is_resource_request())
-      unfinished_work_->add_resource()->set_url(fetcher->url().spec());
-    else if (fetcher->url() != config_url_)
-      unfinished_work_->add_manifest()->set_url(fetcher->url().spec());
+    GURL config_url =
+        config_url_.is_empty() ? GetDefaultConfigURL() : config_url_;
+    if (fetcher->is_resource_request()) {
+      auto resource = unfinished_work_->add_resource();
+      resource->set_url(fetcher->url().spec());
+      resource->set_top_host_name(fetcher->referrer());
+    } else if (fetcher->url() != config_url) {
+      unfinished_work_->add_top_host()->set_hostname(fetcher->referrer());
+    }
   }
-  manifest_urls_to_fetch_.clear();
-  resource_urls_to_fetch_.clear();
+  top_hosts_to_fetch_.reset();
+  resources_to_fetch_.clear();
   pool_.DeleteAll();
   return std::move(unfinished_work_);
 }
 
 void PrecacheFetcher::Start() {
   if (unfinished_work_->has_config_settings()) {
     DCHECK(unfinished_work_->has_start_time());
     DetermineManifests();
     return;
   }
 
   GURL config_url =
       config_url_.is_empty() ? GetDefaultConfigURL() : config_url_;
 
   DCHECK(config_url.is_valid()) << "Config URL not valid: "
                                 << config_url.possibly_invalid_spec();
 
   // Fetch the precache configuration settings from the server.
   DCHECK(pool_.IsEmpty()) << "All parallel requests should be available";
   VLOG(3) << "Fetching " << config_url;
   pool_.Add(base::WrapUnique(new Fetcher(
-      request_context_.get(), config_url,
-      base::Bind(&PrecacheFetcher::OnConfigFetchComplete,
-                 base::Unretained(this)),
+      request_context_.get(), config_url, std::string(),
+      base::Bind(&PrecacheFetcher::OnConfigFetchComplete, AsWeakPtr()),
       false /* is_resource_request */, std::numeric_limits<int32_t>::max())));
 }
 
 void PrecacheFetcher::StartNextResourceFetch() {
   DCHECK(unfinished_work_->has_config_settings());
-  while (!resource_urls_to_fetch_.empty() && pool_.IsAvailable()) {
+  while (!resources_to_fetch_.empty() && pool_.IsAvailable()) {
+    const auto& resource = resources_to_fetch_.front();
     const size_t max_bytes =
         std::min(unfinished_work_->config_settings().max_bytes_per_resource(),
                  unfinished_work_->config_settings().max_bytes_total() -
                      unfinished_work_->total_bytes());
-    VLOG(3) << "Fetching " << resource_urls_to_fetch_.front();
-    pool_.Add(base::WrapUnique(
-        new Fetcher(request_context_.get(), resource_urls_to_fetch_.front(),
-                    base::Bind(&PrecacheFetcher::OnResourceFetchComplete,
-                               base::Unretained(this)),
-                    true /* is_resource_request */, max_bytes)));
+    VLOG(3) << "Fetching " << resource.first << " " << resource.second;
+    pool_.Add(base::WrapUnique(new Fetcher(
+        request_context_.get(), resource.first, resource.second,
+        base::Bind(&PrecacheFetcher::OnResourceFetchComplete, AsWeakPtr()),
+        true /* is_resource_request */, max_bytes)));
 
-    resource_urls_to_fetch_.pop_front();
+    resources_to_fetch_.pop_front();
   }
 }
 
 void PrecacheFetcher::StartNextManifestFetch() {
-  if (manifest_urls_to_fetch_.empty() || !pool_.IsAvailable())
+  if (!top_hosts_to_fetch_ || top_hosts_to_fetch_->empty() ||
+      !pool_.IsAvailable())
     return;
 
   // We only fetch one manifest at a time to keep the size of
-  // resource_urls_to_fetch_ as small as possible.
-  VLOG(3) << "Fetching " << manifest_urls_to_fetch_.front();
+  // resources_to_fetch_ as small as possible.
+  VLOG(3) << "Fetching " << top_hosts_to_fetch_->front().manifest_url;
   pool_.Add(base::WrapUnique(new Fetcher(
-      request_context_.get(), manifest_urls_to_fetch_.front(),
-      base::Bind(&PrecacheFetcher::OnManifestFetchComplete,
-                 base::Unretained(this)),
+      request_context_.get(), top_hosts_to_fetch_->front().manifest_url,
+      top_hosts_to_fetch_->front().hostname,
+      base::Bind(&PrecacheFetcher::OnManifestFetchComplete, AsWeakPtr()),
       false /* is_resource_request */, std::numeric_limits<int32_t>::max())));
-
-  manifest_urls_to_fetch_.pop_front();
+  top_hosts_to_fetch_->pop_front();
 }
 
 void PrecacheFetcher::NotifyDone(
     size_t remaining_manifest_urls_to_fetch,
     size_t remaining_resource_urls_to_fetch) {
   RecordCompletionStatistics(*unfinished_work_,
                              remaining_manifest_urls_to_fetch,
                              remaining_resource_urls_to_fetch);
   precache_delegate_->OnDone();
 }
 
 void PrecacheFetcher::StartNextFetch() {
   DCHECK(unfinished_work_->has_config_settings());
   // If over the precache total size cap, then stop prefetching.
   if (unfinished_work_->total_bytes() >
       unfinished_work_->config_settings().max_bytes_total()) {
     size_t pending_manifests_in_pool = 0;
     size_t pending_resources_in_pool = 0;
     for (const auto& element_pair : pool_.elements()) {
       const Fetcher* fetcher = element_pair.first;
       if (fetcher->is_resource_request())
         pending_resources_in_pool++;
       else if (fetcher->url() != config_url_)
         pending_manifests_in_pool++;
     }
     pool_.DeleteAll();
-    NotifyDone(manifest_urls_to_fetch_.size() + pending_manifests_in_pool,
-               resource_urls_to_fetch_.size() + pending_resources_in_pool);
+    int pending_top_hosts =
+        top_hosts_to_fetch_ ? top_hosts_to_fetch_->size() : 0;
+    NotifyDone(pending_top_hosts + pending_manifests_in_pool,
+               resources_to_fetch_.size() + pending_resources_in_pool);
     return;
   }
 
   StartNextResourceFetch();
   StartNextManifestFetch();
-  if (pool_.IsEmpty()) {
+  if ((!top_hosts_to_fetch_ || top_hosts_to_fetch_->empty()) &&
+      resources_to_fetch_.empty() && pool_.IsEmpty()) {
     // There are no more URLs to fetch, so end the precache cycle.
     NotifyDone(0, 0);
     // OnDone may have deleted this PrecacheFetcher, so don't do anything after
     // it is called.
   }
 }
 
 void PrecacheFetcher::OnConfigFetchComplete(const Fetcher& source) {
   UpdateStats(source.response_bytes(), source.network_response_bytes());
   if (source.network_url_fetcher() == nullptr) {
     pool_.DeleteAll();  // Cancel any other ongoing request.
   } else {
     // Attempt to parse the config proto. On failure, continue on with the
     // default configuration.
     ParseProtoFromFetchResponse(
         *source.network_url_fetcher(),
         unfinished_work_->mutable_config_settings());
     pool_.Delete(source);
     DetermineManifests();
   }
 }
 
 void PrecacheFetcher::DetermineManifests() {
   DCHECK(unfinished_work_->has_config_settings());
-    std::string prefix = manifest_url_prefix_.empty()
-                             ? GetDefaultManifestURLPrefix()
-                             : manifest_url_prefix_;
-    DCHECK_NE(std::string(), prefix)
-        << "Could not determine the precache manifest URL prefix.";
 
+  std::deque<std::string> top_hosts_to_fetch;

[sclittle, 2016/08/16 21:05:05]

nit: Why is this a std::deque? Could this just be an std::vector?

[Raj, 2016/08/18 00:23:23]

Done.

+  std::unique_ptr<std::deque<ManifestHostInfo>> top_hosts_info(

[sclittle, 2016/08/15 20:13:09]

nit: I think you can still remove this std::unique_ptr and not need this
variable at all if you use PostTaskAndReplyWithResult:
https://cs.chromium.org/chromium/src/base/task_runner_util.h?sq=package:chromium

If you change RetrieveManifestInfo to return the deque instead of setting it to
a pointer, and you change OnManifestInfoRetrieved to take in just a
"std::deque<ManifestHostInfo> manifests_info" instead of a unique_ptr, then you
could do it like this:

base::PostTaskAndReplyWithResult(db_task_runner_.get(), FROM_HERE,
    base::Bind(&PrecacheFetcher::RetrieveManifestInfo, AsWeakPtr(),
std::move(top_hosts_to_fetch)),
    base::Bind(&PrecacheFetcher::OnManifestInfoRetrieved, AsWeakPtr());

[Raj, 2016/08/16 17:54:37]

Tried this earlier. PostTaskAndReplyWithResult() does not take weak_ptr, and
fails with compile-time assert.
../../base/bind_internal.h:293:3: error: static assertion failed: weak_ptrs can
only bind to methods without return values

https://cs.chromium.org/chromium/src/base/bind_internal.h?rcl=0&l=293


Also, note my current code has base::Unretained(this) for the reply callback.
base::Bind(&PrecacheFetcher::OnManifestInfoRetrieved, base::Unretained(this),
...)
Using AsWeakPtr() did not work, since the weakptr will be checked in DB thread
(while the weakptr was created in UI thread).
Following is the runtime DCHECK hit.
08-16 10:30:22.624 29153-29153 A: Abort message: '[FATAL:weak_ptr.cc(26)] Check
failed: sequence_checker_.CalledOnValidSequence(). WeakPtrs must be checked on
the same sequenced thread.

[sclittle, 2016/08/16 21:05:05]

Darn, good catch though.

+      new std::deque<ManifestHostInfo>);
+
+  // Attempt to fetch manifests for starting hosts up to the maximum top sites
+  // count. If a manifest does not exist for a particular starting host, then
+  // the fetch will fail, and that starting host will be ignored. Starting
+  // hosts are not added if this is a continuation from a previous precache
+  // session.
+  if (resources_to_fetch_.empty()) {
     // Keep track of manifest URLs that are being fetched, in order to elide
     // duplicates.
-    base::hash_set<std::string> seen_manifest_urls;
+    std::set<base::StringPiece> seen_top_hosts;
+    int64_t rank = 0;
+    for (const auto& host : unfinished_work_->top_host()) {
+      ++rank;
+      if (rank > unfinished_work_->config_settings().top_sites_count())
+        break;
+      if (seen_top_hosts.insert(host.hostname()).second)
+        top_hosts_to_fetch.push_back(host.hostname());
+    }
 
-    // Attempt to fetch manifests for starting hosts up to the maximum top sites
-    // count. If a manifest does not exist for a particular starting host, then
-    // the fetch will fail, and that starting host will be ignored. Starting
-    // hosts are not added if this is a continuation from a previous precache
-    // session.
-    if (manifest_urls_to_fetch_.empty() &&
-        resource_urls_to_fetch_.empty()) {
-      int64_t rank = 0;
-      for (const auto& host : unfinished_work_->top_host()) {
-        ++rank;
-        if (rank > unfinished_work_->config_settings().top_sites_count())
-          break;
-        AppendManifestURLIfValidAndNew(prefix, host.hostname(),
-                                       &seen_manifest_urls,
-                                       &manifest_urls_to_fetch_);
-      }
+    for (const std::string& host :
+         unfinished_work_->config_settings().forced_site()) {
+      if (seen_top_hosts.insert(host).second)
+        top_hosts_to_fetch.push_back(host);
+    }
+  }
+  // We only fetch one manifest at a time to keep the size of
+  // resources_to_fetch_ as small as possible.
+  auto retrieve_manifest_callback =
+      base::Bind(&PrecacheFetcher::RetrieveManifestInfo, AsWeakPtr(),
+                 std::move(top_hosts_to_fetch), top_hosts_info.get());
+  db_task_runner_->PostTaskAndReply(
+      FROM_HERE, retrieve_manifest_callback,
+      base::Bind(&PrecacheFetcher::OnManifestInfoRetrieved,
+                 base::Unretained(this), base::Passed(&top_hosts_info)));

[sclittle, 2016/08/16 21:05:05]

using base::Unretained(this) here doesn't seem safe, could you make this use
AsWeakPtr instead?

[Raj, 2016/08/18 00:23:23]

Done.

+}
 
-      for (const std::string& host
-          : unfinished_work_->config_settings().forced_site()) {
-        AppendManifestURLIfValidAndNew(prefix, host, &seen_manifest_urls,
-                                       &manifest_urls_to_fetch_);
-      }
+void PrecacheFetcher::RetrieveManifestInfo(

[sclittle, 2016/08/16 21:05:05]

Maybe move this method to the PrecacheDatabase instead of the PrecacheFetcher?
The PrecacheDatabase lives on the DB thread, so the lifetimes should work out
OK.

[Raj, 2016/08/18 00:23:23]

Moved it to an anonymous function.

Moving to PrecacheDatabase means, this function will return
vector<PrecacheReferrerHostInfo> where each PrecacheReferrerHostInfo contains
two vectors used_urls, unused_urls.

Another option is to return used_url_hash, unused_url_hash, which means moving
the hash computation logic to databse, which is not appropriate.

[sclittle, 2016/08/24 02:39:44]

Ok, sounds good. Thanks for investigating this.

+    std::deque<std::string> hosts_to_fetch,
+    std::deque<ManifestHostInfo>* hosts_info) {
+  for (const auto& host : hosts_to_fetch) {
+    auto referrer_host_info = precache_database_->GetReferrerHost(host);
+    if (referrer_host_info.id != PrecacheReferrerHostEntry::kInvalidId) {
+      std::vector<GURL> used_urls, unused_urls;
+      precache_database_->GetURLListForReferrerHost(referrer_host_info.id,
+                                                    &used_urls, &unused_urls);
+      hosts_info->emplace_back(referrer_host_info.id, host,

[sclittle, 2016/08/15 20:13:09]

nit: use push_back instead of emplace_back, since that'll use the move
constructor anyways, here and elsewhere.

[Raj, 2016/08/16 17:54:37]

I have changed it.
But emplace_back seems to be better though. It explicitly indicates there is
less copy (even if move is not supported).

It is less code.
hosts_info->emplace_back(...)
hosts_info->push_back(ManifestHostInfo(...))

[sclittle, 2016/08/16 21:05:05]

Yep, there's slightly less copy, but the extra copy in this case is just a few
pointers since it's done as a move operation (and the GURL in the struct is
empty and thus is cheap to copy), so it's not really a big deal. Whichever one
you prefer more is fine, but I'd lean toward using push_back unless there's a
good reason to use emplace_back.

+                               GetResourceURLBase64Hash(used_urls),
+                               GetResourceURLBase64Hash(unused_urls));
+    } else {
+      hosts_info->emplace_back(PrecacheReferrerHostEntry::kInvalidId, host,
+                               std::string(), std::string());
     }
-    unfinished_work_->set_num_manifest_urls(manifest_urls_to_fetch_.size());
-    StartNextFetch();
+  }
 }
 
+void PrecacheFetcher::OnManifestInfoRetrieved(
+    std::unique_ptr<std::deque<ManifestHostInfo>> manifests_info) {
+  DCHECK(manifests_info);
+  const std::string prefix = manifest_url_prefix_.empty()
+                                 ? GetDefaultManifestURLPrefix()
+                                 : manifest_url_prefix_;
+  top_hosts_to_fetch_.reset(new std::deque<ManifestHostInfo>());

[sclittle, 2016/08/16 21:05:05]

Could you just move the existing deque instead of constructing a whole new one
here? e.g. top_hosts_to_fetch_ = std::move(manifests_info);

Then, you could use std::remove_if to remove the invalid IDs from the deque in
O(n) time without needing to copy the whole deque, and before doing any work to
construct the manifest_urls, e.g.:

top_hosts_to_fetch_->erase(
      std::remove_if(top_hosts_to_fetch_->begin(),
                     top_hosts_to_fetch_->end(),
                     [](const ManifestHostInfo& manifest) {
                       return manifest.manifest_id ==
                              PrecacheReferrerHostEntry::kInvalidId;
                     }),
      top_hosts_to_fetch_->end());

[Raj, 2016/08/18 00:23:23]

Done.

+  for (auto& manifest : *manifests_info) {
+    GURL manifest_url(
+        prefix +
+        net::EscapeQueryParamValue(
+            net::EscapeQueryParamValue(manifest.hostname, false), false));
+    if (manifest_url.is_valid() &&

[sclittle, 2016/08/16 21:05:05]

It seems like the manifest_url could only be invalid if the |prefix| is an
invalid URL.

You should still make sure that the |prefix| is a valid URL, since it could be
set by Finch and that shouldn't cause Chrome to crash/perform weirdly if it's
set wrong accidentally. You could remove the is_valid() condition here and just
have an escape hatch above before the for loop, e.g.:

if (!GURL(prefix).is_valid()) {
  // Don't attempt to fetch any manifests if the manifest URL prefix
  // is invalid.
  top_hosts_to_fetch_.clear();
  return;
}

[Raj, 2016/08/18 00:23:23]

Done.

+        manifest.manifest_id != PrecacheReferrerHostEntry::kInvalidId) {
+      manifest_url = net::AppendOrReplaceQueryParameter(
+          manifest_url, "manifest", std::to_string(manifest.manifest_id));
+      manifest_url = net::AppendOrReplaceQueryParameter(
+          manifest_url, "used_resources", manifest.used_url_hash);
+      manifest_url = net::AppendOrReplaceQueryParameter(
+          manifest_url, "unused_resources", manifest.unused_url_hash);
+      DCHECK(manifest_url.is_valid());
+    }
+    manifest.manifest_url = manifest_url;
+    if (manifest_url.is_valid())
+      top_hosts_to_fetch_->push_back(std::move(manifest));
+  }
+  unfinished_work_->set_num_manifest_urls(top_hosts_to_fetch_->size());
+  StartNextFetch();
+}
+
+ManifestHostInfo::ManifestHostInfo(int64_t manifest_id,
+                                   const std::string& hostname,
+                                   const std::string& used_url_hash,
+                                   const std::string& unused_url_hash)
+    : manifest_id(manifest_id),
+      hostname(hostname),
+      used_url_hash(used_url_hash),
+      unused_url_hash(unused_url_hash) {}
+
+ManifestHostInfo::~ManifestHostInfo() {}
+
+ManifestHostInfo::ManifestHostInfo(ManifestHostInfo&&) = default;
+
+ManifestHostInfo& ManifestHostInfo::operator=(ManifestHostInfo&&) = default;
+
 void PrecacheFetcher::OnManifestFetchComplete(const Fetcher& source) {
   DCHECK(unfinished_work_->has_config_settings());
   UpdateStats(source.response_bytes(), source.network_response_bytes());
   if (source.network_url_fetcher() == nullptr) {
     pool_.DeleteAll();  // Cancel any other ongoing request.
   } else {
     PrecacheManifest manifest;
 
     if (ParseProtoFromFetchResponse(*source.network_url_fetcher(), &manifest)) {
       const int32_t len =
           std::min(manifest.resource_size(),
                    unfinished_work_->config_settings().top_resources_count());
       const uint64_t resource_bitset =
           GetResourceBitset(manifest, experiment_id_);
       for (int i = 0; i < len; ++i) {
         if (((0x1ULL << i) & resource_bitset) &&
             manifest.resource(i).has_url()) {
           GURL url(manifest.resource(i).url());
-          if (url.is_valid())
-            resource_urls_to_fetch_.push_back(url);
+          if (url.is_valid()) {
+            resources_to_fetch_.emplace_back(url, source.referrer());
+          }
         }
       }
+      db_task_runner_->PostTask(
+          FROM_HERE,
+          base::Bind(&PrecacheDatabase::UpdatePrecacheReferrerHost,
+                     precache_database_, source.referrer(),
+                     manifest.id().timestamp().seconds(), base::Time::Now()));
     }
   }
 
   pool_.Delete(source);
   StartNextFetch();
 }
 
 void PrecacheFetcher::OnResourceFetchComplete(const Fetcher& source) {
   UpdateStats(source.response_bytes(), source.network_response_bytes());
+
+  db_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&PrecacheDatabase::RecordURLPrefetch, precache_database_,
+                 source.url(), source.referrer(), base::Time::Now(),
+                 source.was_cached(), source.response_bytes()));
+
   pool_.Delete(source);
+
   // The resource has already been put in the cache during the fetch process, so
   // nothing more needs to be done for the resource.
   StartNextFetch();
 }
 
 void PrecacheFetcher::UpdateStats(int64_t response_bytes,
                                   int64_t network_response_bytes) {
   unfinished_work_->set_total_bytes(
       unfinished_work_->total_bytes() + response_bytes);
   unfinished_work_->set_network_bytes(
       unfinished_work_->network_bytes() + network_response_bytes);
 }
 
 }  // namespace precache