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

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 ed3d0752312512eb0ada958f1f834caa146a2ba9..7a73ef50619cb54272484a2ec2f17b8c32cb95b7 100644
--- a/chrome/browser/component_updater/component_updater_service.cc
+++ b/chrome/browser/component_updater/component_updater_service.cc
@@ -196,6 +196,16 @@ struct CrxUpdateItem {
     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;
@@ -218,6 +228,30 @@ struct CrxUpdateItem {
   };
 };
 
+// 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;
@@ -253,6 +287,7 @@ class CrxUpdateService : public 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
@@ -323,7 +358,12 @@ class CrxUpdateService : public ComponentUpdateService {
 
   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);
 
@@ -341,9 +381,12 @@ class CrxUpdateService : public ComponentUpdateService {
 
   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_;
@@ -396,10 +439,13 @@ ComponentUpdateService::Status CrxUpdateService::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());
@@ -409,9 +455,61 @@ void CrxUpdateService::ScheduleNextRun(bool step_delay) {
   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
+        }
+    }
+  }
+
+  int64 delay = (step_status == CrxUpdateItem::kPrevInProgress)
+      ? config_->StepDelay() : config_->NextCheckDelay();
 
-  if (!step_delay) {
+  if (step_status != CrxUpdateItem::kPrevInProgress) {
     content::NotificationService::current()->Notify(
         chrome::NOTIFICATION_COMPONENT_UPDATER_SLEEPING,
         content::Source<ComponentUpdateService>(this),
@@ -503,14 +601,73 @@ bool CrxUpdateService::AddItemToUpdateCheck(CrxUpdateItem* item,
   return true;
 }
 
-// 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.
+// 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::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;
@@ -533,8 +690,8 @@ void CrxUpdateService::ProcessPendingItems() {
     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;
@@ -550,8 +707,8 @@ void CrxUpdateService::ProcessPendingItems() {
     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))
@@ -567,8 +724,8 @@ void CrxUpdateService::ProcessPendingItems() {
 
     // 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;
@@ -599,10 +756,10 @@ void CrxUpdateService::ProcessPendingItems() {
   }
 
   // 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) {
@@ -694,7 +851,9 @@ void CrxUpdateService::OnParseUpdateManifestSucceeded(
   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(
@@ -704,7 +863,7 @@ void CrxUpdateService::OnParseUpdateManifestFailed(
                                   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.
@@ -721,7 +880,7 @@ void CrxUpdateService::OnURLFetchComplete(const net::URLFetcher* source,
     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));
@@ -796,7 +955,9 @@ void CrxUpdateService::DoneInstalling(const std::string& component_id,
   }
 
   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