Add support for uninstalling components and use it in SupervisedUserWhitelistInstaller.

components/component_updater/component_updater_service.cc

 // Copyright 2014 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 "components/component_updater/component_updater_service.h"
 
 #include <algorithm>
 #include <set>
 #include <vector>
 
@@ skipping 82 matching lines @@
  public:
   explicit CrxUpdateService(Configurator* config);
   ~CrxUpdateService() override;
 
   // Overrides for ComponentUpdateService.
   void AddObserver(Observer* observer) override;
   void RemoveObserver(Observer* observer) override;
   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,
                      const base::Closure& callback) override;
   scoped_refptr<base::SequencedTaskRunner> GetSequencedTaskRunner() override;
 
   // Context for a crx download url request.
   struct CRXContext {
     scoped_refptr<ComponentInstaller> installer;
     std::vector<uint8_t> pk_hash;
@@ skipping 341 matching lines @@
         timer_.Start(FROM_HERE,
                      base::TimeDelta::FromSeconds(config_->InitialDelay()),
                      this,
                      &CrxUpdateService::ProcessPendingItems);
     }
   }
 
   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;
   for (UpdateItems::const_iterator it = work_items_.begin();
        it != work_items_.end();
        ++it) {
     const CrxUpdateItem* item = *it;
     component_ids.push_back(item->id);
   }
   return component_ids;
@@ skipping 42 matching lines @@
 // at a time if updates are available. Otherwise, it does an update check or
 // takes a long sleep until the loop runs again.
 void CrxUpdateService::ProcessPendingItems() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   CrxUpdateItem* ready_upgrade = FindReadyComponent();
   if (ready_upgrade) {
     UpdateComponent(ready_upgrade);
     return;
   }
 

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

This is where we could inspect the state of the items which were unregistered,
and clean up.

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

Done.

   if (!CheckForUpdates())
     ScheduleNextRun(kStepDelayLong);
 }
 
 CrxUpdateItem* CrxUpdateService::FindReadyComponent() const {
   class Helper {
    public:
     static bool IsReadyOnDemand(CrxUpdateItem* item) {
       return item->on_demand && IsReady(item);
     }
@@ skipping 237 matching lines @@
 
 // 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::DownloadComplete(
     scoped_ptr<CRXContext> crx_context,
     const CrxDownloader::Result& download_result) {
   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);
 
   AppendDownloadMetrics(crx_downloader_->download_metrics(),
                         &crx->download_metrics);
 
   crx_downloader_.reset();
 
   if (download_result.error) {
     if (crx->status == CrxUpdateItem::kDownloadingDiff) {
@@ skipping 94 matching lines @@
       error_category = kInstallError;
       break;
     default:
       error_category = kUnpackError;
       break;
   }
 
   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;
     item->diff_extra_code1 = extra_code;
     item->diff_update_failed = true;
     size_t count = ChangeItemStatus(CrxUpdateItem::kUpdatingDiff,
                                     CrxUpdateItem::kCanUpdate);
     DCHECK_EQ(count, 1ul);
     ScheduleNextRun(kStepDelayShort);
     return;
@@ skipping 94 matching lines @@
 ///////////////////////////////////////////////////////////////////////////////
 
 // The component update factory. Using the component updater as a singleton
 // is the job of the browser process.
 ComponentUpdateService* ComponentUpdateServiceFactory(Configurator* config) {
   DCHECK(config);
   return new CrxUpdateService(config);
 }
 
 }  // namespace component_updater