Add support for uninstalling components and use it in SupervisedUserWhitelistInstaller.

TBR=agl@chromium.org

BUG=436459

Review URL: https://codereview.chromium.org/879993005

Cr-Commit-Position: refs/heads/master@{#315246}

chrome/browser/component_updater/pepper_flash_component_installer.cc

@@ -344,6 +344,8 @@ class PepperFlashComponentInstaller : public update_client::ComponentInstaller {
   bool GetInstalledFile(const std::string& file,
                         base::FilePath* installed_file) override;
 
+  bool Uninstall() override;
+
  private:
   ~PepperFlashComponentInstaller() override {}
 
@@ -395,6 +397,10 @@ bool PepperFlashComponentInstaller::GetInstalledFile(
   return false;
 }
 
+bool PepperFlashComponentInstaller::Uninstall() {
+  return false;
+}
+
 bool CheckPepperFlashManifest(const base::DictionaryValue& manifest,
                               Version* version_out) {
   std::string name;

chrome/browser/component_updater/pnacl/pnacl_component_installer.cc

@@ -280,6 +280,10 @@ bool PnaclComponentInstaller::GetInstalledFile(const std::string& file,
   return true;
 }
 
+bool PnaclComponentInstaller::Uninstall() {
+  return false;
+}
+
 CrxComponent PnaclComponentInstaller::GetCrxComponent() {
   CrxComponent pnacl_component;
   pnacl_component.version = current_version();

chrome/browser/component_updater/pnacl/pnacl_component_installer.h

@@ -43,12 +43,11 @@ class PnaclComponentInstaller : public update_client::ComponentInstaller {
 
   // ComponentInstaller implementation:
   void OnUpdateError(int error) override;
-
   bool Install(const base::DictionaryValue& manifest,
                const base::FilePath& unpack_path) override;
-
   bool GetInstalledFile(const std::string& file,
                         base::FilePath* installed_file) override;
+  bool Uninstall() override;
 
   // Register a PNaCl component for the first time.
   void RegisterPnaclComponent(ComponentUpdateService* cus);

chrome/browser/component_updater/recovery_component_installer.cc

@@ -186,6 +186,8 @@ class RecoveryComponentInstaller : public update_client::ComponentInstaller {
   bool GetInstalledFile(const std::string& file,
                         base::FilePath* installed_file) override;
 
+  bool Uninstall() override;
+
  private:
   ~RecoveryComponentInstaller() override {}
 
@@ -360,6 +362,10 @@ bool RecoveryComponentInstaller::GetInstalledFile(
   return false;
 }
 
+bool RecoveryComponentInstaller::Uninstall() {
+  return false;
+}
+
 void RegisterRecoveryComponent(ComponentUpdateService* cus,
                                PrefService* prefs) {
 #if !defined(OS_CHROMEOS)

chrome/browser/component_updater/supervised_user_whitelist_installer.cc

@@ -121,7 +121,7 @@ class SupervisedUserWhitelistInstallerImpl
                          const std::string& name,
                          bool newly_added,
                          const WhitelistReadyCallback& callback) override;
-  void UnregisterWhitelist(const std::string& crx_id) override;
+  void UninstallWhitelist(const std::string& crx_id) override;
 
   ComponentUpdateService* cus_;
 };
@@ -141,17 +141,17 @@ void SupervisedUserWhitelistInstallerImpl::RegisterWhitelist(
                                                           callback));
   scoped_refptr<DefaultComponentInstaller> installer(
       new DefaultComponentInstaller(traits.Pass()));
-
   installer->Register(cus_);
 
   if (newly_added)
     TriggerComponentUpdate(&cus_->GetOnDemandUpdater(), crx_id);
 }
 
-void SupervisedUserWhitelistInstallerImpl::UnregisterWhitelist(
-    const std::string& id) {
-  // TODO(bauerb): Implement!
-  NOTIMPLEMENTED();
+void SupervisedUserWhitelistInstallerImpl::UninstallWhitelist(
+    const std::string& crx_id) {
+  const ComponentUpdateService::Status status =
+      cus_->UnregisterComponent(crx_id);
+  DCHECK_EQ(ComponentUpdateService::kOk, status);
 }
 
 }  // namespace

chrome/browser/component_updater/supervised_user_whitelist_installer.h

@@ -47,8 +47,8 @@ class SupervisedUserWhitelistInstaller {
                                  bool new_installation,
                                  const WhitelistReadyCallback& callback) = 0;
 
-  // Unregisters a whitelist.
-  virtual void UnregisterWhitelist(const std::string& crx_id) = 0;
+  // Uninstalls a whitelist.
+  virtual void UninstallWhitelist(const std::string& crx_id) = 0;
 
  protected:
   // Triggers an update for a whitelist to be installed. Protected so it can be

chrome/browser/component_updater/swiftshader_component_installer.cc

@@ -109,6 +109,8 @@ class SwiftShaderComponentInstaller : public update_client::ComponentInstaller {
   bool GetInstalledFile(const std::string& file,
                         base::FilePath* installed_file) override;
 
+  bool Uninstall() override;
+
  private:
   ~SwiftShaderComponentInstaller() override {}
 
@@ -163,6 +165,10 @@ bool SwiftShaderComponentInstaller::GetInstalledFile(
   return false;
 }
 
+bool SwiftShaderComponentInstaller::Uninstall() {
+  return false;
+}
+
 void FinishSwiftShaderUpdateRegistration(ComponentUpdateService* cus,
                                          const Version& version) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

chrome/browser/component_updater/test/supervised_user_whitelist_installer_unittest.cc

@@ -96,6 +96,22 @@ class MockComponentUpdateService : public ComponentUpdateService {
     return kOk;
   }
 
+  Status UnregisterComponent(const std::string& crx_id) override {
+    if (!component_) {
+      ADD_FAILURE();
+      return kError;
+    }
+
+    EXPECT_EQ(GetCrxComponentID(*component_), crx_id);
+    if (!component_->installer->Uninstall()) {
+      ADD_FAILURE();
+      return kError;
+    }
+
+    component_.reset();
+    return kOk;
+  }
+
   OnDemandUpdater& GetOnDemandUpdater() override { return on_demand_updater_; }
 
   void MaybeThrottle(const std::string& kCrxId,
@@ -241,16 +257,27 @@ TEST_F(SupervisedUserWhitelistInstallerTest, InstallNewWhitelist) {
   EXPECT_EQ(kWhitelistContents, whitelist_contents);
 }
 
-TEST_F(SupervisedUserWhitelistInstallerTest, RegisterExistingWhitelist) {
+TEST_F(SupervisedUserWhitelistInstallerTest,
+       RegisterAndUninstallExistingWhitelist) {
   ASSERT_TRUE(base::CreateDirectory(whitelist_directory_));
   ASSERT_NO_FATAL_FAILURE(PrepareWhitelistDirectory(whitelist_directory_));
 
-  base::RunLoop run_loop;
-  bool new_installation = false;
-  LoadWhitelist(new_installation, run_loop.QuitClosure());
-  run_loop.Run();
+  {
+    base::RunLoop run_loop;
+    bool new_installation = false;
+    LoadWhitelist(new_installation, run_loop.QuitClosure());
+    run_loop.Run();
+  }
 
   ASSERT_NO_FATAL_FAILURE(CheckRegisteredComponent(kVersion));
+
+  {
+    base::RunLoop run_loop;
+    installer_->UninstallWhitelist(kCrxId);
+    run_loop.RunUntilIdle();
+  }
+  EXPECT_FALSE(component_update_service_.registered_component());
+  EXPECT_FALSE(base::DirectoryExists(whitelist_directory_));
 }
 
 }  // namespace component_updater

chrome/browser/net/crl_set_fetcher.cc

@@ -241,4 +241,8 @@ bool CRLSetFetcher::GetInstalledFile(
   return false;
 }
 
+bool CRLSetFetcher::Uninstall() {
+  return false;
+}
+
 CRLSetFetcher::~CRLSetFetcher() {}

chrome/browser/net/crl_set_fetcher.h

@@ -41,6 +41,7 @@ class CRLSetFetcher : public update_client::ComponentInstaller {
                const base::FilePath& unpack_path) override;
   bool GetInstalledFile(const std::string& file,
                         base::FilePath* installed_file) override;
+  bool Uninstall() override;
 
  private:
   friend class base::RefCountedThreadSafe<CRLSetFetcher>;

chrome/browser/supervised_user/supervised_user_whitelist_service.cc

@@ -281,7 +281,7 @@ void SupervisedUserWhitelistService::RemoveWhitelist(
   base::RecordAction(base::UserMetricsAction("ManagedUsers_Whitelist_Removed"));
 
   pref_dict->RemoveWithoutPathExpansion(id, NULL);
-  installer_->UnregisterWhitelist(id);
+  installer_->UninstallWhitelist(id);
   UnloadWhitelist(id);
 }
 

chrome/browser/supervised_user/supervised_user_whitelist_service_unittest.cc

@@ -56,10 +56,10 @@ class MockSupervisedUserWhitelistInstaller
     ready_callbacks_[crx_id] = callback;
   }
 
-  void UnregisterWhitelist(const std::string& crx_id) override {
+  void UninstallWhitelist(const std::string& crx_id) override {
     EXPECT_TRUE(WhitelistIsRegistered(crx_id)) << crx_id;
     registered_whitelists_.erase(crx_id);
-    // Don't remove the callback (see above).
+    // Don't remove the ready callback (see above).
   }
 
  private:

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,
@@ -154,6 +155,9 @@ class CrxUpdateService : public ComponentUpdateService, public OnDemandUpdater {
 
   void ProcessPendingItems();
 
+  // Uninstall and remove all unregistered work items.
+  void UninstallUnregisteredItems();
+
   // Find a component that is ready to update.
   CrxUpdateItem* FindReadyComponent() const;
 
@@ -432,6 +436,7 @@ ComponentUpdateService::Status CrxUpdateService::RegisterComponent(
   CrxUpdateItem* uit = FindUpdateItemById(id);
   if (uit) {
     uit->component = component;
+    uit->unregistered = false;
     return kReplaced;
   }
 
@@ -461,6 +466,19 @@ ComponentUpdateService::Status CrxUpdateService::RegisterComponent(
   return kOk;
 }
 
+ComponentUpdateService::Status CrxUpdateService::UnregisterComponent(
+    const std::string& crx_id) {
+  auto it = std::find_if(work_items_.begin(), work_items_.end(),
+                         CrxUpdateItem::FindById(crx_id));
+  if (it == work_items_.end())
+    return kError;
+
+  (*it)->unregistered = true;
+
+  ScheduleNextRun(kStepDelayShort);
+  return kOk;
+}
+
 std::vector<std::string> CrxUpdateService::GetComponentIDs() const {
   DCHECK(thread_checker_.CalledOnValidThread());
   std::vector<std::string> component_ids;
@@ -492,7 +510,7 @@ void CrxUpdateService::MaybeThrottle(const std::string& crx_id,
 
 scoped_refptr<base::SequencedTaskRunner>
 CrxUpdateService::GetSequencedTaskRunner() {
-  return config_->GetSequencedTaskRunner();
+  return blocking_task_runner_;
 }
 
 bool CrxUpdateService::GetComponentDetails(const std::string& component_id,
@@ -524,10 +542,26 @@ void CrxUpdateService::ProcessPendingItems() {
     return;
   }
 
+  UninstallUnregisteredItems();
+
   if (!CheckForUpdates())
     ScheduleNextRun(kStepDelayLong);
 }
 
+void CrxUpdateService::UninstallUnregisteredItems() {
+  std::vector<CrxUpdateItem*> new_work_items;
+  for (CrxUpdateItem* item : work_items_) {
+    scoped_ptr<CrxUpdateItem> owned_item(item);
+    if (owned_item->unregistered) {
+      const bool success = owned_item->component.installer->Uninstall();
+      DCHECK(success);
+    } else {
+      new_work_items.push_back(owned_item.release());
+    }
+  }
+  new_work_items.swap(work_items_);
+}
+
 CrxUpdateItem* CrxUpdateService::FindReadyComponent() const {
   class Helper {
    public:
@@ -781,6 +815,7 @@ void CrxUpdateService::DownloadComplete(
   DCHECK(thread_checker_.CalledOnValidThread());
 
   CrxUpdateItem* crx = FindUpdateItemById(crx_context->id);
+
   DCHECK(crx->status == CrxUpdateItem::kDownloadingDiff ||
          crx->status == CrxUpdateItem::kDownloading);
 
@@ -895,6 +930,7 @@ void CrxUpdateService::DoneInstalling(const std::string& component_id,
   const bool is_success = error == ComponentUnpacker::kNone;
 
   CrxUpdateItem* item = FindUpdateItemById(component_id);
+
   if (item->status == CrxUpdateItem::kUpdatingDiff && !is_success) {
     item->diff_error_category = error_category;
     item->diff_error_code = error;

components/component_updater/component_updater_service.h

@@ -124,6 +124,14 @@ class ComponentUpdateService {
   virtual Status RegisterComponent(
       const update_client::CrxComponent& component) = 0;
 
+  // Unregisters the component with the given ID. This means that the component
+  // is not going to be included in future update checks. If a download or
+  // update operation for the component is currently in progress, it will
+  // silently finish without triggering the next step.
+  // Note that the installer for the component is responsible for removing any
+  // existing versions of the component from disk.
+  virtual Status UnregisterComponent(const std::string& crx_id) = 0;
+
   // Returns a list of registered components.
   virtual std::vector<std::string> GetComponentIDs() const = 0;
 

components/component_updater/default_component_installer.cc

@@ -17,16 +17,18 @@
 #include "components/component_updater/component_updater_service.h"
 #include "components/component_updater/default_component_installer.h"
 #include "components/update_client/component_unpacker.h"
-#include "components/update_client/update_client.h"
+#include "components/update_client/utils.h"
 
 using update_client::CrxComponent;
 
 namespace component_updater {
 
 namespace {
+
 // Version "0" corresponds to no installed version. By the server's conventions,
 // we represent it as a dotted quad.
 const char kNullVersion[] = "0.0.0.0";
+
 }  // namespace
 
 ComponentInstallerTraits::~ComponentInstallerTraits() {
@@ -40,7 +42,6 @@ DefaultComponentInstaller::DefaultComponentInstaller(
 }
 
 DefaultComponentInstaller::~DefaultComponentInstaller() {
-  DCHECK(thread_checker_.CalledOnValidThread());
 }
 
 void DefaultComponentInstaller::Register(ComponentUpdateService* cus) {
@@ -77,6 +78,8 @@ bool DefaultComponentInstaller::InstallHelper(
 
 bool DefaultComponentInstaller::Install(const base::DictionaryValue& manifest,
                                         const base::FilePath& unpack_path) {
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
+
   std::string manifest_version;
   manifest.GetStringASCII("version", &manifest_version);
   base::Version version(manifest_version.c_str());
@@ -119,6 +122,13 @@ bool DefaultComponentInstaller::GetInstalledFile(
   return true;
 }
 
+bool DefaultComponentInstaller::Uninstall() {
+  task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&DefaultComponentInstaller::UninstallOnTaskRunner, this));
+  return true;
+}
+
 void DefaultComponentInstaller::StartRegistration(ComponentUpdateService* cus) {
   DCHECK(task_runner_.get());
   DCHECK(task_runner_->RunsTasksOnCurrentThread());
@@ -196,6 +206,33 @@ void DefaultComponentInstaller::StartRegistration(ComponentUpdateService* cus) {
                  this, cus));
 }
 
+void DefaultComponentInstaller::UninstallOnTaskRunner() {
+  DCHECK(task_runner_.get());
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
+  const base::FilePath base_dir = installer_traits_->GetBaseDirectory();
+
+  base::FileEnumerator file_enumerator(base_dir, false,
+                                       base::FileEnumerator::DIRECTORIES);
+  for (base::FilePath path = file_enumerator.Next(); !path.value().empty();
+       path = file_enumerator.Next()) {
+    base::Version version(path.BaseName().MaybeAsASCII());
+
+    // Ignore folders that don't have valid version names. These folders are not
+    // managed by the component installer, so do not try to remove them.
+    if (!version.IsValid())
+      continue;
+
+    if (!base::DeleteFile(path, true))
+      DLOG(ERROR) << "Couldn't delete " << path.value();
+  }
+
+  // Delete the base directory if it's empty now.
+  if (base::IsDirectoryEmpty(base_dir)) {
+    if (base::DeleteFile(base_dir, false))
+      DLOG(ERROR) << "Couldn't delete " << base_dir.value();
+  }
+}
+
 base::FilePath DefaultComponentInstaller::GetInstallDirectory() {
   return installer_traits_->GetBaseDirectory()
       .AppendASCII(current_version_.GetString());

components/component_updater/default_component_installer.h

@@ -98,6 +98,7 @@ class DefaultComponentInstaller : public update_client::ComponentInstaller {
                const base::FilePath& unpack_path) override;
   bool GetInstalledFile(const std::string& file,
                         base::FilePath* installed_file) override;
+  bool Uninstall() override;
 
  private:
   ~DefaultComponentInstaller() override;
@@ -109,6 +110,7 @@ class DefaultComponentInstaller : public update_client::ComponentInstaller {
   void StartRegistration(ComponentUpdateService* cus);
   void FinishRegistration(ComponentUpdateService* cus);
   void ComponentReady(scoped_ptr<base::DictionaryValue> manifest);
+  void UninstallOnTaskRunner();
 
   base::Version current_version_;
   std::string current_fingerprint_;

components/update_client/crx_update_item.h

@@ -71,6 +71,10 @@ struct CrxUpdateItem {
   // enforce conditions or notify observers of the change.
   Status status;
 
+  // True if the component was recently unregistered and will be uninstalled
+  // soon (after the currently operation is finished, if there is one).
+  bool unregistered;
+
   std::string id;
   CrxComponent component;
 

components/update_client/test/test_installer.cc

@@ -33,6 +33,10 @@ bool TestInstaller::GetInstalledFile(const std::string& file,
 TestInstaller::~TestInstaller() {
 }
 
+bool TestInstaller::Uninstall() {
+  return false;
+}
+
 ReadOnlyTestInstaller::ReadOnlyTestInstaller(const base::FilePath& install_dir)
     : install_directory_(install_dir) {
 }

components/update_client/test/test_installer.h

@@ -31,6 +31,8 @@ class TestInstaller : public ComponentInstaller {
   bool GetInstalledFile(const std::string& file,
                         base::FilePath* installed_file) override;
 
+  bool Uninstall() override;
+
   int error() const {
     return error_;
   }

components/update_client/update_client.cc

@@ -10,6 +10,7 @@ namespace update_client {
 
 CrxUpdateItem::CrxUpdateItem()
     : status(kNew),
+      unregistered(false),
       on_demand(false),
       diff_update_failed(false),
       error_category(0),
@@ -23,8 +24,7 @@ CrxUpdateItem::CrxUpdateItem()
 CrxUpdateItem::~CrxUpdateItem() {
 }
 
-CrxComponent::CrxComponent()
-    : installer(NULL), allow_background_download(true) {
+CrxComponent::CrxComponent() : allow_background_download(true) {
 }
 
 CrxComponent::~CrxComponent() {

components/update_client/update_client.h

@@ -45,6 +45,11 @@ class ComponentInstaller
   virtual bool GetInstalledFile(const std::string& file,
                                 base::FilePath* installed_file) = 0;
 
+  // Called by the component updater when a component has been unregistered and
+  // all versions should be uninstalled from disk. Returns true if
+  // uninstallation is supported, false otherwise.
+  virtual bool Uninstall() = 0;
+
  protected:
   friend class base::RefCountedThreadSafe<ComponentInstaller>;