Differential updates for components.

chrome/browser/component_updater/component_updater_service.cc

Index: chrome/browser/component_updater/component_updater_service.cc
diff --git a/chrome/browser/component_updater/component_updater_service.cc b/chrome/browser/component_updater/component_updater_service.cc
index f3eee77c204791f46acb13e813f98b5b3b1f8a6b..6fc25854214dc7bad89e71712f486dffb864f3b2 100644
--- a/chrome/browser/component_updater/component_updater_service.cc
+++ b/chrome/browser/component_updater/component_updater_service.cc
@@ -10,6 +10,7 @@
 
 #include "base/at_exit.h"
 #include "base/bind.h"
+#include "base/compiler_specific.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
@@ -21,11 +22,13 @@
 #include "base/strings/stringprintf.h"
 #include "base/timer.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/component_updater/component_patcher.h"
 #include "chrome/browser/component_updater/component_unpacker.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_utility_messages.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/extensions/extension.h"
+#include "chrome/common/omaha_query_params/omaha_query_params.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/utility_process_host.h"
@@ -47,21 +50,24 @@ using extensions::Extension;
 // base::Bind() calls are not refcounted.
 
 namespace {
+
 // Manifest sources, from most important to least important.
 const CrxComponent::UrlSource kManifestSources[] = {
   CrxComponent::BANDAID,
   CrxComponent::CWS_PUBLIC,
-  CrxComponent::CWS_SANDBOX
+  CrxComponent::CWS_SANDBOX,
 };
 
 // Extends an omaha compatible update check url |query| string. Does
 // not mutate the string if it would be longer than |limit| chars.
 bool AddQueryString(const std::string& id,
                     const std::string& version,
+                    const std::string& fingerprint,
                     size_t limit,
                     std::string* query) {
   std::string additional =
-      base::StringPrintf("id=%s&v=%s&uc", id.c_str(), version.c_str());
+      base::StringPrintf("id=%s&v=%s&fp=%s&uc",
+                         id.c_str(), version.c_str(), fingerprint.c_str());
   additional = "x=" + net::EscapeQueryParamValue(additional, true);
   if ((additional.size() + query->size() + 1) > limit)
     return false;
@@ -163,34 +169,81 @@ void StartFetch(net::URLFetcher* fetcher,
   fetcher->Start();
 }
 
-// Returs true if the url request of |fetcher| was succesful.
+// Returns true if the url request of |fetcher| was succesful.
 bool FetchSuccess(const net::URLFetcher& fetcher) {
   return (fetcher.GetStatus().status() == net::URLRequestStatus::SUCCESS) &&
          (fetcher.GetResponseCode() == 200);
 }
 
+// Returns the error code which occured during the fetch. This function
+// is intended to be called for logging purposes only, since it folds different
+// types of errors to fit them in the returned type. The function returns 0 if
+// the fetch was successful. If errors happen, the function could return a
+// network error, an http response code, or the status of the fetch, if the
+// fetch is pending or canceled.
+int GetFetchError(const net::URLFetcher& fetcher) {
+  if (FetchSuccess(fetcher))
+    return 0;
+
+  const net::URLRequestStatus::Status status(fetcher.GetStatus().status());
+  if (status == net::URLRequestStatus::FAILED)
+    return fetcher.GetStatus().error();
+
+  if (status == net::URLRequestStatus::IO_PENDING ||
+      status == net::URLRequestStatus::CANCELED)
+    return status;
+
+  const int response_code(fetcher.GetResponseCode());
+  if (status == net::URLRequestStatus::SUCCESS && response_code != 200)
+    return response_code;
+
+  return -1;
+}
+
+
 // This is the one and only per-item state structure. Designed to be hosted
 // in a std::vector or a std::list. The two main members are |component|
 // which is supplied by the the component updater client and |status| which
 // is modified as the item is processed by the update pipeline. The expected
 // transition graph is:
-//                                error          error         error
-//              +--kNoUpdate<------<-------+------<------+------<------+
-//              |                          |             |             |
-//              V                  yes     |             |             |
-//  kNew --->kChecking-->[update?]----->kCanUpdate-->kDownloading-->kUpdating
-//              ^               |                                      |
-//              |               |no                                    |
-//              |--kUpToDate<---+                                      |
-//              |                                  success             |
-//              +--kUpdated<-------------------------------------------+
+//
+//                                 kNew
+//                                  |
+//                                  V
+//     +----------------------> kChecking -<---------+-----<-------+
+//     |                            |                |             |
+//     |              error         V       no       |             |
+//  kNoUpdate <---------------- [update?] ->---- kUpToDate     kUpdated
+//     ^                            |                              ^
+//     |                        yes |                              |
+//     |        diff=false          V                              |
+//     |          +-----------> kCanUpdate                         |
+//     |          |                 |                              |
+//     |          |                 V              no              |
+//     |          |        [differential update?]->----+           |
+//     |          |                 |                  |           |
+//     |          |             yes |                  |           |
+//     |          |   error         V                  |           |
+//     |          +---------<- kDownloadingDiff        |           |
+//     |          |                 |                  |           |
+//     |          |                 |                  |           |
+//     |          |   error         V                  |           |
+//     |          +---------<- kUpdatingDiff ->--------|-----------+ success
+//     |                                               |           |
+//     |              error                            V           |
+//     +----------------------------------------- kDownloading     |
+//     |                                               |           |
+//     |              error                            V           |
+//     +------------------------------------------ kUpdating ->----+ success
 //
 struct CrxUpdateItem {
   enum Status {
     kNew,
     kChecking,
     kCanUpdate,
+    kDownloadingDiff,
     kDownloading,
+    kUpdatingDiff,
     kUpdating,
     kUpdated,
     kUpToDate,
@@ -199,13 +252,49 @@ struct CrxUpdateItem {
   };
 
   Status status;
-  GURL crx_url;
   std::string id;
-  base::Time last_check;
   CrxComponent component;
-  Version next_version;
 
-  CrxUpdateItem() : status(kNew) {}
+  base::Time last_check;
+
+  // True if the update response includes an update for this component.
+  bool is_update_available;
+
+  // These members are initialized with their corresponding values from the
+  // update server response.
+  GURL crx_url;
+  GURL diff_crx_url;

[cpu_(ooo_6.6-7.5), 2013/06/20 20:40:34]

please put some comment somewhere in this file (here or elsewhere) explaining
what the two package hash mean and how they differ from the fingeprints. 

It is a bit unfortunate that is this code we have pk hash and package hash and
fingerprint. I think we should rename one of them. I suggest we rename pk_hash
to pubkey_hash or pkey_hash or something like that.

But before that, what is the value in this CL of the package hash? I see where
we get it but I don't see it put to use. Maybe it should follow the same fate as
the items of line 282 - 285?

[Sorin Jianu, 2013/06/20 21:34:55]

Members removed. At Omaha we like the idea of having checks to verify the data
integrity of the packet. Even if the CRX itself is tamper resistent, having a
checksum for the whole download helps validating we are actually getting what we
were supposed to get.

+  std::string package_hash;
+  std::string diff_package_hash;
+  int size;
+  int diff_size;
+
+  // The from/to version and fingerprint values.
+  Version previous_version;
+  Version next_version;
+  std::string previous_fp;
+  std::string next_fp;
+
+  // True if the differential update failed for any reason.
+  bool diff_update_failed;
+
+  // The error information for full and differential updates.

[cpu_(ooo_6.6-7.5), 2013/06/20 20:40:34]

please remove all the members that are used only for logging or pinging that was
removed, I believe most of the ones below.

[Sorin Jianu, 2013/06/20 21:34:55]

I have removed the error members.

These members are very useful for debugging since they contain the result of the
update. We will have to introduce the ping code in the next CL and we will need
these data members. 

Please allow me to ask this question for the sake of understanding the current
design and avoid future friction. Why is there a concern with having members in
an object which purpose is to maintain the state of the crx as the crx is run by
the state machine?

+  int error_code;
+  int extra_code1;
+  int diff_error_code;
+  int diff_extra_code1;
+
+  CrxUpdateItem()
+      : status(kNew),
+        is_update_available(false),
+        size(0),
+        diff_size(0),
+        diff_update_failed(false),
+        error_code(0),
+        extra_code1(0),
+        diff_error_code(0),
+        diff_extra_code1(0) {
+  }
 
   // Function object used to find a specific component.
   class FindById {
@@ -220,9 +309,21 @@ struct CrxUpdateItem {
   };
 };
 
-}  // namespace.
+// Returns true if a differential update is available for the update item.
+bool IsDiffUpdateAvailable(const CrxUpdateItem* update_item) {
+  return update_item->diff_crx_url.is_valid();
+}
 
-typedef ComponentUpdateService::Configurator Config;
+// Returns true if a differential update is available, it has not failed yet,
+// and the configuration allows it.
+bool CanTryDiffUpdate(const CrxUpdateItem* update_item,
+                      const ComponentUpdateService::Configurator& config) {
+  return IsDiffUpdateAvailable(update_item) &&
+         !update_item->diff_update_failed &&
+         config.DeltasEnabled();
+}
+
+}  // namespace.
 
 CrxComponent::CrxComponent()
     : installer(NULL),
@@ -240,7 +341,7 @@ CrxComponent::~CrxComponent() {
 // rest of the browser, so even if we have many components registered and
 // eligible for update, we only do one thing at a time with pauses in between
 // the tasks. Also when we do network requests there is only one |url_fetcher_|
-// in flight at at a time.
+// in flight at a time.
 // There are no locks in this code, the main structure |work_items_| is mutated
 // only from the UI thread. The unpack and installation is done in the file
 // thread and the network requests are done in the IO thread and in the file
@@ -304,6 +405,7 @@ class CrxUpdateService : public ComponentUpdateService {
     ComponentInstaller* installer;
     std::vector<uint8> pk_hash;
     std::string id;
+    std::string fingerprint;
     CRXContext() : installer(NULL) {}
   };
 
@@ -319,8 +421,7 @@ class CrxUpdateService : public ComponentUpdateService {
       const UpdateManifest::Results& results);
 
   // See ManifestParserBridge.
-  void OnParseUpdateManifestFailed(
-      const std::string& error_message);
+  void OnParseUpdateManifestFailed(const std::string& error_message);
 
   bool AddItemToUpdateCheck(CrxUpdateItem* item, std::string* query);
 
@@ -333,19 +434,22 @@ class CrxUpdateService : public ComponentUpdateService {
   void Install(const CRXContext* context, const base::FilePath& crx_path);
 
   void DoneInstalling(const std::string& component_id,
-                      ComponentUnpacker::Error error);
+                      ComponentUnpacker::Error error,
+                      int extended_error);
 
   size_t ChangeItemStatus(CrxUpdateItem::Status from,
                           CrxUpdateItem::Status to);
 
   CrxUpdateItem* FindUpdateItemById(const std::string& id);
 
-  scoped_ptr<Config> config_;
+  scoped_ptr<ComponentUpdateService::Configurator> config_;
+
+  scoped_ptr<ComponentPatcher> component_patcher_;
 
   scoped_ptr<net::URLFetcher> url_fetcher_;
 
-  typedef std::vector<CrxUpdateItem*> UpdateItems;
   // A collection of every work item.
+  typedef std::vector<CrxUpdateItem*> UpdateItems;
   UpdateItems work_items_;
 
   // A particular set of items from work_items_, which should be checked ASAP.
@@ -353,7 +457,8 @@ class CrxUpdateService : public ComponentUpdateService {
 
   base::OneShotTimer<CrxUpdateService> timer_;
 
-  Version chrome_version_;
+  const Version chrome_version_;
+  const std::string prod_id_;
 
   bool running_;
 
@@ -362,10 +467,12 @@ class CrxUpdateService : public ComponentUpdateService {
 
 //////////////////////////////////////////////////////////////////////////////
 
-CrxUpdateService::CrxUpdateService(
-    ComponentUpdateService::Configurator* config)
+CrxUpdateService::CrxUpdateService(ComponentUpdateService::Configurator* config)
     : config_(config),
+      component_patcher_(config->CreateComponentPatcher()),
       chrome_version_(chrome::VersionInfo().Version()),
+      prod_id_(chrome::OmahaQueryParams::GetProdIdString(
+          chrome::OmahaQueryParams::CHROME)),
       running_(false) {
 }
 
@@ -450,7 +557,7 @@ CrxUpdateItem* CrxUpdateService::FindUpdateItemById(const std::string& id) {
 }
 
 // Changes all the components in |work_items_| that have |from| status to
-// |to| statatus and returns how many have been changed.
+// |to| status and returns how many have been changed.
 size_t CrxUpdateService::ChangeItemStatus(CrxUpdateItem::Status from,
                                           CrxUpdateItem::Status to) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -491,6 +598,7 @@ ComponentUpdateService::Status CrxUpdateService::RegisterComponent(
   uit = new CrxUpdateItem;
   uit->id.swap(id);
   uit->component = component;
+
   work_items_.push_back(uit);
   // If this is the first component registered we call Start to
   // schedule the first timer.
@@ -501,17 +609,29 @@ ComponentUpdateService::Status CrxUpdateService::RegisterComponent(
 }
 
 // Sets a component to be checked for updates.
-// The componet to add is |crxit| and the |query| string is modified with the
+// The component to add is |crxit| and the |query| string is modified with the
 // required omaha compatible query. Returns false when the query strings
 // is longer than specified by UrlSizeLimit().
 bool CrxUpdateService::AddItemToUpdateCheck(CrxUpdateItem* item,
                                             std::string* query) {
   if (!AddQueryString(item->id,
                       item->component.version.GetString(),
+                      item->component.fingerprint,
                       config_->UrlSizeLimit(), query))
     return false;
+
   item->status = CrxUpdateItem::kChecking;
   item->last_check = base::Time::Now();
+  item->is_update_available = false;
+  item->previous_version = item->component.version;
+  item->next_version = Version();
+  item->previous_fp = item->component.fingerprint;
+  item->next_fp = "";

[cpu_(ooo_6.6-7.5), 2013/06/20 20:40:34]

.clear()

[Sorin Jianu, 2013/06/20 21:34:55]

Done.

+  item->diff_update_failed = false;
+  item->error_code = 0;
+  item->extra_code1 = 0;
+  item->diff_error_code = 0;
+  item->diff_extra_code1 = 0;
   return true;
 }
 
@@ -535,16 +655,17 @@ ComponentUpdateService::Status CrxUpdateService::CheckForUpdateSoon(
 
   // Check if the request is too soon.
   base::TimeDelta delta = base::Time::Now() - uit->last_check;
-  if (delta < base::TimeDelta::FromSeconds(config_->OnDemandDelay())) {
+  if (delta < base::TimeDelta::FromSeconds(config_->OnDemandDelay()))
     return kError;
-  }
 
   switch (uit->status) {
     // If the item is already in the process of being updated, there is
     // no point in this call, so return kInProgress.
     case CrxUpdateItem::kChecking:
     case CrxUpdateItem::kCanUpdate:
+    case CrxUpdateItem::kDownloadingDiff:
     case CrxUpdateItem::kDownloading:
+    case CrxUpdateItem::kUpdatingDiff:
     case CrxUpdateItem::kUpdating:
       return kInProgress;
     // Otherwise the item was already checked a while back (or it is new),
@@ -583,13 +704,21 @@ void CrxUpdateService::ProcessPendingItems() {
     if (item->status != CrxUpdateItem::kCanUpdate)
       continue;
     // Found component to update, start the process.
-    item->status = CrxUpdateItem::kDownloading;
     CRXContext* context = new CRXContext;
     context->pk_hash = item->component.pk_hash;
     context->id = item->id;
     context->installer = item->component.installer;
+    context->fingerprint = item->next_fp;
+    GURL package_url;
+    if (CanTryDiffUpdate(item, *config_)) {
+      package_url = item->diff_crx_url;
+      item->status = CrxUpdateItem::kDownloadingDiff;
+    } else {
+      package_url = item->crx_url;
+      item->status = CrxUpdateItem::kDownloading;
+    }
     url_fetcher_.reset(net::URLFetcher::Create(
-        0, item->crx_url, net::URLFetcher::GET,
+        0, package_url, net::URLFetcher::GET,
         MakeContextDelegate(this, context)));
     StartFetch(url_fetcher_.get(), config_->RequestContext(), true);
     return;
@@ -700,11 +829,10 @@ void CrxUpdateService::ParseManifest(const std::string& xml) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (config_->InProcess()) {
     UpdateManifest manifest;
-    if (!manifest.Parse(xml)) {
+    if (!manifest.Parse(xml))
        CrxUpdateService::OnParseUpdateManifestFailed(manifest.errors());
-    } else {
+    else
        CrxUpdateService::OnParseUpdateManifestSucceeded(manifest.results());
-    }
   } else {
     UtilityProcessHost* host =
         UtilityProcessHost::Create(new ManifestParserBridge(this),
@@ -752,9 +880,16 @@ void CrxUpdateService::OnParseUpdateManifestSucceeded(
     }
     // All test passed. Queue an upgrade for this component and fire the
     // notifications.
+    crx->is_update_available = true;
     crx->crx_url = it->crx_url;
+    crx->package_hash = it->package_hash;
+    crx->size = it->size;
+    crx->diff_crx_url = it->diff_crx_url;
+    crx->diff_package_hash = it->diff_package_hash;
+    crx->diff_size = it->diff_size;
     crx->status = CrxUpdateItem::kCanUpdate;
     crx->next_version = Version(it->version);
+    crx->next_fp = it->package_fingerprint;
     ++update_pending;
 
     content::NotificationService::current()->Notify(
@@ -789,19 +924,42 @@ void CrxUpdateService::OnURLFetchComplete(const net::URLFetcher* source,
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   int error_code = net::OK;
 
+  CrxUpdateItem* crx = FindUpdateItemById(context->id);
+  DCHECK(crx->status == CrxUpdateItem::kDownloadingDiff ||
+         crx->status == CrxUpdateItem::kDownloading);
+
   if (source->FileErrorOccurred(&error_code) || !FetchSuccess(*source)) {
+    if (crx->status == CrxUpdateItem::kDownloadingDiff) {
+      crx->diff_error_code = GetFetchError(*source);
+      crx->diff_update_failed = true;
+      size_t count = ChangeItemStatus(CrxUpdateItem::kDownloadingDiff,
+                                      CrxUpdateItem::kCanUpdate);
+      DCHECK_EQ(count, 1ul);
+      ScheduleNextRun(true);
+      return;
+    }
+    crx->error_code = GetFetchError(*source);
     size_t count = ChangeItemStatus(CrxUpdateItem::kDownloading,
                                     CrxUpdateItem::kNoUpdate);
     DCHECK_EQ(count, 1ul);
     config_->OnEvent(Configurator::kNetworkError, CrxIdtoUMAId(context->id));
     url_fetcher_.reset();
+
     ScheduleNextRun(false);
   } else {
     base::FilePath temp_crx_path;
     CHECK(source->GetResponseAsFilePath(true, &temp_crx_path));
-    size_t count = ChangeItemStatus(CrxUpdateItem::kDownloading,
-                                    CrxUpdateItem::kUpdating);
+
+    size_t count = 0;
+    if (crx->status == CrxUpdateItem::kDownloadingDiff) {
+      count = ChangeItemStatus(CrxUpdateItem::kDownloadingDiff,
+                               CrxUpdateItem::kUpdatingDiff);
+    } else {
+      count = ChangeItemStatus(CrxUpdateItem::kDownloading,
+                               CrxUpdateItem::kUpdating);
+    }
     DCHECK_EQ(count, 1ul);
+
     url_fetcher_.reset();
 
     content::NotificationService::current()->Notify(
@@ -829,17 +987,19 @@ void CrxUpdateService::Install(const CRXContext* context,
                                const base::FilePath& crx_path) {
   // This function owns the |crx_path| and the |context| object.
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-  ComponentUnpacker
-      unpacker(context->pk_hash, crx_path, context->installer);
-  if (!file_util::Delete(crx_path, false)) {
+  ComponentUnpacker unpacker(context->pk_hash,
+                             crx_path,
+                             context->fingerprint,
+                             component_patcher_.get(),
+                             context->installer);
+  if (!file_util::Delete(crx_path, false))
     NOTREACHED() << crx_path.value();
-  }
   // Why unretained? See comment at top of file.
   BrowserThread::PostDelayedTask(
       BrowserThread::UI,
       FROM_HERE,
       base::Bind(&CrxUpdateService::DoneInstalling, base::Unretained(this),
-                 context->id, unpacker.error()),
+                 context->id, unpacker.error(), unpacker.extended_error()),
       base::TimeDelta::FromMilliseconds(config_->StepDelay()));
   delete context;
 }
@@ -847,14 +1007,35 @@ void CrxUpdateService::Install(const CRXContext* context,
 // Installation has been completed. Adjust the component status and
 // schedule the next check.
 void CrxUpdateService::DoneInstalling(const std::string& component_id,
-                                      ComponentUnpacker::Error error) {
+                                      ComponentUnpacker::Error error,
+                                      int extra_code) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   CrxUpdateItem* item = FindUpdateItemById(component_id);
+  if (item->status == CrxUpdateItem::kUpdatingDiff) {
+    if (error != ComponentUnpacker::kNone) {
+      item->diff_error_code = error;
+      item->diff_extra_code1 = extra_code;
+      item->diff_update_failed = true;
+      size_t count = ChangeItemStatus(CrxUpdateItem::kUpdatingDiff,
+                                      CrxUpdateItem::kCanUpdate);
+      DCHECK_EQ(count, 1ul);
+      ScheduleNextRun(true);
+      return;
+    }
+  }
+
+  if (error != ComponentUnpacker::kNone) {
+    item->error_code = error;
+    item->extra_code1 = extra_code;
+  }
+
   item->status = (error == ComponentUnpacker::kNone) ? CrxUpdateItem::kUpdated :
                                                        CrxUpdateItem::kNoUpdate;
-  if (item->status == CrxUpdateItem::kUpdated)
+  if (item->status == CrxUpdateItem::kUpdated) {
     item->component.version = item->next_version;
+    item->component.fingerprint = item->next_fp;
+  }
 
   Configurator::Events event;
   switch (error) {