Add in-profile external_extensions.json file for Chrome OS OEM customization.

BUG=57289
TEST=ExtensionServiceTest.ExternalPrefProvider, manualy craete an external_extensions.json file in the new in-profile path.

Review URL: http://codereview.chromium.org/6370009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@72187 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/extension_service.cc

@@ -239,7 +239,7 @@ ExtensionServiceBackend::~ExtensionServiceBackend() {
 
 void ExtensionServiceBackend::LoadSingleExtension(
     const FilePath& path_in, scoped_refptr<ExtensionService> frontend) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
   frontend_ = frontend;
 
@@ -272,7 +272,7 @@ void ExtensionServiceBackend::LoadSingleExtension(
 
 void ExtensionServiceBackend::ReportExtensionLoadError(
     const FilePath& extension_path, const std::string &error) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       NewRunnableMethod(
@@ -282,7 +282,7 @@ void ExtensionServiceBackend::ReportExtensionLoadError(
 }
 
 void ExtensionService::CheckExternalUninstall(const std::string& id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // Check if the providers know about this extension.
   ProviderCollection::const_iterator i;
@@ -303,7 +303,7 @@ void ExtensionService::ClearProvidersForTesting() {
 
 void ExtensionService::AddProviderForTesting(
     ExternalExtensionProviderInterface* test_provider) {
-  DCHECK(test_provider);
+  CHECK(test_provider);
   external_extension_providers_.push_back(
       linked_ptr<ExternalExtensionProviderInterface>(test_provider));
 }
@@ -320,11 +320,11 @@ void ExtensionService::OnExternalExtensionUpdateUrlFound(
     return;
   }
   AddPendingExtensionFromExternalUpdateUrl(id, update_url, location);
-  external_extension_added_ |= true;
+  external_extension_url_added_ |= true;
 }
 
 bool ExtensionService::IsDownloadFromGallery(const GURL& download_url,
-                                              const GURL& referrer_url) {
+                                             const GURL& referrer_url) {
   // Special-case the themes mini-gallery.
   // TODO(erikkay) When that gallery goes away, remove this code.
   if (IsDownloadFromMiniGallery(download_url) &&
@@ -392,7 +392,6 @@ bool ExtensionService::IsInstalledApp(const GURL& url) {
 bool ExtensionService::UninstallExtensionHelper(
     ExtensionService* extensions_service,
     const std::string& extension_id) {
-  DCHECK(extensions_service);
 
   // We can't call UninstallExtension with an invalid extension ID, so check it
   // first.
@@ -422,7 +421,7 @@ ExtensionService::ExtensionService(Profile* profile,
       default_apps_(profile->GetPrefs(),
                     g_browser_process->GetApplicationLocale()),
       event_routers_initialized_(false) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // Figure out if extension installation should be enabled.
   if (command_line->HasSwitch(switches::kDisableExtensions)) {
@@ -519,7 +518,7 @@ const Extension* ExtensionService::GetExtensionById(const std::string& id,
 }
 
 void ExtensionService::Init() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   DCHECK(!ready_);  // Can't redo init.
   DCHECK_EQ(extensions_.size(), 0u);
@@ -549,7 +548,7 @@ namespace {
   // TODO(akalin): Put this somewhere where both crx_installer.cc and
   // this file can use it.
   void DeleteFileHelper(const FilePath& path, bool recursive) {
-    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+    CHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
     file_util::Delete(path, recursive);
   }
 }  // namespace
@@ -557,7 +556,7 @@ namespace {
 void ExtensionService::UpdateExtension(const std::string& id,
                                         const FilePath& extension_path,
                                         const GURL& download_url) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   PendingExtensionMap::const_iterator it = pending_extensions_.find(id);
   bool is_pending_extension = (it != pending_extensions_.end());
@@ -675,7 +674,7 @@ void ExtensionService::AddPendingExtensionInternal(
     bool is_from_sync, bool install_silently,
     bool enable_on_install, bool enable_incognito_on_install,
     Extension::Location install_source) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // If a non-sync update is pending, a sync request should not
   // overwrite it.  This is important for external extensions.
@@ -702,7 +701,7 @@ void ExtensionService::AddPendingExtensionInternal(
 }
 
 void ExtensionService::ReloadExtension(const std::string& extension_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   FilePath path;
   const Extension* current_extension = GetExtensionById(extension_id, false);
 
@@ -746,7 +745,7 @@ void ExtensionService::ReloadExtension(const std::string& extension_id) {
 
 void ExtensionService::UninstallExtension(const std::string& extension_id,
                                            bool external_uninstall) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   const Extension* extension =
       GetExtensionByIdInternal(extension_id, true, true);
@@ -803,7 +802,7 @@ void ExtensionService::ClearExtensionData(const GURL& extension_url) {
 }
 
 void ExtensionService::EnableExtension(const std::string& extension_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   const Extension* extension =
       GetExtensionByIdInternal(extension_id, false, true);
@@ -830,7 +829,7 @@ void ExtensionService::EnableExtension(const std::string& extension_id) {
 }
 
 void ExtensionService::DisableExtension(const std::string& extension_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   const Extension* extension =
       GetExtensionByIdInternal(extension_id, true, false);
@@ -912,7 +911,7 @@ void ExtensionService::LoadComponentExtensions() {
 }
 
 void ExtensionService::LoadAllExtensions() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   base::TimeTicks start_time = base::TimeTicks::Now();
 
@@ -1338,14 +1337,18 @@ void ExtensionService::SetBrowserActionVisibility(const Extension* extension,
 // Errors are reported through ExtensionErrorReporter. Succcess is not
 // reported.
 void ExtensionService::CheckForExternalUpdates() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // Note that this installation is intentionally silent (since it didn't
   // go through the front-end).  Extensions that are registered in this
   // way are effectively considered 'pre-bundled', and so implicitly
   // trusted.  In general, if something has HKLM or filesystem access,
   // they could install an extension manually themselves anyway.
-  external_extension_added_ = false;
+
+  // If any external extension records give a URL, a provider will set
+  // this to true.  Used by OnExternalProviderReady() to see if we need
+  // to start an update check to fetch a new external extension.
+  external_extension_url_added_ = false;
 
   // Ask each external extension provider to give us a call back for each
   // extension they know about. See OnExternalExtension(File|UpdateUrl)Found.
@@ -1357,26 +1360,30 @@ void ExtensionService::CheckForExternalUpdates() {
   }
 
   // Uninstall of unclaimed extensions will happen after all the providers
-  // had reported ready. Trigger uninstall even if there are no providers
-  // installed:
-  OnExternalProviderReady();
+  // had reported ready.  Every provider calls OnExternalProviderReady()
+  // when it finishes, and OnExternalProviderReady() only acts when all
+  // providers are ready.  In case there are no providers, we call it
+  // to trigger removal of extensions that used to have an external source.
+  if (external_extension_providers_.empty())
+    OnExternalProviderReady();
 }
 
 void ExtensionService::OnExternalProviderReady() {
-  // An external provider has finished loading. We only take action
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  // An external provider has finished loading.  We only take action
   // if all of them are finished. So we check them first.
   ProviderCollection::const_iterator i;
   for (i = external_extension_providers_.begin();
        i != external_extension_providers_.end(); ++i) {
     ExternalExtensionProviderInterface* provider = i->get();
-    if (!provider->IsReady()) {
+    if (!provider->IsReady())
       return;
-    }
   }
 
-  // All the providers are ready. Install any pending extensions.
-  if (external_extension_added_ && updater()) {
-    external_extension_added_ = false;
+  // All the providers are ready.  Install any pending extensions.
+  if (external_extension_url_added_ && updater()) {
+    external_extension_url_added_ = false;
     updater()->CheckNow();
   }
 
@@ -1649,7 +1656,7 @@ void ExtensionService::UpdateActiveExtensionsInCrashReporter() {
 }
 
 void ExtensionService::OnExtensionInstalled(const Extension* extension) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // Ensure extension is deleted unless we transfer ownership.
   scoped_refptr<const Extension> scoped_extension(extension);

chrome/browser/extensions/extension_service.h

@@ -150,10 +150,10 @@ class ExtensionService
   // Constructor stores pointers to |profile| and |extension_prefs| but
   // ownership remains at caller.
   ExtensionService(Profile* profile,
-                    const CommandLine* command_line,
-                    const FilePath& install_directory,
-                    ExtensionPrefs* extension_prefs,
-                    bool autoupdate_enabled);
+                   const CommandLine* command_line,
+                   const FilePath& install_directory,
+                   ExtensionPrefs* extension_prefs,
+                   bool autoupdate_enabled);
 
   // Gets the list of currently installed extensions.
   virtual const ExtensionList* extensions() const;
@@ -623,7 +623,7 @@ class ExtensionService
   // Set to true by OnExternalExtensionUpdateUrlFound() when an external
   // extension URL is found.  Used in CheckForExternalUpdates() to see
   // if an update check is needed to install pending extensions.
-  bool external_extension_added_;
+  bool external_extension_url_added_;
 
   FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest,
                            UpdatePendingExtensionAlreadyInstalled);

chrome/browser/extensions/extension_service_unittest.cc

@@ -3034,6 +3034,67 @@ TEST_F(ExtensionServiceTest, ExternalUninstall) {
   ASSERT_EQ(0u, loaded_.size());
 }
 
+// Test that running multiple update checks simultaneously does not
+// keep the update from succeeding.
+TEST_F(ExtensionServiceTest, MultipleExternalUpdateCheck) {
+  InitializeEmptyExtensionService();
+
+  MockExtensionProvider* provider =
+      new MockExtensionProvider(service_.get(), Extension::EXTERNAL_PREF);
+  AddMockExternalProvider(provider);
+
+  // Verify that starting with no providers loads no extensions.
+  service_->Init();
+  loop_.RunAllPending();
+  ASSERT_EQ(0u, loaded_.size());
+
+  // Start two checks for updates.
+  provider->set_visit_count(0);
+  service_->CheckForExternalUpdates();
+  service_->CheckForExternalUpdates();
+  loop_.RunAllPending();
+
+  // Two calls should cause two checks for external extensions.
+  EXPECT_EQ(2, provider->visit_count());
+  EXPECT_EQ(0u, GetErrors().size());
+  EXPECT_EQ(0u, loaded_.size());
+
+  // Register a test extension externally using the mock registry provider.
+  FilePath source_path;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &source_path));
+  source_path = source_path.AppendASCII("extensions").AppendASCII("good.crx");
+  provider->UpdateOrAddExtension(good_crx, "1.0.0.0", source_path);
+
+  // Two checks for external updates should find the extension, and install it
+  // once.
+  provider->set_visit_count(0);
+  service_->CheckForExternalUpdates();
+  service_->CheckForExternalUpdates();
+  loop_.RunAllPending();
+  EXPECT_EQ(2, provider->visit_count());
+  ASSERT_EQ(0u, GetErrors().size());
+  ASSERT_EQ(1u, loaded_.size());
+  ASSERT_EQ(Extension::EXTERNAL_PREF, loaded_[0]->location());
+  ASSERT_EQ("1.0.0.0", loaded_[0]->version()->GetString());
+  ValidatePrefKeyCount(1);
+  ValidateIntegerPref(good_crx, "state", Extension::ENABLED);
+  ValidateIntegerPref(good_crx, "location", Extension::EXTERNAL_PREF);
+
+  provider->RemoveExtension(good_crx);
+  provider->set_visit_count(0);
+  service_->CheckForExternalUpdates();
+  service_->CheckForExternalUpdates();
+  loop_.RunAllPending();
+
+  // Two calls should cause two checks for external extensions.
+  // Because the external source no longer includes good_crx,
+  // good_crx will be uninstalled.  So, expect that no extensions
+  // are loaded.
+  EXPECT_EQ(2, provider->visit_count());
+  EXPECT_EQ(0u, GetErrors().size());
+  EXPECT_EQ(0u, loaded_.size());
+}
+
 TEST_F(ExtensionServiceTest, ExternalPrefProvider) {
   InitializeEmptyExtensionService();
 

chrome/browser/extensions/external_extension_provider_impl.cc

@@ -143,7 +143,7 @@ void ExternalExtensionProviderImpl::SetPrefs(DictionaryValue* prefs) {
         continue;
       }
       service_->OnExternalExtensionFileFound(extension_id, version.get(), path,
-                                            crx_location_);
+                                             crx_location_);
     } else { // if (has_external_update_url)
       CHECK(has_external_update_url);  // Checking of keys above ensures this.
       if (download_location_ == Extension::INVALID) {
@@ -227,9 +227,22 @@ void ExternalExtensionProviderImpl::CreateExternalProviders(
       linked_ptr<ExternalExtensionProviderInterface>(
           new ExternalExtensionProviderImpl(
               service,
-              new ExternalPrefExtensionLoader(app::DIR_EXTERNAL_EXTENSIONS),
+              new ExternalPrefExtensionLoader(
+                  app::DIR_EXTERNAL_EXTENSIONS),
               Extension::EXTERNAL_PREF,
               Extension::EXTERNAL_PREF_DOWNLOAD)));
+
+#if defined(OS_CHROMEOS)
+  // Chrome OS specific source for OEM customization.
+  provider_list->push_back(
+      linked_ptr<ExternalExtensionProviderInterface>(
+          new ExternalExtensionProviderImpl(
+              service,
+              new ExternalPrefExtensionLoader(
+                  chrome::DIR_USER_EXTERNAL_EXTENSIONS),
+              Extension::EXTERNAL_PREF,
+              Extension::EXTERNAL_PREF_DOWNLOAD)));
+#endif
 #if defined(OS_WIN)
   provider_list->push_back(
       linked_ptr<ExternalExtensionProviderInterface>(

chrome/browser/extensions/external_policy_extension_loader_unittest.cc

@@ -85,11 +85,11 @@ class MockExternalPolicyExtensionProviderVisitor
     // Remove the extension from our list.
     StringValue ext_str(id + ";" + update_url.spec());
     EXPECT_NE(-1, remaining_extensions->Remove(ext_str));
- }
+  }
 
- virtual void OnExternalProviderReady() {
+  virtual void OnExternalProviderReady() {
     EXPECT_TRUE(provider_->IsReady());
- }
+  }
 
  private:
   ListValue* remaining_extensions;

chrome/browser/extensions/external_pref_extension_loader.cc

@@ -14,15 +14,18 @@
 
 namespace {
 
-// Caller takes ownership of the returned dictionary
-DictionaryValue* ExtractPrefs(ValueSerializer* serializer) {
+// Caller takes ownership of the returned dictionary.
+DictionaryValue* ExtractPrefs(const FilePath& path,
+                              ValueSerializer* serializer) {
   std::string error_msg;
   Value* extensions = serializer->Deserialize(NULL, &error_msg);
   if (!extensions) {
-    LOG(WARNING) << "Unable to deserialize json data: " << error_msg;
+    LOG(WARNING) << "Unable to deserialize json data: " << error_msg
+                 << " In file " << path.value() << " .";
   } else {
     if (!extensions->IsType(Value::TYPE_DICTIONARY)) {
-      NOTREACHED() << "Invalid json data";
+      LOG(WARNING) << "Expected a JSON dictionary in file "
+                   << path.value() << " .";
     } else {
       return static_cast<DictionaryValue*>(extensions);
     }
@@ -40,11 +43,7 @@ ExternalPrefExtensionLoader::ExternalPrefExtensionLoader(int base_path_key)
 const FilePath ExternalPrefExtensionLoader::GetBaseCrxFilePath() {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  // LoadOnFileThread() should set |external_file_path_| to a non-empty
-  // path.  This function should not be called until after LoadOnFileThread()
-  // is complete.
-  CHECK(!base_path_.empty());
-
+  // |base_path_| was set in LoadOnFileThread().
   return base_path_;
 }
 
@@ -60,8 +59,14 @@ void ExternalPrefExtensionLoader::StartLoading() {
 void ExternalPrefExtensionLoader::LoadOnFileThread() {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
-  scoped_ptr<DictionaryValue> prefs;
+  // TODO(skerner): Some values of base_path_key_ will cause
+  // PathService::Get() to return false, because the path does
+  // not exist.  Find and fix the build/install scripts so that
+  // this can become a CHECK().  Known examples include chrome
+  // OS developer builds and linux install packages.
+  // Tracked as crbug.com/70402 .
 
+  scoped_ptr<DictionaryValue> prefs;
   if (PathService::Get(base_path_key_, &base_path_)) {
     FilePath json_file;
     json_file =
@@ -69,7 +74,7 @@ void ExternalPrefExtensionLoader::LoadOnFileThread() {
 
     if (file_util::PathExists(json_file)) {
       JSONFileValueSerializer serializer(json_file);
-      prefs.reset(ExtractPrefs(&serializer));
+      prefs.reset(ExtractPrefs(json_file, &serializer));
     }
   }
 
@@ -77,11 +82,17 @@ void ExternalPrefExtensionLoader::LoadOnFileThread() {
     prefs.reset(new DictionaryValue());
 
   prefs_.reset(prefs.release());
+
+  // If we have any records to process, then we must have
+  // read the .json file.  If we read the .json file, then
+  // we were able to set |base_path_|.
+  CHECK(!prefs_->empty() || !base_path_.empty());
+
   BrowserThread::PostTask(
-       BrowserThread::UI, FROM_HERE,
-       NewRunnableMethod(
-           this,
-           &ExternalPrefExtensionLoader::LoadFinished));
+      BrowserThread::UI, FROM_HERE,
+      NewRunnableMethod(
+          this,
+          &ExternalPrefExtensionLoader::LoadFinished));
 }
 
 ExternalTestingExtensionLoader::ExternalTestingExtensionLoader(
@@ -90,7 +101,8 @@ ExternalTestingExtensionLoader::ExternalTestingExtensionLoader(
     : fake_base_path_(fake_base_path) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   JSONStringValueSerializer serializer(json_data);
-  testing_prefs_.reset(ExtractPrefs(&serializer));
+  FilePath fake_json_path = fake_base_path.AppendASCII("fake.json");
+  testing_prefs_.reset(ExtractPrefs(fake_json_path, &serializer));
 }
 
 void ExternalTestingExtensionLoader::StartLoading() {

chrome/common/chrome_paths.cc

@@ -319,6 +319,14 @@ bool PathProvider(int key, FilePath* result) {
         return false;
       break;
     }
+#endif
+#if defined(OS_CHROMEOS)
+    case chrome::DIR_USER_EXTERNAL_EXTENSIONS: {
+      if (!PathService::Get(chrome::DIR_USER_DATA, &cur))
+        return false;
+      cur = cur.Append(FILE_PATH_LITERAL("External Extensions"));
+      break;
+    }
 #endif
     default:
       return false;

chrome/common/chrome_paths.h

@@ -49,6 +49,12 @@ enum {
   DIR_MANAGED_PREFS,            // Directory that stores the managed prefs plist
                                 // files for the current user.
 #endif
+#if defined(OS_CHROMEOS)
+  DIR_USER_EXTERNAL_EXTENSIONS,  // Directory for per-user external extensions.
+                                 // Used for OEM customization on Chrome OS.
+                                 // Getting this path does not create it.
+#endif
+
   FILE_RESOURCE_MODULE,         // Full path and filename of the module that
                                 // contains embedded resources (version,
                                 // strings, images, etc.).