Implement ID/Hash-based extension cache map.

BUG=447077
TBR=benjhayden,pkasting

Review URL: https://codereview.chromium.org/901413003

Cr-Commit-Position: refs/heads/master@{#318418}

8 files changed, 75 insertions, 22 deletions

diff --git a/extensions/browser/updater/extension_cache.h b/extensions/browser/updater/extension_cache.h
index f6bdb02..c576ae6 100644
--- a/extensions/browser/updater/extension_cache.h
+++ b/extensions/browser/updater/extension_cache.h
@@ -40,6 +40,7 @@ class ExtensionCache {
   // |version| for the extension. Extension will be marked as used with current
   // timestamp.
   virtual bool GetExtension(const std::string& id,
+                            const std::string& expected_hash,
                             base::FilePath* file_path,
                             std::string* version) = 0;
 
@@ -50,6 +51,7 @@ class ExtensionCache {
   // deleted from the disk. There is no guarantee that |callback| will be
   // called.
   virtual void PutExtension(const std::string& id,
+                            const std::string& expected_hash,
                             const base::FilePath& file_path,
                             const std::string& version,
                             const PutExtensionCallback& callback) = 0;
diff --git a/extensions/browser/updater/extension_downloader.cc b/extensions/browser/updater/extension_downloader.cc
index f572fa0..e1e61f8 100644
--- a/extensions/browser/updater/extension_downloader.cc
+++ b/extensions/browser/updater/extension_downloader.cc
@@ -714,12 +714,14 @@ void ExtensionDownloader::FetchUpdatedExtension(
   } else {
     std::string version;
     if (extension_cache_ &&
-        extension_cache_->GetExtension(fetch_data->id, NULL, &version) &&
+        extension_cache_->GetExtension(fetch_data->id, fetch_data->package_hash,
+                                       NULL, &version) &&
         version == fetch_data->version) {
       base::FilePath crx_path;
       // Now get .crx file path and mark extension as used.
-      extension_cache_->GetExtension(fetch_data->id, &crx_path, &version);
-      NotifyDelegateDownloadFinished(fetch_data.Pass(), crx_path, false);
+      extension_cache_->GetExtension(fetch_data->id, fetch_data->package_hash,
+                                     &crx_path, &version);
+      NotifyDelegateDownloadFinished(fetch_data.Pass(), true, crx_path, false);
     } else {
       extensions_queue_.ScheduleRequest(fetch_data.Pass());
     }
@@ -728,13 +730,34 @@ void ExtensionDownloader::FetchUpdatedExtension(
 
 void ExtensionDownloader::NotifyDelegateDownloadFinished(
     scoped_ptr<ExtensionFetch> fetch_data,
+    bool from_cache,
     const base::FilePath& crx_path,
     bool file_ownership_passed) {
+  // Dereference required params before passing a scoped_ptr.
+  const std::string& id = fetch_data->id;
+  const std::string& package_hash = fetch_data->package_hash;
+  const GURL& url = fetch_data->url;
+  const std::string& version = fetch_data->version;
+  const std::set<int>& request_ids = fetch_data->request_ids;
   delegate_->OnExtensionDownloadFinished(
-      CRXFileInfo(fetch_data->id, crx_path, fetch_data->package_hash),
-      file_ownership_passed, fetch_data->url, fetch_data->version,
-      ping_results_[fetch_data->id], fetch_data->request_ids);
+      CRXFileInfo(id, crx_path, package_hash), file_ownership_passed, url,
+      version, ping_results_[id], request_ids,
+      from_cache ? base::Bind(&ExtensionDownloader::CacheInstallDone,
+                              weak_ptr_factory_.GetWeakPtr(),
+                              base::Passed(&fetch_data))
+                 : ExtensionDownloaderDelegate::InstallCallback());
+  if (!from_cache)
+    ping_results_.erase(id);
+}
+
+void ExtensionDownloader::CacheInstallDone(
+    scoped_ptr<ExtensionFetch> fetch_data,
+    bool should_download) {
   ping_results_.erase(fetch_data->id);
+  if (should_download) {
+    // Resume download from cached manifest data.
+    extensions_queue_.ScheduleRequest(fetch_data.Pass());
+  }
 }
 
 void ExtensionDownloader::CreateExtensionFetcher() {
@@ -802,15 +825,14 @@ void ExtensionDownloader::OnCRXFetchComplete(
         extensions_queue_.reset_active_request();
     if (extension_cache_) {
       const std::string& version = fetch_data->version;
+      const std::string& expected_hash = fetch_data->package_hash;
       extension_cache_->PutExtension(
-          id,
-          crx_path,
-          version,
+          id, expected_hash, crx_path, version,
           base::Bind(&ExtensionDownloader::NotifyDelegateDownloadFinished,
-                     weak_ptr_factory_.GetWeakPtr(),
-                     base::Passed(&fetch_data)));
+                     weak_ptr_factory_.GetWeakPtr(), base::Passed(&fetch_data),
+                     false));
     } else {
-      NotifyDelegateDownloadFinished(fetch_data.Pass(), crx_path, true);
+      NotifyDelegateDownloadFinished(fetch_data.Pass(), false, crx_path, true);
     }
   } else if (IterateFetchCredentialsAfterFailure(
                  &active_request, status, response_code)) {
diff --git a/extensions/browser/updater/extension_downloader.h b/extensions/browser/updater/extension_downloader.h
index 4a93def..9c321b5 100644
--- a/extensions/browser/updater/extension_downloader.h
+++ b/extensions/browser/updater/extension_downloader.h
@@ -245,9 +245,14 @@ class ExtensionDownloader : public net::URLFetcherDelegate,
 
   // Notify delegate and remove ping results.
   void NotifyDelegateDownloadFinished(scoped_ptr<ExtensionFetch> fetch_data,
+                                      bool from_cache,
                                       const base::FilePath& crx_path,
                                       bool file_ownership_passed);
 
+  // Cached extension installation completed. If it was not successful, we will
+  // try to download it from the web store using already fetched manifest.
+  void CacheInstallDone(scoped_ptr<ExtensionFetch> fetch_data, bool installed);
+
   // Potentially updates an ExtensionFetch's authentication state and returns
   // |true| if the fetch should be retried. Returns |false| if the failure was
   // not related to authentication, leaving the ExtensionFetch data unmodified.
diff --git a/extensions/browser/updater/extension_downloader_delegate.h b/extensions/browser/updater/extension_downloader_delegate.h
index 2e5eaa5..a1076da 100644
--- a/extensions/browser/updater/extension_downloader_delegate.h
+++ b/extensions/browser/updater/extension_downloader_delegate.h
@@ -8,6 +8,7 @@
 #include <set>
 #include <string>
 
+#include "base/callback.h"
 #include "base/time/time.h"
 #include "extensions/browser/crx_file_info.h"
 #include "extensions/browser/updater/manifest_fetch_data.h"
@@ -59,6 +60,10 @@ class ExtensionDownloaderDelegate {
     base::Time day_start;
   };
 
+  // A callback that is called to indicate if ExtensionDownloader should ignore
+  // the cached entry and download a new .crx file.
+  typedef base::Callback<void(bool should_download)> InstallCallback;
+
   // One of the following 3 methods is always invoked for a given extension
   // id, if AddExtension() or AddPendingExtension() returned true when that
   // extension was added to the ExtensionDownloader.
@@ -81,14 +86,25 @@ class ExtensionDownloaderDelegate {
 
   // Invoked if the extension had an update available and its crx was
   // successfully downloaded to |path|. |ownership_passed| is true if delegate
-  // should get ownership of the file.
-  virtual void OnExtensionDownloadFinished(
-      const CRXFileInfo& file,
-      bool file_ownership_passed,
-      const GURL& download_url,
-      const std::string& version,
-      const PingResult& ping_result,
-      const std::set<int>& request_ids) = 0;
+  // should get ownership of the file. The downloader may be able to get the
+  // .crx file both from a locally cached version or by issuing a network
+  // request. If the install attempt by the delegate fails and the source was
+  // the cache, the cached version may be corrupt (or simply not the desired
+  // one), and we'd like to try downloading the .crx from the network and have
+  // the delegate attempt install again. So if the |callback| parameter is
+  // non-null (if the file was taken from the cache), on install failure the
+  // downloader should be notified to try download from network by calling the
+  // callback with true; on successful install it should be called with false so
+  // that downloader could release all downloaded metadata. After downloading
+  // the delegate will be once again called with OnExtensionDownloadFinished (or
+  // OnExtensionDownloadFailed) called again with the same |request_ids|.
+  virtual void OnExtensionDownloadFinished(const CRXFileInfo& file,
+                                           bool file_ownership_passed,
+                                           const GURL& download_url,
+                                           const std::string& version,
+                                           const PingResult& ping_result,
+                                           const std::set<int>& request_ids,
+                                           const InstallCallback& callback) = 0;
 
   // The remaining methods are used by the ExtensionDownloader to retrieve
   // information about extensions from the delegate.
diff --git a/extensions/browser/updater/null_extension_cache.cc b/extensions/browser/updater/null_extension_cache.cc
index 3241d20..3f60d60 100644
--- a/extensions/browser/updater/null_extension_cache.cc
+++ b/extensions/browser/updater/null_extension_cache.cc
@@ -26,12 +26,14 @@ void NullExtensionCache::AllowCaching(const std::string& id) {
 }
 
 bool NullExtensionCache::GetExtension(const std::string& id,
+                                      const std::string& expected_hash,
                                       base::FilePath* file_path,
                                       std::string* version) {
   return false;
 }
 
 void NullExtensionCache::PutExtension(const std::string& id,
+                                      const std::string& expected_hash,
                                       const base::FilePath& file_path,
                                       const std::string& version,
                                       const PutExtensionCallback& callback) {
diff --git a/extensions/browser/updater/null_extension_cache.h b/extensions/browser/updater/null_extension_cache.h
index 217dbe0..1a2161a 100644
--- a/extensions/browser/updater/null_extension_cache.h
+++ b/extensions/browser/updater/null_extension_cache.h
@@ -20,9 +20,11 @@ class NullExtensionCache : public ExtensionCache {
   void Shutdown(const base::Closure& callback) override;
   void AllowCaching(const std::string& id) override;
   bool GetExtension(const std::string& id,
+                    const std::string& expected_hash,
                     base::FilePath* file_path,
                     std::string* version) override;
   void PutExtension(const std::string& id,
+                    const std::string& expected_hash,
                     const base::FilePath& file_path,
                     const std::string& version,
                     const PutExtensionCallback& callback) override;
diff --git a/extensions/browser/updater/update_service.cc b/extensions/browser/updater/update_service.cc
index 10a86c3..0cb055c 100644
--- a/extensions/browser/updater/update_service.cc
+++ b/extensions/browser/updater/update_service.cc
@@ -58,11 +58,14 @@ void UpdateService::OnExtensionDownloadFinished(
     const GURL& download_url,
     const std::string& version,
     const PingResult& ping,
-    const std::set<int>& request_id) {
+    const std::set<int>& request_id,
+    const InstallCallback& install_callback) {
   // TODO(rockot): Actually unpack and install the CRX.
   auto callback = download_callback_;
   download_callback_.Reset();
   callback.Run(true);
+  if (!install_callback.is_null())
+    install_callback.Run(true);
 }
 
 bool UpdateService::IsExtensionPending(const std::string& id) {
diff --git a/extensions/browser/updater/update_service.h b/extensions/browser/updater/update_service.h
index 2f4cd80..4d19204 100644
--- a/extensions/browser/updater/update_service.h
+++ b/extensions/browser/updater/update_service.h
@@ -53,7 +53,8 @@ class UpdateService : public KeyedService, public ExtensionDownloaderDelegate {
                                    const GURL& download_url,
                                    const std::string& version,
                                    const PingResult& ping,
-                                   const std::set<int>& request_id) override;
+                                   const std::set<int>& request_id,
+                                   const InstallCallback& callback) override;
   bool IsExtensionPending(const std::string& id) override;
   bool GetExtensionExistingVersion(const std::string& id,
                                    std::string* version) override;