Add pause/resume functionality to precache

components/precache/core/precache_fetcher.h

Index: components/precache/core/precache_fetcher.h
diff --git a/components/precache/core/precache_fetcher.h b/components/precache/core/precache_fetcher.h
index 324ab08fa474c727cafd5a9c55bd2ebe3a5955ce..b5b572d6244bc9821ab412c008cc7b2258120e5a 100644
--- a/components/precache/core/precache_fetcher.h
+++ b/components/precache/core/precache_fetcher.h
@@ -15,13 +15,18 @@
 
 #include "base/callback.h"
 #include "base/macros.h"
-#include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
+#include "base/single_thread_task_runner.h"
 #include "base/time/time.h"
 #include "components/precache/core/fetcher_pool.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_fetcher_delegate.h"
 #include "url/gurl.h"
 
+namespace base {
+class SingleThreadTaskRunner;
+}
+
 namespace net {
 class URLRequestContextGetter;
 }
@@ -29,6 +34,8 @@ class URLRequestContextGetter;
 namespace precache {
 
 class PrecacheConfigurationSettings;
+class PrecacheDatabase;
+class UnfinishedWork;
 
 // Visible for testing.
 extern const int kNoTracking;
@@ -59,18 +66,27 @@ extern const int kNoTracking;
 //   void PrecacheResourcesForTopURLs(
 //       net::URLRequestContextGetter* request_context,
 //       const std::list<GURL>& top_urls) {
-//     fetcher_.reset(new PrecacheFetcher(request_context, top_urls, this));
+//     fetcher_.reset(new PrecacheFetcher(...));
+//     fetcher_->Init();
+//   }
+//
+//   virtual void OnInitDone() {
 //     fetcher_->Start();
 //   }
 //
 //   virtual void OnDone() {
-//     // Do something when precaching is done.
+//     // Do something when precaching is done, e.g.:
+//     fetcher_->Shutdown();
+//   }
+//
+//   virtual void OnShutdownDone() {
+//     fetcher_.reset();
 //   }
 //
 //  private:
 //   std::unique_ptr<PrecacheFetcher> fetcher_;
 // };
-class PrecacheFetcher {
+class PrecacheFetcher : public base::SupportsWeakPtr<PrecacheFetcher> {
  public:
   class PrecacheDelegate {
    public:
@@ -78,6 +94,13 @@ class PrecacheFetcher {
     // were fetched or not. If the PrecacheFetcher is destroyed before OnDone is
     // called, then precaching will be canceled and OnDone will not be called.
     virtual void OnDone() = 0;
+
+    // Called when |PrecacheFetcher| has finished initializing and can be
+    // started.
+    virtual void OnInitDone() = 0;
+
+    // Called after |PrecacheFetcher| has shut down and can be destroyed.
+    virtual void OnShutdownDone() = 0;
   };
 
   // Visible for testing.
@@ -87,20 +110,38 @@ class PrecacheFetcher {
   // prioritized list of hosts that the user commonly visits. These hosts are
   // used by a server side component to construct a list of resource URLs that
   // the user is likely to fetch.
-  PrecacheFetcher(const std::vector<std::string>& starting_hosts,
-                  net::URLRequestContextGetter* request_context,
-                  const GURL& config_url,
-                  const std::string& manifest_url_prefix,
-                  PrecacheDelegate* precache_delegate);
+  PrecacheFetcher(
+      const std::vector<std::string>& starting_hosts,
+      net::URLRequestContextGetter* request_context,
+      const GURL& config_url,
+      const std::string& manifest_url_prefix,
+      PrecacheDelegate* precache_delegate,
+      base::WeakPtr<PrecacheDatabase> precache_database,

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

It seems odd for the PrecacheFetcher to have knowledge of the database. AIUI,
ideally PrecacheFetcher would just fetch URLs that it's told to fetch. What if
the classes were reorganized like this:

- Make UnfinishedWork contain almost all the state of PrecacheFetcher, including
the starting_hosts, the config_url, and the ConfigurationSettings itself if it's
already been fetched. That way, the UnfinishedWork can represent the state right
before precaching is started.
- Make PrecacheFetcher always take in an UnfinishedWork and develop its state
around that, where UnfinishedWork could represent the state before precaching
was even started.

Then, PrecacheManager could use PrecacheDatabase to check if any partially done
UnfinishedWork was saved and use that if it exists, otherwise it could just
create a new UnfinishedWork representing the state before any precaching is
done, e.g. only the starting_hosts and stuff are set.

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

Done.

+      scoped_refptr<base::SingleThreadTaskRunner> database_task_runner);
 
   virtual ~PrecacheFetcher();
 
+  void Init(const base::TimeDelta& max_age);
+
   // Starts fetching resources to precache. URLs are fetched sequentially. Can
   // be called from any thread. Start should only be called once on a
   // PrecacheFetcher instance.
   void Start();
 
+  void Shutdown();
+
+  // Used for testing to put the fetcher into a state where it has work to do.
+  void 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,

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

nit: Why an int? Why not a size_t?

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

Done.

+                         const base::TimeTicks& start_time);
+
  private:
+  void OnInitDone(scoped_refptr<UnfinishedWork> unfinished_work);
+  void OnShutdownDone();
+
   // Fetches the next resource or manifest URL, if any remain. Fetching is done
   // sequentially and depth-first: all resources are fetched for a manifest
   // before the next manifest is fetched. This is done to limit the length of
@@ -147,6 +188,12 @@ class PrecacheFetcher {
 
   std::unique_ptr<PrecacheConfigurationSettings> config_;
 
+  // The database for storing unfinished work. Should only be used on
+  // the DB thread.
+  base::WeakPtr<PrecacheDatabase> precache_database_;
+
+  const scoped_refptr<base::SingleThreadTaskRunner> database_task_runner_;
+
   // Tally of the total number of bytes contained in URL fetches, including
   // config, manifests, and resources. This the number of bytes as they would be
   // compressed over the network.