Add support for uninstalling components and use it in SupervisedUserWhitelistInstaller.

components/component_updater/component_updater_service.cc

Index: components/component_updater/component_updater_service.cc
diff --git a/components/component_updater/component_updater_service.cc b/components/component_updater/component_updater_service.cc
index 89195c482f203701105671039219ff05c33fe8ee..bcd324a4ca87468775359fe1555efc38585de23a 100644
--- a/components/component_updater/component_updater_service.cc
+++ b/components/component_updater/component_updater_service.cc
@@ -100,6 +100,7 @@ class CrxUpdateService : public ComponentUpdateService, public OnDemandUpdater {
   Status Start() override;
   Status Stop() override;
   Status RegisterComponent(const CrxComponent& component) override;
+  Status UnregisterComponent(const std::string& crx_id) override;
   std::vector<std::string> GetComponentIDs() const override;
   OnDemandUpdater& GetOnDemandUpdater() override;
   void MaybeThrottle(const std::string& crx_id,
@@ -461,6 +462,27 @@ ComponentUpdateService::Status CrxUpdateService::RegisterComponent(
   return kOk;
 }
 
+ComponentUpdateService::Status CrxUpdateService::UnregisterComponent(
+    const std::string& crx_id) {
+  UpdateItems::iterator it = std::find_if(
+      work_items_.begin(), work_items_.end(), CrxUpdateItem::FindById(crx_id));
+  if (it == work_items_.end())
+    return kError;
+
+  CrxUpdateItem* item = *it;
+  CrxUpdateItem::Status status = item->status;
+  if (status == CrxUpdateItem::kDownloading ||
+      status == CrxUpdateItem::kDownloadingDiff ||
+      status == CrxUpdateItem::kUpdating ||
+      status == CrxUpdateItem::kUpdatingDiff) {
+    ChangeItemState(item, CrxUpdateItem::kNoUpdate);
+    ping_manager_->OnUpdateComplete(item);
+  }
+
+  work_items_.erase(it);

[Sorin Jianu, 2015/02/05 22:55:35]

We wonder if we could do this in a different way. The idea here is that with the
current design, we'd like to avoid removing items from work_items_, and mutating
the state of the item in this function. Basically, we don't know what else could
or would break in the rest of the code, client-side and server-side included.

For instance, in this case, we change the state of the item to indicate
kNoUpdate, but since an update was available and downloaded, the server expects
a positive ping response back, as we are computing update metrics based on that.
Messing with the state of the item is undesirable.

We suggest that this function just marks the item as unregistered, and the clean
up of work items happens in a different place, where there are no side-effects.
We'll tell you where to possibly hook the code up.'

CrxUpdateItem can maintain a boolean flag to remember the state of the item:
active or unregistered.

[Bernhard Bauer, 2015/02/05 23:57:35]

Okay, that makes sense. I'll try to work on that tomorrow.

+  return kOk;
+}
+
 std::vector<std::string> CrxUpdateService::GetComponentIDs() const {
   DCHECK(thread_checker_.CalledOnValidThread());
   std::vector<std::string> component_ids;
@@ -781,6 +803,23 @@ void CrxUpdateService::DownloadComplete(
   DCHECK(thread_checker_.CalledOnValidThread());
 
   CrxUpdateItem* crx = FindUpdateItemById(crx_context->id);
+
+  // If the item has been removed in the mean time, clean up and return.

[Sorin Jianu, 2015/02/05 22:55:35]

The code to clean up an item can be hooked up in the ProcessPendingItems, which
runs a loop to filter the work_items and act accordingly: update, unregister, or
kick off an update check.

[Bernhard Bauer, 2015/02/06 16:05:05]

Done.

+  if (!crx) {
+    crx_downloader_.reset();
+    base::FilePath crx_path = download_result.response;
+    if (!crx_path.empty()) {
+      blocking_task_runner_->PostTask(
+          FROM_HERE,
+          base::Bind(base::IgnoreResult(
+                         &update_client::DeleteFileAndEmptyParentDirectory),
+                     crx_path));
+    }
+    // Move on to the next update, if there is one available.
+    ScheduleNextRun(kStepDelayMedium);
+    return;
+  }
+
   DCHECK(crx->status == CrxUpdateItem::kDownloadingDiff ||
          crx->status == CrxUpdateItem::kDownloading);
 
@@ -895,6 +934,13 @@ void CrxUpdateService::DoneInstalling(const std::string& component_id,
   const bool is_success = error == ComponentUnpacker::kNone;
 
   CrxUpdateItem* item = FindUpdateItemById(component_id);
+
+  // If the item has been removed in the mean time, return.

[Sorin Jianu, 2015/02/05 22:55:35]

Same. The suggest is to centralize the unregistration code in the
ProcessPendingItems, and avoid handling it in different states of the state
machine.

[Bernhard Bauer, 2015/02/06 16:05:05]

Done.

+  if (!item) {
+    ScheduleNextRun(kStepDelayMedium);
+    return;
+  }
+
   if (item->status == CrxUpdateItem::kUpdatingDiff && !is_success) {
     item->diff_error_category = error_category;
     item->diff_error_code = error;