Add an API to component_updater that asks to do an update check "now".

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 178 matching lines @@
     kChecking,
     kCanUpdate,
     kDownloading,
     kUpdating,
     kUpdated,
     kUpToDate,
     kNoUpdate,
     kLastStatus
   };
 
+  // A value that summarizes the result of a previously scheduled step.
+  // Essentially, these are the labels on the edges in the above diagram.
+  enum PrevStepStatus {
+    kPrevUnknown,
+    kPrevInProgress,
+    kPrevNoUpdate,
+    kPrevError,
+    kPrevSuccess
+  };
+
   Status status;
   GURL crx_url;
   std::string id;
   base::Time last_check;
   CrxComponent component;
   Version next_version;
 
   CrxUpdateItem() : status(kNew) {}
 
   // Function object used to find a specific component.
   class FindById {
    public:
     explicit FindById(const std::string& id) : id_(id) {}
 
     bool operator() (CrxUpdateItem* item) const {
       return (item->id == id_);
     }
    private:
     const std::string& id_;
   };
 };
 
+// This represents a CrxUpdateItem which a client asked to be checked ASAP.
+//
+// The item is initially in the "queued" state until ScheduleNextRun()
+// is called again. During the next ScheduleNextRun(), it will transition to
+// the "checking" state, and the service will check for an update only
+// for the particular RequestedCrxUpdateItem. This interrupts the update
+// cycle of other work items.
+//
+// When a full cycle of ScheduleNextRun()s completes successfully
+// or with an error, we delete the RequestedCrxUpdateItem and resume
+// the cycle of ScheduleNextRun()s for the interrupted work items.
+// To accurately determine the amount of delay for resuming the interrupted
+// task, we record the kind of step that was interrupted.
+struct RequestedCrxUpdateItem {
+  enum Status {
+    kQueued,
+    kChecking
+  };
+
+  Status status;
+  CrxUpdateItem* item;
+  CrxUpdateItem::PrevStepStatus interrupted_step_status;
+};
+
 }  // namespace.
 
 typedef ComponentUpdateService::Configurator Config;
 
 CrxComponent::CrxComponent()
     : installer(NULL),
       source(BANDAID) {
 }
 
 CrxComponent::~CrxComponent() {
@@ skipping 15 matching lines @@
 class CrxUpdateService : public ComponentUpdateService {
  public:
   explicit CrxUpdateService(ComponentUpdateService::Configurator* config);
 
   virtual ~CrxUpdateService();
 
   // Overrides for ComponentUpdateService.
   virtual Status Start() OVERRIDE;
   virtual Status Stop() OVERRIDE;
   virtual Status RegisterComponent(const CrxComponent& component) OVERRIDE;
+  virtual Status CheckForUpdateSoon(const CrxComponent& component) OVERRIDE;
 
   // The only purpose of this class is to forward the
   // UtilityProcessHostClient callbacks so CrxUpdateService does
   // not have to derive from it because that is refcounted.
   class ManifestParserBridge : public UtilityProcessHostClient {
    public:
     explicit ManifestParserBridge(CrxUpdateService* service)
         : service_(service) {}
 
     virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE {
@@ skipping 50 matching lines @@
       const UpdateManifest::Results& results);
 
   // See ManifestParserBridge.
   void OnParseUpdateManifestFailed(
       const std::string& error_message);
 
   bool AddItemToUpdateCheck(CrxUpdateItem* item, std::string* query);
 
   void ProcessPendingItems();
 
-  void ScheduleNextRun(bool step_delay);
+  void ProcessRequestedItem();
+
+  typedef std::vector<CrxUpdateItem*> UpdateItems;
+  void ProcessWorkItems(const UpdateItems& work_items);
+
+  void ScheduleNextRun(CrxUpdateItem::PrevStepStatus prev_status);
 
   void ParseManifest(const std::string& xml);
 
   void Install(const CRXContext* context, const FilePath& crx_path);
 
   void DoneInstalling(const std::string& component_id,
                       ComponentUnpacker::Error error);
 
   size_t ChangeItemStatus(CrxUpdateItem::Status from,
                           CrxUpdateItem::Status to);
 
   CrxUpdateItem* FindUpdateItemById(const std::string& id);
 
   scoped_ptr<Config> config_;
 
   scoped_ptr<net::URLFetcher> url_fetcher_;
 
-  typedef std::vector<CrxUpdateItem*> UpdateItems;
+  // A collection of every work item.
   UpdateItems work_items_;
 
+  // A particular work item from work_items_, which should be checked ASAP.
+  scoped_ptr<RequestedCrxUpdateItem> requested_work_item_;
+
   base::OneShotTimer<CrxUpdateService> timer_;
 
   Version chrome_version_;
 
   bool running_;
 
   DISALLOW_COPY_AND_ASSIGN(CrxUpdateService);
 };
 
 //////////////////////////////////////////////////////////////////////////////
@@ skipping 32 matching lines @@
 }
 
 // Stop the main check + update loop. In flight operations will be
 // completed.
 ComponentUpdateService::Status CrxUpdateService::Stop() {
   running_ = false;
   timer_.Stop();
   return kOk;
 }
 
-// This function sets the timer which will call ProcessPendingItems() there
-// are two kind of waits, the short one (with step_delay = true) and the
-// long one.
-void CrxUpdateService::ScheduleNextRun(bool step_delay) {
+// This function sets the timer which will call ProcessPendingItems() or
+// ProcessRequestedItem() if there is an important requested item.  There
+// are two kinds of waits: a short step_delay (when step_status is
+// kPrevInProgress) and a long one when a full check/update cycle
+// has completed either successfully or with an error.
+void CrxUpdateService::ScheduleNextRun(
+    CrxUpdateItem::PrevStepStatus step_status) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(url_fetcher_.get() == NULL);
   CHECK(!timer_.IsRunning());
   // It could be the case that Stop() had been called while a url request
   // or unpacking was in flight, if so we arrive here but |running_| is
   // false. In that case do not loop again.
   if (!running_)
     return;
 
-  int64 delay = step_delay ? config_->StepDelay() : config_->NextCheckDelay();
+  // If there is a requested_work_item_, process just that one item
+  // (not the others).

[cpu_(ooo_6.6-7.5), 2013/02/05 00:04:47]

this again in terms of fairness.

+  if (requested_work_item_) {
+    switch (requested_work_item_->status) {
+      case RequestedCrxUpdateItem::kQueued:
+        requested_work_item_->interrupted_step_status = step_status;
+        requested_work_item_->status = RequestedCrxUpdateItem::kChecking;
+        timer_.Start(FROM_HERE,
+                     base::TimeDelta::FromSeconds(config_->StepDelay()),
+                     this, &CrxUpdateService::ProcessRequestedItem);
+        return;
+      case RequestedCrxUpdateItem::kChecking:
+        if (step_status == CrxUpdateItem::kPrevInProgress) {
+          // Keep hammering on this particular item.
+          timer_.Start(FROM_HERE,
+                       base::TimeDelta::FromSeconds(config_->StepDelay()),
+                       this, &CrxUpdateService::ProcessRequestedItem);
+          return;
+        } else {
+          // This particular item either finished successfully or hit an error.
+          switch (step_status) {
+            case CrxUpdateItem::kPrevSuccess:
+              content::NotificationService::current()->Notify(
+                  chrome::NOTIFICATION_COMPONENT_UPDATE_COMPLETE,

[cpu_(ooo_6.6-7.5), 2013/02/05 00:04:47]

not sure we need this one, I mean the installer got called so the installer is
the one to know.

[jvoung (off chromium), 2013/02/05 00:17:56]

I imagine that it is the "thing" that called CheckForUpdateSoon(), isn't
necessarily the installer (see the other CL). It makes sense to notify whoever
called CheckForUpdateSoon().

Or, should everything go through the installer, even if the installer is the one
that notifies others?

[cpu_(ooo_6.6-7.5), 2013/02/06 22:25:32]

That would be ideal, that way you don't have to have code that filters for your
notification.

In general the notifications that are in this code are not doing anything except
helping tests, probably I over designed that part.

[jvoung (off chromium), 2013/02/11 21:04:26]

Alright, removed the use of notification service.  Will have to think of
something else to notify a user of errors during kDownloading/kUpdating, since
there is also not a guarantee that the requested items go beyond the
kChecking/kCanUpdate stages, since only one of the kCanUpdate items get updated
at once?

+                  content::Source<std::string>(&requested_work_item_->item->id),
+                  content::NotificationService::NoDetails());
+              break;
+            case CrxUpdateItem::kPrevNoUpdate:
+              content::NotificationService::current()->Notify(
+                  chrome::NOTIFICATION_COMPONENT_UPDATE_NOT_UPDATED,
+                  content::Source<std::string>(&requested_work_item_->item->id),
+                  content::NotificationService::NoDetails());
+              break;
+            case CrxUpdateItem::kPrevError:
+              content::NotificationService::current()->Notify(
+                  chrome::NOTIFICATION_COMPONENT_UPDATE_FAILED,
+                  content::Source<std::string>(&requested_work_item_->item->id),
+                  content::NotificationService::NoDetails());
+              break;
+            default:
+              NOTREACHED() << "Unexpected step status: " << step_status;
+          }
+          // Delete the requested item and proceed.
+          step_status = requested_work_item_->interrupted_step_status;
+          DCHECK(step_status != CrxUpdateItem::kPrevUnknown);
+          requested_work_item_.reset(NULL);
+          // fall through
+        }
+    }
+  }
 
-  if (!step_delay) {
+  int64 delay = (step_status == CrxUpdateItem::kPrevInProgress)
+      ? config_->StepDelay() : config_->NextCheckDelay();
+
+  if (step_status != CrxUpdateItem::kPrevInProgress) {
     content::NotificationService::current()->Notify(
         chrome::NOTIFICATION_COMPONENT_UPDATER_SLEEPING,
         content::Source<ComponentUpdateService>(this),
         content::NotificationService::NoDetails());
     // Zero is only used for unit tests.
     if (0 == delay)
       return;
   }
 
   timer_.Start(FROM_HERE, base::TimeDelta::FromSeconds(delay),
@@ skipping 71 matching lines @@
                                             std::string* query) {
   if (!AddQueryString(item->id,
                       item->component.version.GetString(),
                       config_->UrlSizeLimit(), query))
     return false;
   item->status = CrxUpdateItem::kChecking;
   item->last_check = base::Time::Now();
   return true;
 }
 
-// Here is where the work gets scheduled. Given that our |work_items_| list
+// Here is where the work gets scheduled.
+void CrxUpdateService::ProcessPendingItems() {
+  ProcessWorkItems(work_items_);
+}
+
+void CrxUpdateService::ProcessRequestedItem() {
+  UpdateItems one_item;
+  one_item.push_back(requested_work_item_->item);
+  ProcessWorkItems(one_item);
+}
+
+// Start the process of checking for an update, for a particular component
+// that was previously registered.  If the component does not exist return
+// kError, otherwise kOk.
+ComponentUpdateService::Status CrxUpdateService::CheckForUpdateSoon(
+    const CrxComponent& component) {
+  if (component.pk_hash.empty() ||
+      !component.version.IsValid() ||
+      !component.installer)
+    return kError;
+
+  std::string id =
+    HexStringToID(StringToLowerASCII(base::HexEncode(&component.pk_hash[0],
+                                     component.pk_hash.size()/2)));
+
+  CrxUpdateItem* uit;
+  uit = FindUpdateItemById(id);
+  if (!uit)
+    return kError;
+
+  // There is already a request submitted.
+  if (requested_work_item_) {
+    return kInProgress;
+  }

[cpu_(ooo_6.6-7.5), 2013/02/05 00:04:47]

hmm, we are trying to be fair here with all the components, I don't want that
one component can pre-empt all others.

[jvoung (off chromium), 2013/02/05 00:17:56]

Should the effect of the CL not be to "cut in line", but to keep doing short
step_delays, for everyone, until the requested item makes it to the front of the
line?

[cpu_(ooo_6.6-7.5), 2013/02/06 22:25:32]

Yeah, until the component changes from kNew

[jvoung (off chromium), 2013/02/11 21:04:26]

Done.

+
+  requested_work_item_.reset(new RequestedCrxUpdateItem());
+  requested_work_item_->status = RequestedCrxUpdateItem::kQueued;
+  requested_work_item_->item = uit;
+  requested_work_item_->interrupted_step_status = CrxUpdateItem::kPrevUnknown;
+
+  // If the delay is long, try to set the timer to a shorter value
+  // to get the ball rolling.
+  if (timer_.IsRunning()) {
+    base::TimeDelta remaining_delay =
+        timer_.desired_run_time() - base::TimeTicks::Now();
+    base::TimeDelta step_delay =
+        base::TimeDelta::FromSeconds(config_->StepDelay());
+    if (remaining_delay > step_delay) {
+      timer_.Stop();
+      // The only task it could have interrupted was a call to
+      // ProcessPendingItems, not ProcessRequestedItem.

[cpu_(ooo_6.6-7.5), 2013/02/05 00:04:47]

assuming that the config_ object returns constant values, which is something I
don't want unless is impossible to do otherwise, that is why I keep calling
config_ methods all the time instead of calling them once.

[jvoung (off chromium), 2013/02/05 00:17:56]

The step_delay that is saved to the variable is the one that was compared
against remaining_delay, though.  If config_ can return a different value, then
the comparison doesn't make sense?

Should I just always reset the timer and skip the comparison?

[cpu_(ooo_6.6-7.5), 2013/02/06 22:25:32]

Yes, that would be best.

[jvoung (off chromium), 2013/02/11 21:04:26]

Done.

+      timer_.Start(FROM_HERE, step_delay,
+                   this, &CrxUpdateService::ProcessPendingItems);
+    }
+  }
+
+  return kOk;
+}
+
+// Here is where the work gets scheduled. Given that our |work_items| list
 // is expected to be ten or less items, we simply loop several times.
-void CrxUpdateService::ProcessPendingItems() {
+void CrxUpdateService::ProcessWorkItems(const UpdateItems& work_items) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   // First check for ready upgrades and do one. The first
   // step is to fetch the crx package.
-  for (UpdateItems::const_iterator it = work_items_.begin();
-       it != work_items_.end(); ++it) {
+  for (UpdateItems::const_iterator it = work_items.begin();
+       it != work_items.end(); ++it) {
     CrxUpdateItem* item = *it;
     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;
     url_fetcher_.reset(net::URLFetcher::Create(
         0, item->crx_url, net::URLFetcher::GET,
         MakeContextDelegate(this, context)));
     StartFetch(url_fetcher_.get(), config_->RequestContext(), true);
     return;
   }
 
   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 if there are new components we have not
     // checked against the server. We can batch some in a single url request.
-    for (UpdateItems::const_iterator it = work_items_.begin();
-         it != work_items_.end(); ++it) {
+    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) {
+    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 as
     // we have not checked them recently.
-    for (UpdateItems::const_iterator it = work_items_.begin();
-         it != work_items_.end(); ++it) {
+    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;
@@ skipping 10 matching lines @@
                        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);
     return;
   }
 
   // No components to update. Next check after the long sleep.
-  ScheduleNextRun(false);
+  ScheduleNextRun(CrxUpdateItem::kPrevNoUpdate);
 }
 
-// Caled when we got a response from the update server. It consists of an xml
+// Called 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);
     url_fetcher_.reset();
     ParseManifest(xml);
   } else {
@@ skipping 71 matching lines @@
         chrome::NOTIFICATION_COMPONENT_UPDATE_FOUND,
         content::Source<std::string>(&crx->id),
         content::NotificationService::NoDetails());
   }
 
   // All the components that are not mentioned in the manifest we
   // consider them up to date.
   ChangeItemStatus(CrxUpdateItem::kChecking, CrxUpdateItem::kUpToDate);
 
   // If there are updates pending we do a short wait.
-  ScheduleNextRun(update_pending > 0);
+  ScheduleNextRun(update_pending > 0
+                  ? CrxUpdateItem::kPrevInProgress
+                  : CrxUpdateItem::kPrevNoUpdate);
 }
 
 void CrxUpdateService::OnParseUpdateManifestFailed(
     const std::string& error_message) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   size_t count = ChangeItemStatus(CrxUpdateItem::kChecking,
                                   CrxUpdateItem::kNoUpdate);
   config_->OnEvent(Configurator::kManifestError, static_cast<int>(count));
   DCHECK_GT(count, 0ul);
-  ScheduleNextRun(false);
+  ScheduleNextRun(CrxUpdateItem::kPrevError);
 }
 
 // Called when the CRX package has been downloaded to a temporary location.
 // Here we fire the notifications and schedule the component-specific installer
 // to be called in the file thread.
 void CrxUpdateService::OnURLFetchComplete(const net::URLFetcher* source,
                                           CRXContext* context) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   base::PlatformFileError error_code;
 
   if (source->FileErrorOccurred(&error_code) || !FetchSuccess(*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);
+    ScheduleNextRun(CrxUpdateItem::kPrevError);
   } else {
     FilePath temp_crx_path;
     CHECK(source->GetResponseAsFilePath(true, &temp_crx_path));
     size_t count = ChangeItemStatus(CrxUpdateItem::kDownloading,
                                     CrxUpdateItem::kUpdating);
     DCHECK_EQ(count, 1ul);
     url_fetcher_.reset();
 
     content::NotificationService::current()->Notify(
         chrome::NOTIFICATION_COMPONENT_UPDATE_READY,
@@ skipping 54 matching lines @@
       break;
     case ComponentUnpacker::kInstallerError:
       event = Configurator::kInstallerError;
       break;
     default:
       event = Configurator::kUnpackError;
       break;
   }
 
   config_->OnEvent(event, CrxIdtoUMAId(component_id));
-  ScheduleNextRun(false);
+  ScheduleNextRun(error == ComponentUnpacker::kNone
+                  ? CrxUpdateItem::kPrevSuccess
+                  : CrxUpdateItem::kPrevError);
 }
 
 // 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);
 }