Add support for uninstalling components and use it in SupervisedUserWhitelistInstaller.

components/component_updater/default_component_installer.cc

Index: components/component_updater/default_component_installer.cc
diff --git a/components/component_updater/default_component_installer.cc b/components/component_updater/default_component_installer.cc
index d73b4fc9cf868c795eb881dbea54e6a41399060b..bf56a8871cbafb29b9e8a8f6efa141c156300a92 100644
--- a/components/component_updater/default_component_installer.cc
+++ b/components/component_updater/default_component_installer.cc
@@ -17,16 +17,43 @@
 #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";
+
+void UninstallOnTaskRunner(const base::FilePath& base_dir) {
+  std::vector<base::FilePath> paths;
+  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();
+  }
+}
+
 }  // namespace
 
 ComponentInstallerTraits::~ComponentInstallerTraits() {
@@ -35,12 +62,18 @@ ComponentInstallerTraits::~ComponentInstallerTraits() {
 DefaultComponentInstaller::DefaultComponentInstaller(
     scoped_ptr<ComponentInstallerTraits> installer_traits)
     : current_version_(kNullVersion),
+      should_uninstall_on_destruction_(false),
       main_task_runner_(base::MessageLoopProxy::current()) {
   installer_traits_ = installer_traits.Pass();
 }
 
 DefaultComponentInstaller::~DefaultComponentInstaller() {
   DCHECK(thread_checker_.CalledOnValidThread());
+  if (should_uninstall_on_destruction_) {

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

If Chrome is on the exit path, it is possible that the task_runner_ may not run
this task. Therefore, the uninstall code may not run, and the files remain on
the file system. Will this be a problem? Will it cause the component to be
registered again at Chrome boot time? Will these files ever be deleted?

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

No, SupervisedUserWhitelistService keeps track of installed whitelists
separately, so it won't register them on the next startup. The files will still
be around, however. Maybe we could delete unknown whitelists on startup?

[Sorin Jianu, 2015/02/06 00:22:04]

I think cleaning up on startup could be wise. I don't really know the
registration execution path, so that would be something for you to consider.

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

OK. Something for a followup CL maybe?

+      task_runner_->PostTask(FROM_HERE,
+                             base::Bind(&UninstallOnTaskRunner,
+                                        installer_traits_->GetBaseDirectory()));
+  }
 }
 
 void DefaultComponentInstaller::Register(ComponentUpdateService* cus) {
@@ -77,6 +110,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 +154,19 @@ bool DefaultComponentInstaller::GetInstalledFile(
   return true;
 }
 
+void DefaultComponentInstaller::Unregister(ComponentUpdateService* cus) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  CrxComponent crx;
+  installer_traits_->GetHash(&crx.pk_hash);
+  ComponentUpdateService::Status status =
+      cus->UnregisterComponent(GetCrxComponentID(crx));
+  DCHECK_EQ(ComponentUpdateService::kOk, status);
+
+  // Wait with uninstallation until all references have been dropped.
+  should_uninstall_on_destruction_ = true;

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

Would it be possible to run the uninstall code here instead of the destruction
path?

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

The problem with that is that when this method is called, the ComponentInstaller
might be currently doing something on its task runner. And even if we make sure
to post the uninstallation task to the same task runner (which is a
SequencedTaskRunner, thereby making sure it will run after the current task
running there), the ComponentUnpacker will also hold a reference to this object
and start installation when it's finished with unpacking, and that could happen
at _any_ point, including after uninstallation has finished.

[Sorin Jianu, 2015/02/06 00:22:04]

Right. I wonder if this would work. ComponentInstaller is an interface which
gets called by the service at relevant times. It has an Install method. It could
also have an Uninstall method, and when that method is called, then the service
can release its reference to the installer, and this code can run.

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

Done. 

+}
+
 void DefaultComponentInstaller::StartRegistration(ComponentUpdateService* cus) {
   DCHECK(task_runner_.get());
   DCHECK(task_runner_->RunsTasksOnCurrentThread());