predictors: Pass manifests from Bork to store in ResourcePrefetchPredictor.

components/precache/content/precache_manager.h

 // 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.
 
 #ifndef COMPONENTS_PRECACHE_CONTENT_PRECACHE_MANAGER_H_
 #define COMPONENTS_PRECACHE_CONTENT_PRECACHE_MANAGER_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
@@ skipping 37 matching lines @@
 }
 
 namespace syncer {
 class SyncService;
 }
 
 namespace precache {
 
 class PrecacheDatabase;
 class PrecacheUnfinishedWork;
+class PrecacheManifest;
 
 extern const char kPrecacheFieldTrialName[];
 
 // Visible for test.
 extern const char kMinCacheSizeParam[];
 size_t NumTopHosts();
 
 // Class that manages all precaching-related activities. Owned by the
 // BrowserContext that it is constructed for. Use
 // PrecacheManagerFactory::GetForBrowserContext to get an instance of this
 // class. All methods must be called on the UI thread unless indicated
 // otherwise.
 // TODO(sclittle): Delete precache history when browsing history is deleted.
 // http://crbug.com/326549
 class PrecacheManager : public KeyedService,
                         public PrecacheFetcher::PrecacheDelegate,
                         public base::SupportsWeakPtr<PrecacheManager> {
  public:
+  class Delegate {
+   public:
+    // Called when a precache manifest has been successfully fetched and parsed.
+    virtual void OnManifestFetched(const std::string& host,
+                                   const PrecacheManifest& manifest) = 0;
+  };
+
   typedef base::Callback<void(bool)> PrecacheCompletionCallback;
 
   PrecacheManager(content::BrowserContext* browser_context,
                   const syncer::SyncService* sync_service,
                   const history::HistoryService* history_service,
                   const data_reduction_proxy::DataReductionProxySettings*
                       data_reduction_proxy_settings,
+                  Delegate* delegate,
                   const base::FilePath& db_path,
                   std::unique_ptr<PrecacheDatabase> precache_database);
   ~PrecacheManager() override;
 
   // Returns true if the client is in the experiment group -- that is,
   // precaching is allowed based on user settings, and enabled as part of a
   // field trial or by commandline flag. Virtual for testing.
   virtual bool IsInExperimentGroup() const;
 
   // Returns true if the client is in the control group -- that is, precaching
@@ skipping 53 matching lines @@
     ALLOWED,
     DISALLOWED,
     PENDING
   };
 
   // From KeyedService.
   void Shutdown() override;
 
   // From PrecacheFetcher::PrecacheDelegate.
   void OnDone() override;
+  void OnManifestFetched(const std::string& host,
+                         const PrecacheManifest& manifest) override;
 
   // Registers the precache synthetic field trial for users whom the precache
   // task was run recently. |last_precache_time| is the last time precache task
   // was run.
   void RegisterSyntheticFieldTrial(const base::Time last_precache_time);
 
   // Callback when fetching unfinished work from storage is done.
   void OnGetUnfinishedWorkDone(
       std::unique_ptr<PrecacheUnfinishedWork> unfinished_work);
 
@@ skipping 46 matching lines @@
 
   // The history service corresponding to the browser context. Used to determine
   // the list of top hosts. May be null.
   const history::HistoryService* const history_service_;
 
   // The data reduction proxy settings object corresponding to the browser
   // context. Used to determine if the proxy is enabled.
   const data_reduction_proxy::DataReductionProxySettings* const
       data_reduction_proxy_settings_;
 
+  // The Delegate corresponding to the browser context. Used to notify the
+  // browser about a new available manifest. May be null.
+  Delegate* delegate_;

[Benoit L, 2017/03/21 13:24:15]

I understand why this is fine here, as the service resolution mechanism enforces
a longer lifetime for the delegate than this service, but that's quite subtle.

Can you put a comment at the location where you provide the delegate, to make it
clear that's safe?

[alexilin, 2017/03/21 15:32:13]

Isn't it clear from DependsOn() list in factory class? 
There are other dependencies injected by a factory in the same way. And it seems
like it's a common practice in Chromium.
On the contrary, I'd suggest to add comment to PrecacheManger class that all
injected in constructor dependencies must outlive the class, WDYT?

[Benoit L, 2017/03/21 15:46:15]

Acknowledged.

+
   // The PrecacheFetcher used to precache resources. Should only be used on the
   // UI thread.
   std::unique_ptr<PrecacheFetcher> precache_fetcher_;
 
   // The callback that will be run if precaching finishes without being
   // canceled.
   PrecacheCompletionCallback precache_completion_callback_;
 
   // The PrecacheDatabase for tracking precache metrics. Should only be used on
   // the DB thread.
@@ skipping 13 matching lines @@
 
   // Work that hasn't yet finished.
   std::unique_ptr<PrecacheUnfinishedWork> unfinished_work_;
 
   DISALLOW_COPY_AND_ASSIGN(PrecacheManager);
 };
 
 }  // namespace precache
 
 #endif  // COMPONENTS_PRECACHE_CONTENT_PRECACHE_MANAGER_H_