Add pause/resume functionality to precache

components/precache/core/precache_fetcher.cc

Index: components/precache/core/precache_fetcher.cc
diff --git a/components/precache/core/precache_fetcher.cc b/components/precache/core/precache_fetcher.cc
index e7a5a0bce807a63ca3c8913f66924d154b331601..99fb6417b63bf354298de56eaf985bdbec7ae897 100644
--- a/components/precache/core/precache_fetcher.cc
+++ b/components/precache/core/precache_fetcher.cc
@@ -11,13 +11,17 @@
 #include <vector>
 
 #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 "components/precache/core/precache_database.h"
 #include "components/precache/core/precache_switches.h"
 #include "components/precache/core/proto/precache.pb.h"
 #include "net/base/completion_callback.h"
@@ -261,13 +265,17 @@ PrecacheFetcher::PrecacheFetcher(
     net::URLRequestContextGetter* request_context,
     const GURL& config_url,
     const std::string& manifest_url_prefix,
-    PrecacheFetcher::PrecacheDelegate* precache_delegate)
+    PrecacheFetcher::PrecacheDelegate* precache_delegate,
+    base::WeakPtr<PrecacheDatabase> precache_database,
+    scoped_refptr<base::SingleThreadTaskRunner> database_task_runner)
     : starting_hosts_(starting_hosts),
       request_context_(request_context),
       config_url_(config_url),
       manifest_url_prefix_(manifest_url_prefix),
       precache_delegate_(precache_delegate),
       config_(new PrecacheConfigurationSettings),
+      precache_database_(precache_database),
+      database_task_runner_(database_task_runner),
       total_response_bytes_(0),
       network_response_bytes_(0),
       num_manifest_urls_to_fetch_(0),
@@ -310,6 +318,57 @@ PrecacheFetcher::~PrecacheFetcher() {
                               100);
 }
 
+
+
+
+void PrecacheFetcher::Init(
+    const base::TimeDelta& max_age) {
+  // Fetch unfinished work that is no older than |max_age|.
+  scoped_refptr<UnfinishedWork> unfinished_work =
+      new UnfinishedWork(base::TimeTicks::Now() - max_age);
+  database_task_runner_->PostTaskAndReply(

[sclittle, 2016/05/10 00:01:26]

I'm not sure that UnfinishedWork needs to be refcounted or passed as a
scoped_refptr.

Could you just use PostTaskAndReplyWithResult, where the result is a
std::unique_ptr<UnfinishedWork>?
https://code.google.com/p/chromium/codesearch#chromium/src/base/task_runner_u...

[bengr, 2016/05/19 01:25:44]

Done.

+      FROM_HERE,
+      base::Bind(&PrecacheDatabase::GetUnfinishedWork, precache_database_,
+                 unfinished_work),
+      base::Bind(&PrecacheFetcher::OnInitDone, AsWeakPtr(),
+                 unfinished_work));
+}
+
+void PrecacheFetcher::OnInitDone(
+    scoped_refptr<UnfinishedWork> unfinished_work) {
+  if (unfinished_work->manifests.empty() &&
+      unfinished_work->resources.empty()) {
+    if (unfinished_work->total_bytes > 0) {
+      //TODO(rajendrant): report expired unfinished job.

[sclittle, 2016/05/10 00:01:26]

nit: add space between "//" and "TODO", and capitalize "report".

[bengr, 2016/05/19 01:25:44]

Done.

+    }
+  } else {
+    // Starting hosts are cleared because this is a continuation.
+    manifest_urls_to_fetch_ = unfinished_work->manifests;

[sclittle, 2016/05/10 00:01:26]

Use std::move on the lists here to avoid copying them all.

[bengr, 2016/05/19 01:25:44]

Done.

+    resource_urls_to_fetch_ = unfinished_work->resources;
+    total_response_bytes_ = unfinished_work->total_bytes;
+    network_response_bytes_ = unfinished_work->network_bytes;
+    num_manifest_urls_to_fetch_ = unfinished_work->num_manifest_urls;
+    start_time_ = unfinished_work->start_time;
+  }
+  precache_delegate_->OnInitDone();
+}
+
+void PrecacheFetcher::Shutdown() {
+  scoped_refptr<UnfinishedWork> unfinished_work = new UnfinishedWork(
+      manifest_urls_to_fetch_, resource_urls_to_fetch_,
+      total_response_bytes_, network_response_bytes_,
+      num_manifest_urls_to_fetch_, start_time_);
+  database_task_runner_->PostTaskAndReply(
+      FROM_HERE,
+      base::Bind(&PrecacheDatabase::SaveUnfinishedWork, precache_database_,
+                 unfinished_work),
+      base::Bind(&PrecacheFetcher::OnShutdownDone, AsWeakPtr()));
+}
+
+void PrecacheFetcher::OnShutdownDone() {
+  precache_delegate_->OnShutdownDone();
+}
+
 void PrecacheFetcher::Start() {
   GURL config_url =
       config_url_.is_empty() ? GetDefaultConfigURL() : config_url_;
@@ -317,7 +376,8 @@ void PrecacheFetcher::Start() {
   DCHECK(config_url.is_valid()) << "Config URL not valid: "
                                 << config_url.possibly_invalid_spec();
 
-  start_time_ = base::TimeTicks::Now();
+  if (start_time_.is_null())
+    start_time_ = base::TimeTicks::Now();
 
   // Fetch the precache configuration settings from the server.
   DCHECK(pool_.IsEmpty()) << "All parallel requests should be available";
@@ -403,20 +463,24 @@ void PrecacheFetcher::OnConfigFetchComplete(const Fetcher& source) {
 
     // 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.
-    int64_t rank = 0;
-    for (const std::string& host : starting_hosts_) {
-      ++rank;
-      if (rank > config_->top_sites_count())
-        break;
-      AppendManifestURLIfNew(prefix, host, &seen_manifest_urls,
-                             &manifest_urls_to_fetch_);
-    }
-
-    for (const std::string& host : config_->forced_site())
-      AppendManifestURLIfNew(prefix, host, &seen_manifest_urls,
-                             &manifest_urls_to_fetch_);
+    // 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()) {

[sclittle, 2016/05/10 00:01:26]

What about if the UnfinishedWork already finished fetching the manifests, and
only has resource URLs left? It would try to fetch all the manifests again here,
wouldn't it?

[bengr, 2016/05/19 01:25:44]

Good catch. Thanks.

+      int64_t rank = 0;
+      for (const std::string& host : starting_hosts_) {
+        ++rank;
+        if (rank > config_->top_sites_count())
+          break;
+        AppendManifestURLIfNew(prefix, host, &seen_manifest_urls,
+                               &manifest_urls_to_fetch_);
+      }
 
+      for (const std::string& host : config_->forced_site()) {
+        AppendManifestURLIfNew(prefix, host, &seen_manifest_urls,
+                               &manifest_urls_to_fetch_);
+      }
+    }
     num_manifest_urls_to_fetch_ = manifest_urls_to_fetch_.size();
   }
   pool_.Delete(source);
@@ -435,9 +499,8 @@ void PrecacheFetcher::OnManifestFetchComplete(const Fetcher& source) {
       const int32_t len =
           std::min(manifest.resource_size(), config_->top_resources_count());
       for (int i = 0; i < len; ++i) {
-        if (manifest.resource(i).has_url()) {
+        if (manifest.resource(i).has_url())
           resource_urls_to_fetch_.push_back(GURL(manifest.resource(i).url()));
-        }
       }
     }
   }
@@ -460,4 +523,20 @@ void PrecacheFetcher::UpdateStats(int64_t response_bytes,
   network_response_bytes_ += network_response_bytes;
 }
 
+void PrecacheFetcher::AssignWorkForTest(const std::list<GURL>& manifests,
+                                        const std::list<GURL>& resources,
+                                        size_t total_response_bytes,
+                                        size_t network_response_bytes,
+                                        int num_manifest_urls_to_fetch,
+                                        const base::TimeTicks& start_time) {
+  DCHECK(manifest_urls_to_fetch_.empty());
+  DCHECK(resource_urls_to_fetch_.empty());
+  manifest_urls_to_fetch_ = manifests;
+  resource_urls_to_fetch_ = resources;
+  total_response_bytes_ = total_response_bytes;
+  network_response_bytes_ = network_response_bytes;
+  num_manifest_urls_to_fetch_ = num_manifest_urls_to_fetch;
+  start_time_ = start_time;
+}
+
 }  // namespace precache