Add a way to specify different source urls for the component updater

chrome/browser/component_updater/component_updater_service.cc

 // Copyright (c) 2012 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.
 
 #include "chrome/browser/component_updater/component_updater_service.h"
 
 #include <algorithm>
 #include <vector>
 
 #include "base/at_exit.h"
@@ skipping 27 matching lines @@
 
 using content::BrowserThread;
 using content::UtilityProcessHost;
 using content::UtilityProcessHostClient;
 using extensions::Extension;
 
 // The component updater is designed to live until process shutdown, so
 // 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
+};
+
 // 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,
                     size_t limit,
                     std::string* query) {
   std::string additional =
       base::StringPrintf("id=%s&v=%s&uc", id.c_str(), version.c_str());
   additional = "x=" + net::EscapeQueryParamValue(additional, true);
   if ((additional.size() + query->size() + 1) > limit)
@@ skipping 150 matching lines @@
     }
    private:
     const std::string& id_;
   };
 };
 
 }  // namespace.
 
 typedef ComponentUpdateService::Configurator Config;
 
-CrxComponent::CrxComponent() : installer(NULL) {}
-CrxComponent::~CrxComponent() {}
+CrxComponent::CrxComponent()
+    : installer(NULL),
+      source(BANDAID) {

[cpu_(ooo_6.6-7.5), 2013/01/21 02:47:19]

the default source is the current one so I don't have to change all the
consumers.

+}
+
+CrxComponent::~CrxComponent() {
+}
 
 //////////////////////////////////////////////////////////////////////////////
 // The one and only implementation of the ComponentUpdateService interface. In
 // charge of running the show. The main method is ProcessPendingItems() which
 // is called periodically to do the upgrades/installs or the update checks.
 // An important consideration here is to be as "low impact" as we can to the
 // 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.
@@ skipping 277 matching lines @@
     CRXContext* context = new CRXContext;
     context->pk_hash = item->component.pk_hash;
     context->id = item->id;
     context->installer = item->component.installer;
     url_fetcher_.reset(net::URLFetcher::Create(
         0, item->crx_url, net::URLFetcher::GET,
         MakeContextDelegate(this, context)));
     StartFetch(url_fetcher_.get(), config_->RequestContext(), true);
     return;
   }
 

[cpu_(ooo_6.6-7.5), 2013/01/21 02:47:19]

this new for loop is the meat of the change. Basically the code mostly stays the
same except that 518-575 block is now done potentially 3 times
(arraysize(kManifestSources)) 

-  std::string query;
-  // If no pending upgrades, we check the if there are new
-  // components we have not checked against the server. We
-  // can batch a bunch in a single url request.
-  for (UpdateItems::const_iterator it = work_items_.begin();
-       it != work_items_.end(); ++it) {
-    CrxUpdateItem* item = *it;
-    if (item->status != CrxUpdateItem::kNew)
+  for (size_t ix = 0; ix != arraysize(kManifestSources); ++ix) {
+    const CrxComponent::UrlSource manifest_source = kManifestSources[ix];
+
+    std::string query;
+    // If no pending upgrades, we check the if there are new

[asargent_no_longer_on_chrome, 2013/01/23 17:58:32]

typo: "we check the if there"

[cpu_(ooo_6.6-7.5), 2013/01/23 18:56:06]

Done.

+    // components we have not checked against the server. We
+    // can batch a bunch in a single url request.
+    for (UpdateItems::const_iterator it = work_items_.begin();
+         it != work_items_.end(); ++it) {
+      CrxUpdateItem* item = *it;
+      if (item->status != CrxUpdateItem::kNew)
+        continue;
+      if (item->component.source != manifest_source)
+        continue;
+      if (!AddItemToUpdateCheck(item, &query))
+        break;
+    }
+
+    // Next we can go back to components we already checked, here
+    // we can also batch them in a single url request, as long as
+    // we have not checked them recently.
+    const base::TimeDelta min_delta_time =
+        base::TimeDelta::FromSeconds(config_->MinimumReCheckWait());
+
+    for (UpdateItems::const_iterator it = work_items_.begin();
+         it != work_items_.end(); ++it) {
+      CrxUpdateItem* item = *it;
+      if ((item->status != CrxUpdateItem::kNoUpdate) &&
+          (item->status != CrxUpdateItem::kUpToDate))
+        continue;
+      if (item->component.source != manifest_source)
+        continue;
+      base::TimeDelta delta = base::Time::Now() - item->last_check;
+      if (delta < min_delta_time)
+        continue;
+      if (!AddItemToUpdateCheck(item, &query))
+        break;
+    }
+
+    // Finally, we check components that we already updated as long
+    // we have not checked them recently.

[asargent_no_longer_on_chrome, 2013/01/23 17:58:32]

typo: comment should say "..updated as long as we ..." (missing the second 'as')

[cpu_(ooo_6.6-7.5), 2013/01/23 18:56:06]

Done.

+    for (UpdateItems::const_iterator it = work_items_.begin();
+         it != work_items_.end(); ++it) {
+      CrxUpdateItem* item = *it;
+      if (item->status != CrxUpdateItem::kUpdated)
+        continue;
+      if (item->component.source != manifest_source)
+        continue;
+      base::TimeDelta delta = base::Time::Now() - item->last_check;
+      if (delta < min_delta_time)
+        continue;
+      if (!AddItemToUpdateCheck(item, &query))
+        break;
+    }

[asargent_no_longer_on_chrome, 2013/01/23 17:58:32]

I assume that work_items_ will always be very short right?

(If it were going to be very long, I'd say we'd want to avoid iterating over it
3 separate times inside the body of the outer for loop since this CL now makes
it 9 times total; perhaps by separating into separate lists based on
item->status). 

[cpu_(ooo_6.6-7.5), 2013/01/23 18:56:06]

Yep, currently at 4 items.

On 2013/01/23 17:58:32, Antony Sargent wrote:

+
+    // If no components to update we move down to the next source.
+    if (query.empty())
       continue;
-    if (!AddItemToUpdateCheck(item, &query))
-      break;
-  }
 
-  // Next we can go back to components we already checked, here
-  // we can also batch them in a single url request, as long as
-  // we have not checked them recently.
-  const base::TimeDelta min_delta_time =
-      base::TimeDelta::FromSeconds(config_->MinimumReCheckWait());
+    // We got components to check. Start the url request and exit.
+    const std::string full_query =
+        MakeFinalQuery(config_->UpdateUrl(manifest_source).spec(),
+                       query,
+                       config_->ExtraRequestParams());
 
-  for (UpdateItems::const_iterator it = work_items_.begin();
-       it != work_items_.end(); ++it) {
-    CrxUpdateItem* item = *it;
-    if ((item->status != CrxUpdateItem::kNoUpdate) &&
-        (item->status != CrxUpdateItem::kUpToDate))
-      continue;
-    base::TimeDelta delta = base::Time::Now() - item->last_check;
-    if (delta < min_delta_time)
-      continue;
-    if (!AddItemToUpdateCheck(item, &query))
-      break;
-  }
-  // Finally, we check components that we already updated.
-  for (UpdateItems::const_iterator it = work_items_.begin();
-       it != work_items_.end(); ++it) {
-    CrxUpdateItem* item = *it;
-    if (item->status != CrxUpdateItem::kUpdated)
-      continue;
-    base::TimeDelta delta = base::Time::Now() - item->last_check;
-    if (delta < min_delta_time)
-      continue;
-    if (!AddItemToUpdateCheck(item, &query))
-      break;
-  }
-
-  if (query.empty()) {
-    // Next check after the long sleep.
-    ScheduleNextRun(false);
+    url_fetcher_.reset(net::URLFetcher::Create(
+        0, GURL(full_query), net::URLFetcher::GET,
+        MakeContextDelegate(this, new UpdateContext())));
+    StartFetch(url_fetcher_.get(), config_->RequestContext(), false);
     return;
   }
 
-  // We got components to check. Start the url request.
-  const std::string full_query = MakeFinalQuery(config_->UpdateUrl().spec(),
-                                                query,
-                                                config_->ExtraRequestParams());
-  url_fetcher_.reset(net::URLFetcher::Create(
-      0, GURL(full_query), net::URLFetcher::GET,
-      MakeContextDelegate(this, new UpdateContext())));
-  StartFetch(url_fetcher_.get(), config_->RequestContext(), false);
+  // No components to update. Next check after the long sleep.
+  ScheduleNextRun(false);
 }
 
 // Caled when we got a response from the update server. It consists of an xml
 // document following the omaha update scheme.
 void CrxUpdateService::OnURLFetchComplete(const net::URLFetcher* source,
                                           UpdateContext* context) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (FetchSuccess(*source)) {
     std::string xml;
     source->GetResponseAsString(&xml);
@@ skipping 186 matching lines @@
   ScheduleNextRun(false);
 }
 
 // The component update factory. Using the component updater as a singleton
 // is the job of the browser process.
 ComponentUpdateService* ComponentUpdateServiceFactory(
     ComponentUpdateService::Configurator* config) {
   DCHECK(config);
   return new CrxUpdateService(config);
 }