Cleaning Up Extensions When Local Content Removed

chrome/browser/extensions/extension_service_unittest.cc

 // Copyright (c) 2012 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 "chrome/browser/extensions/extension_service_unittest.h"
 
 #include <algorithm>
 #include <set>
 #include <vector>
 
@@ skipping 231 matching lines @@
   mutable int visit_count_;
 
   int creation_flags_;
 
   DISALLOW_COPY_AND_ASSIGN(MockExtensionProvider);
 };
 
 class MockProviderVisitor
     : public ExternalExtensionProviderInterface::VisitorInterface {
  public:
-
   // The provider will return |fake_base_path| from
   // GetBaseCrxFilePath().  User can test the behavior with
   // and without an empty path using this parameter.
   explicit MockProviderVisitor(FilePath fake_base_path)
       : ids_found_(0),
         fake_base_path_(fake_base_path) {
   }
 
   int Visit(const std::string& json_data) {
     // Give the test json file to the provider for parsing.
@@ skipping 735 matching lines @@
   ASSERT_TRUE(file_util::PathExists(private_key_path));
 }
 
 // The tests are designed so that we never expect to see a packing error.
 void PackExtensionTestClient::OnPackFailure(const std::string& error_message,
                                             ExtensionCreator::ErrorType type) {
   if (type == ExtensionCreator::kCRXExists)
      FAIL() << "Packing should not fail.";
   else
      FAIL() << "Existing CRX should have been overwritten.";
-
 }
 
 // Test loading good extensions from the profile directory.
 TEST_F(ExtensionServiceTest, LoadAllExtensionsFromDirectorySuccess) {
   PluginService::GetInstance()->Init();
 
   // Initialize the test dir with a good Preferences/extensions.
   FilePath source_install_dir = data_dir_
       .AppendASCII("good")
       .AppendASCII("Extensions");
@@ skipping 123 matching lines @@
       std::string("Could not load extension from '*'. ") +
       extension_manifest_errors::kMissingFile)) <<
       UTF16ToUTF8(GetErrors()[2]);
 
   EXPECT_TRUE(MatchPattern(UTF16ToUTF8(GetErrors()[3]),
       std::string("Could not load extension from '*'. ") +
       extension_manifest_errors::kManifestUnreadable)) <<
       UTF16ToUTF8(GetErrors()[3]);
 };
 
-// Test that partially deleted extensions are cleaned up during startup
-// Test loading bad extensions from the profile directory.
-TEST_F(ExtensionServiceTest, CleanupOnStartup) {
+// Test that old versions of extensions are deleted during garbage collection.
+TEST_F(ExtensionServiceTest, GarbageCollectOldVersions) {
   PluginService::GetInstance()->Init();
 
   FilePath source_install_dir = data_dir_
+      .AppendASCII("garbage_collection")
+      .AppendASCII("Extensions");
+
+  FilePath pref_path = source_install_dir
+      .DirName()
+      .AppendASCII("Preferences");
+
+  InitializeInstalledExtensionService(pref_path, source_install_dir);
+
+  // Verify that there are two versions present initially.
+  ASSERT_TRUE(file_util::PathExists(
+      extensions_install_dir_.AppendASCII("fdoajpacpdeapbmhbblepcnilfkpmkff")
+                             .AppendASCII("1.0_0")));
+  ASSERT_TRUE(file_util::PathExists(
+      extensions_install_dir_.AppendASCII("fdoajpacpdeapbmhbblepcnilfkpmkff")
+                             .AppendASCII("1.1_0")));
+  service_->Init();
+  loop_.RunAllPending();
+
+  // Garbage collection should have deleted the old version.
+  ASSERT_FALSE(file_util::PathExists(
+      extensions_install_dir_.AppendASCII("fdoajpacpdeapbmhbblepcnilfkpmkff")
+                             .AppendASCII("1.0_0")));
+  ASSERT_TRUE(file_util::PathExists(
+      extensions_install_dir_.AppendASCII("fdoajpacpdeapbmhbblepcnilfkpmkff")
+                             .AppendASCII("1.1_0")));
+}
+
+// Test that extensions which were deleted from the preferences are cleaned up
+// during startup.
+// Test loading bad extensions from the profile directory.
+TEST_F(ExtensionServiceTest, GarbageCollectOnStartup) {
+  PluginService::GetInstance()->Init();
+
+  FilePath source_install_dir = data_dir_
       .AppendASCII("good")
       .AppendASCII("Extensions");
   FilePath pref_path = source_install_dir
       .DirName()
       .AppendASCII("Preferences");
 
   InitializeInstalledExtensionService(pref_path, source_install_dir);
 
   // Simulate that one of them got partially deleted by clearing its pref.
   {
     DictionaryPrefUpdate update(profile_->GetPrefs(), "extensions.settings");
     DictionaryValue* dict = update.Get();
-    ASSERT_TRUE(dict != NULL);
+    ASSERT_TRUE(dict);
     dict->Remove("behllobkkfkfnphdnhnkndlbkcpglgmj", NULL);
   }
 
   service_->Init();
   // Wait for GarbageCollectExtensions task to complete.
   loop_.RunAllPending();
 
   file_util::FileEnumerator dirs(extensions_install_dir_, false,
                                  file_util::FileEnumerator::DIRECTORIES);
   size_t count = 0;
   while (!dirs.Next().empty())
     count++;
 
   // We should have only gotten two extensions now.
   EXPECT_EQ(2u, count);
 
   // And extension1 dir should now be toast.
   FilePath extension_dir = extensions_install_dir_
       .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj");
   ASSERT_FALSE(file_util::PathExists(extension_dir));
 }
 
+// Test that internal extensions which are referenced in the preferences but
+// had their content deleted are cleaned up during startup.
+TEST_F(ExtensionServiceTest, GarbageCollectInternalExtensionsMissingContent) {
+  PluginService::GetInstance()->Init();

[Aaron Boodman, 2012/04/07 18:01:03]

I don't think this is needed unless you're using plugin functionality somehow.
Try removing it.

[Devlin, 2012/04/14 18:13:21]

Tried removing; it broke.

+
+  FilePath source_dir = data_dir_.AppendASCII("good").AppendASCII("Extensions");
+  FilePath pref_path = source_dir.DirName().AppendASCII("Preferences");
+
+  InitializeInstalledExtensionService(pref_path, source_dir);

[Aaron Boodman, 2012/04/07 18:01:03]

ASSERT the number of extensions in the prefs you're starting with?

[Devlin, 2012/04/14 18:13:21]

Done.

+
+  // Delete the extension's directory.
+  FilePath extension_dir = extensions_install_dir_
+      .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj");
+  ASSERT_TRUE(file_util::Delete(extension_dir, true));
+
+  // Run GarbageCollectExtensions.
+  service_->Init();
+  loop_.RunAllPending();
+
+  file_util::FileEnumerator dirs(extensions_install_dir_, false,
+                                 file_util::FileEnumerator::DIRECTORIES);
+  size_t count = 0;
+  while (!dirs.Next().empty())
+    count++;
+
+  // We should have only gotten two extensions now.
+  EXPECT_EQ(2u, count);
+
+  DictionaryPrefUpdate update(profile_->GetPrefs(), "extensions.settings");

[Aaron Boodman, 2012/04/07 18:01:03]

Nit: If you're not actually doing any updating, you can just access the
Preferences object from GetPrefs() directly.

[Devlin, 2012/04/14 18:13:21]

Done.

+  DictionaryValue* dictionary = update.Get();
+  ASSERT_TRUE(dictionary);
+  ASSERT_FALSE(dictionary->HasKey("behllobkkfkfnphdnhnkndlbkcpglgmj"));
+}
+
+// Test that unpacked extensions which are referenced in the preferences but
+// had their content deleted are cleaned up during startup.
+TEST_F(ExtensionServiceTest, GarbageCollectUnpackedExtensionsMissingContent) {

[Aaron Boodman, 2012/04/07 18:01:03]

Can you look into moving all the garbage collection tests extension
extension_garbage_collector_unittest.cc?

[Devlin, 2012/04/14 18:13:21]

Unfortunately, these tests use a few things unique to ExtensionServiceTest
(versus ExtensionServiceTestBase, which is what's in the header file), e.g.
loaded_, ValidatePrefKeyCount(), etc. Moving the tests would mean moving the
definition of ExtensionServiceTest out of the cc file. Since other services use
this as a testing ground (e.g. installing, uninstalling, packing extensions), I
assumed it was alright for garbage collection to be here, too. If you want me to
refactor enough to move garbage collection out, lemme know and I'd be happy to
do it in a separate CL (this one's growing a little large, and it's a decent
size refactor).

+  InitializeEmptyExtensionService();
+
+  ScopedTempDir temp;
+  ASSERT_TRUE(temp.CreateUniqueTempDir());
+
+  // Write the manifest dynamically, since we have to delete the file anyway.
+  FilePath extension_path = temp.path();
+  FilePath manifest_path = extension_path.Append(Extension::kManifestFilename);
+  ASSERT_FALSE(file_util::PathExists(manifest_path));
+
+  // Construct a simple manifest and install it.
+  DictionaryValue manifest;
+  manifest.SetString("version", "1.0");
+  manifest.SetString("name", "Cleanup Unpacked Extensions Missing Content");
+  manifest.SetInteger("manifest_version", 2);
+
+  JSONFileValueSerializer serializer(manifest_path);
+  ASSERT_TRUE(serializer.Serialize(manifest));
+
+  extensions::UnpackedInstaller::Create(service_)->Load(extension_path);
+  loop_.RunAllPending();
+
+  // Make sure it installed correctly.
+  EXPECT_EQ(0u, GetErrors().size());
+  ASSERT_EQ(1u, loaded_.size());
+  EXPECT_EQ(Extension::LOAD, loaded_[0]->location());
+  EXPECT_EQ(1u, service_->extensions()->size());
+
+  std::string extension_id = loaded_[0]->id();
+
+  // Make sure it is in the preferences at this point.
+  DictionaryPrefUpdate initial_update(
+      profile_->GetPrefs(), "extensions.settings");
+  DictionaryValue* initial_dictionary = initial_update.Get();
+  ASSERT_TRUE(initial_dictionary);
+  ASSERT_TRUE(initial_dictionary->HasKey(extension_id));
+
+  // Delete local content, GarbageCollectExtensions, and test whether the key
+  // is still in the preferences.
+  ASSERT_TRUE(file_util::Delete(extension_path, true));
+  service_->GarbageCollectExtensions();
+  loop_.RunAllPending();
+
+  DictionaryPrefUpdate final_update(
+      profile_->GetPrefs(), "extensions.settings");
+  DictionaryValue* final_dictionary = final_update.Get();
+  ASSERT_TRUE(final_dictionary);
+  ASSERT_FALSE(final_dictionary->HasKey(extension_id));
+}
+
 // Test installing extensions. This test tries to install few extensions using
 // crx files. If you need to change those crx files, feel free to repackage
 // them, throw away the key used and change the id's above.
 TEST_F(ExtensionServiceTest, InstallExtension) {
   InitializeEmptyExtensionService();
 
   // Extensions not enabled.
   set_extensions_enabled(false);
   FilePath path = data_dir_.AppendASCII("good.crx");
   InstallCRX(path, INSTALL_FAILED);
@@ skipping 3657 matching lines @@
   provider->UpdateOrAddExtension(hosted_app, "1.0.0.0",
                                  data_dir_.AppendASCII("hosted_app.crx"));
 
   service_->CheckForExternalUpdates();
   loop_.RunAllPending();
 
   ASSERT_TRUE(service_->PopulateExtensionGlobalError(
       extension_global_error.get()));
   ASSERT_EQ(1u, extension_global_error->get_external_extension_ids()->size());
 }