diff options
18 files changed, 497 insertions, 407 deletions
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc index 52010a29..8b7c516 100644 --- a/chrome/browser/extensions/extension_service.cc +++ b/chrome/browser/extensions/extension_service.cc @@ -1241,9 +1241,59 @@ void ExtensionService::CheckForUpdatesSoon() { } } +ExtensionSyncData ExtensionService::GetSyncDataHelper( + const Extension& extension) const { + const std::string& id = extension.id(); + ExtensionSyncData data; + data.id = id; + data.uninstalled = false; + data.enabled = IsExtensionEnabled(id); + data.incognito_enabled = IsIncognitoEnabled(id); + data.version = *extension.version(); + data.update_url = extension.update_url(); + data.name = extension.name(); + return data; +} + +bool ExtensionService::GetSyncData( + const std::string& id, + ExtensionFilter filter, + ExtensionSyncData* extension_sync_data) const { + DCHECK(Extension::IdIsValid(id)); + // TODO(akalin): Figure out what to do with terminated extensions. + const Extension* extension = GetExtensionById(id, true); + if (!extension || !(*filter)(*extension)) { + return false; + } + *extension_sync_data = GetSyncDataHelper(*extension); + return true; +} + +void ExtensionService::GetSyncDataListHelper( + const ExtensionList& extensions, + ExtensionFilter filter, + std::vector<ExtensionSyncData>* sync_data_list) const { + for (ExtensionList::const_iterator it = extensions.begin(); + it != extensions.end(); ++it) { + const Extension& extension = **it; + if ((*filter)(extension)) { + sync_data_list->push_back(GetSyncDataHelper(extension)); + } + } +} + +std::vector<ExtensionSyncData> ExtensionService::GetSyncDataList( + ExtensionFilter filter) const { + std::vector<ExtensionSyncData> sync_data_list; + GetSyncDataListHelper(extensions_, filter, &sync_data_list); + GetSyncDataListHelper(disabled_extensions_, filter, &sync_data_list); + // TODO(akalin): Figure out what to do with terminated extensions. + return sync_data_list; +} + void ExtensionService::ProcessSyncData( const ExtensionSyncData& extension_sync_data, - PendingExtensionInfo::ShouldAllowInstallPredicate should_allow) { + ExtensionFilter filter) { const std::string& id = extension_sync_data.id; // Handle uninstalls first. @@ -1291,7 +1341,7 @@ void ExtensionService::ProcessSyncData( pending_extension_manager()->AddFromSync( id, extension_sync_data.update_url, - should_allow, + filter, true, // install_silently extension_sync_data.enabled, extension_sync_data.incognito_enabled); diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h index 3db8d76..eb87da9 100644 --- a/chrome/browser/extensions/extension_service.h +++ b/chrome/browser/extensions/extension_service.h @@ -11,6 +11,7 @@ #include <string> #include <vector> +#include "base/compiler_specific.h" #include "base/command_line.h" #include "base/file_path.h" #include "base/gtest_prod_util.h" @@ -54,9 +55,13 @@ class Version; // various classes have on ExtensionService. This allows easy mocking. class ExtensionServiceInterface { public: + // A function that returns true if the given extension should be + // included and false if it should be filtered out. Identical to + // PendingExtensionInfo::ShouldAllowInstallPredicate. + typedef bool (*ExtensionFilter)(const Extension&); + virtual ~ExtensionServiceInterface() {} virtual const ExtensionList* extensions() const = 0; - virtual const ExtensionList* disabled_extensions() const = 0; virtual PendingExtensionManager* pending_extension_manager() = 0; virtual void UpdateExtension(const std::string& id, const FilePath& path, @@ -64,49 +69,43 @@ class ExtensionServiceInterface { virtual const Extension* GetExtensionById(const std::string& id, bool include_disabled) const = 0; - virtual bool UninstallExtension(const std::string& extension_id, - bool external_uninstall, - std::string* error) = 0; - virtual bool IsExtensionEnabled(const std::string& extension_id) const = 0; virtual bool IsExternalExtensionUninstalled( const std::string& extension_id) const = 0; - virtual void EnableExtension(const std::string& extension_id) = 0; - virtual void DisableExtension(const std::string& extension_id) = 0; virtual void UpdateExtensionBlacklist( const std::vector<std::string>& blacklist) = 0; virtual void CheckAdminBlacklist() = 0; - virtual bool IsIncognitoEnabled(const std::string& extension_id) const = 0; - virtual void SetIsIncognitoEnabled(const std::string& extension_id, - bool enabled) = 0; - // Safe to call multiple times in a row. // // TODO(akalin): Remove this method (and others) once we refactor // themes sync to not use it directly. virtual void CheckForUpdatesSoon() = 0; + // Methods used by sync. + // + // TODO(akalin): We'll eventually need separate methods for app + // sync. See http://crbug.com/58077 and http://crbug.com/61447. + + // Get the sync data for a particular id. If an extension with the + // given ID exists and passes |filter|, fill in + // |extension_sync_data| and return true. Otherwise, return false. + virtual bool GetSyncData(const std::string& id, + ExtensionFilter filter, + ExtensionSyncData* extension_sync_data) const = 0; + + // Return a list of ExtensionSyncData objects for all extensions + // matching |filter|. + virtual std::vector<ExtensionSyncData> GetSyncDataList( + ExtensionFilter filter) const = 0; + // Take any actions required to make the local state of the // extension match the state in |extension_sync_data| (including // installing/uninstalling the extension). - // - // TODO(akalin): We'll eventually need a separate method for app - // sync. See http://crbug.com/58077 and http://crbug.com/61447. virtual void ProcessSyncData( const ExtensionSyncData& extension_sync_data, - PendingExtensionInfo::ShouldAllowInstallPredicate should_allow) = 0; - - // TODO(akalin): Add a method like: - // - // virtual void - // GetInitialSyncData(bool (*filter)(Extension), - // map<string, ExtensionSyncData>* out) const; - // - // which would fill |out| with sync data for the extensions that - // match |filter|. Sync would use this for the initial syncing - // step. + ExtensionFilter filter) = 0; }; // Manages installed and running Chromium extensions. @@ -174,12 +173,12 @@ class ExtensionService virtual ~ExtensionService(); // Gets the list of currently installed extensions. - virtual const ExtensionList* extensions() const; - virtual const ExtensionList* disabled_extensions() const; - virtual const ExtensionList* terminated_extensions() const; + virtual const ExtensionList* extensions() const OVERRIDE; + const ExtensionList* disabled_extensions() const; + const ExtensionList* terminated_extensions() const; // Gets the object managing the set of pending extensions. - virtual PendingExtensionManager* pending_extension_manager(); + virtual PendingExtensionManager* pending_extension_manager() OVERRIDE; // Registers an extension to be loaded as a component extension. void register_component_extension(const ComponentExtensionInfo& info) { @@ -230,8 +229,8 @@ class ExtensionService void InitEventRouters(); // Look up an extension by ID. - virtual const Extension* GetExtensionById(const std::string& id, - bool include_disabled) const; + virtual const Extension* GetExtensionById( + const std::string& id, bool include_disabled) const OVERRIDE; // Looks up a terminated (crashed) extension by ID. GetExtensionById does // not include terminated extensions. @@ -243,7 +242,7 @@ class ExtensionService // CrxInstaller directly instead. virtual void UpdateExtension(const std::string& id, const FilePath& extension_path, - const GURL& download_url); + const GURL& download_url) OVERRIDE; // Reloads the specified extension. void ReloadExtension(const std::string& extension_id); @@ -258,9 +257,10 @@ class ExtensionService bool external_uninstall, std::string* error); - virtual bool IsExtensionEnabled(const std::string& extension_id) const; + virtual bool IsExtensionEnabled( + const std::string& extension_id) const OVERRIDE; virtual bool IsExternalExtensionUninstalled( - const std::string& extension_id) const; + const std::string& extension_id) const OVERRIDE; // Enable or disable an extension. No action if the extension is already // enabled/disabled. @@ -357,19 +357,25 @@ class ExtensionService // Go through each extensions in pref, unload blacklisted extensions // and update the blacklist state in pref. virtual void UpdateExtensionBlacklist( - const std::vector<std::string>& blacklist); + const std::vector<std::string>& blacklist) OVERRIDE; // Go through each extension and unload those that the network admin has // put on the blacklist (not to be confused with the Google managed blacklist // set of extensions. - virtual void CheckAdminBlacklist(); + virtual void CheckAdminBlacklist() OVERRIDE; - virtual void CheckForUpdatesSoon(); + virtual void CheckForUpdatesSoon() OVERRIDE; + // Sync methods implementation. + virtual bool GetSyncData( + const std::string& id, + ExtensionFilter filter, + ExtensionSyncData* extension_sync_data) const OVERRIDE; + virtual std::vector<ExtensionSyncData> GetSyncDataList( + ExtensionFilter filter) const OVERRIDE; virtual void ProcessSyncData( const ExtensionSyncData& extension_sync_data, - PendingExtensionInfo::ShouldAllowInstallPredicate - should_allow_install); + ExtensionFilter filter) OVERRIDE; void set_extensions_enabled(bool enabled) { extensions_enabled_ = enabled; } bool extensions_enabled() { return extensions_enabled_; } @@ -491,6 +497,16 @@ class ExtensionService }; typedef std::list<NaClModuleInfo> NaClModuleInfoList; + // Gets the sync data for the given extension. + ExtensionSyncData GetSyncDataHelper(const Extension& extension) const; + + // Appends sync data objects for every extension in |extensions| + // that passes |filter|. + void GetSyncDataListHelper( + const ExtensionList& extensions, + ExtensionFilter filter, + std::vector<ExtensionSyncData>* sync_data_list) const; + // Clear all persistent data that may have been stored by the extension. void ClearExtensionData(const GURL& extension_url); diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc index f29d6cc..391331a 100644 --- a/chrome/browser/extensions/extension_service_unittest.cc +++ b/chrome/browser/extensions/extension_service_unittest.cc @@ -3431,12 +3431,111 @@ TEST_F(ExtensionServiceTest, ComponentExtensions) { namespace { -bool AlwaysInstall(const Extension& extension) { +bool AllExtensions(const Extension& extension) { return true; } +bool NoExtensions(const Extension& extension) { + return false; +} + +bool ExtensionsOnly(const Extension& extension) { + return extension.GetType() == Extension::TYPE_EXTENSION; +} + +bool ThemesOnly(const Extension& extension) { + return extension.is_theme(); +} + +bool GoodOnly(const Extension& extension) { + return extension.id() == good_crx; +} + } // namespace +TEST_F(ExtensionServiceTest, GetSyncData) { + InitializeEmptyExtensionService(); + InstallCrx(data_dir_.AppendASCII("good.crx"), true); + + ExtensionSyncData data; + EXPECT_TRUE(service_->GetSyncData(good_crx, &AllExtensions, &data)); + const Extension* extension = service_->GetExtensionById(good_crx, true); + ASSERT_TRUE(extension); + EXPECT_EQ(extension->id(), data.id); + EXPECT_FALSE(data.uninstalled); + EXPECT_EQ(service_->IsExtensionEnabled(good_crx), data.enabled); + EXPECT_EQ(service_->IsIncognitoEnabled(good_crx), data.incognito_enabled); + EXPECT_TRUE(data.version.Equals(*extension->version())); + EXPECT_EQ(extension->update_url(), data.update_url); + EXPECT_EQ(extension->name(), data.name); +} + +TEST_F(ExtensionServiceTest, GetSyncDataFilter) { + InitializeEmptyExtensionService(); + InstallCrx(data_dir_.AppendASCII("good.crx"), true); + ExtensionSyncData data; + EXPECT_FALSE(service_->GetSyncData(good_crx, &ThemesOnly, &data)); +} + +TEST_F(ExtensionServiceTest, GetSyncDataNotPresent) { + InitializeEmptyExtensionService(); + ExtensionSyncData data; + EXPECT_FALSE(service_->GetSyncData(good_crx, &AllExtensions, &data)); +} + +TEST_F(ExtensionServiceTest, GetSyncDataUserSettings) { + InitializeEmptyExtensionService(); + InstallCrx(data_dir_.AppendASCII("good.crx"), true); + + { + ExtensionSyncData data; + EXPECT_TRUE(service_->GetSyncData(good_crx, &AllExtensions, &data)); + EXPECT_TRUE(data.enabled); + EXPECT_FALSE(data.incognito_enabled); + } + + service_->DisableExtension(good_crx); + { + ExtensionSyncData data; + EXPECT_TRUE(service_->GetSyncData(good_crx, &AllExtensions, &data)); + EXPECT_FALSE(data.enabled); + EXPECT_FALSE(data.incognito_enabled); + } + + service_->SetIsIncognitoEnabled(good_crx, true); + { + ExtensionSyncData data; + EXPECT_TRUE(service_->GetSyncData(good_crx, &AllExtensions, &data)); + EXPECT_FALSE(data.enabled); + EXPECT_TRUE(data.incognito_enabled); + } + + service_->EnableExtension(good_crx); + { + ExtensionSyncData data; + EXPECT_TRUE(service_->GetSyncData(good_crx, &AllExtensions, &data)); + EXPECT_TRUE(data.enabled); + EXPECT_TRUE(data.incognito_enabled); + } +} + +TEST_F(ExtensionServiceTest, GetSyncDataList) { + InitializeEmptyExtensionService(); + InstallCrx(data_dir_.AppendASCII("good.crx"), true); + InstallCrx(data_dir_.AppendASCII("page_action.crx"), true); + InstallCrx(data_dir_.AppendASCII("theme.crx"), true); + InstallCrx(data_dir_.AppendASCII("theme2.crx"), true); + + service_->DisableExtension(page_action); + service_->DisableExtension(theme2_crx); + + EXPECT_EQ(4u, service_->GetSyncDataList(&AllExtensions).size()); + EXPECT_EQ(0u, service_->GetSyncDataList(&NoExtensions).size()); + EXPECT_EQ(2u, service_->GetSyncDataList(&ExtensionsOnly).size()); + EXPECT_EQ(2u, service_->GetSyncDataList(&ThemesOnly).size()); + EXPECT_EQ(1u, service_->GetSyncDataList(&GoodOnly).size()); +} + TEST_F(ExtensionServiceTest, ProcessSyncDataUninstall) { InitializeEmptyExtensionService(); @@ -3445,7 +3544,7 @@ TEST_F(ExtensionServiceTest, ProcessSyncDataUninstall) { extension_sync_data.uninstalled = true; // Should do nothing. - service_->ProcessSyncData(extension_sync_data, &AlwaysInstall); + service_->ProcessSyncData(extension_sync_data, &AllExtensions); // Install the extension. FilePath extension_path = data_dir_.AppendASCII("good.crx"); @@ -3453,11 +3552,11 @@ TEST_F(ExtensionServiceTest, ProcessSyncDataUninstall) { EXPECT_TRUE(service_->GetExtensionById(good_crx, true)); // Should uninstall the extension. - service_->ProcessSyncData(extension_sync_data, &AlwaysInstall); + service_->ProcessSyncData(extension_sync_data, &AllExtensions); EXPECT_FALSE(service_->GetExtensionById(good_crx, true)); // Should again do nothing. - service_->ProcessSyncData(extension_sync_data, &AlwaysInstall); + service_->ProcessSyncData(extension_sync_data, &AllExtensions); } @@ -3475,19 +3574,19 @@ TEST_F(ExtensionServiceTest, ProcessSyncDataSettings) { *(service_->GetExtensionById(good_crx, true)->version()); extension_sync_data.enabled = false; - service_->ProcessSyncData(extension_sync_data, &AlwaysInstall); + service_->ProcessSyncData(extension_sync_data, &AllExtensions); EXPECT_FALSE(service_->IsExtensionEnabled(good_crx)); EXPECT_FALSE(service_->IsIncognitoEnabled(good_crx)); extension_sync_data.enabled = true; extension_sync_data.incognito_enabled = true; - service_->ProcessSyncData(extension_sync_data, &AlwaysInstall); + service_->ProcessSyncData(extension_sync_data, &AllExtensions); EXPECT_TRUE(service_->IsExtensionEnabled(good_crx)); EXPECT_TRUE(service_->IsIncognitoEnabled(good_crx)); extension_sync_data.enabled = false; extension_sync_data.incognito_enabled = true; - service_->ProcessSyncData(extension_sync_data, &AlwaysInstall); + service_->ProcessSyncData(extension_sync_data, &AllExtensions); EXPECT_FALSE(service_->IsExtensionEnabled(good_crx)); EXPECT_TRUE(service_->IsIncognitoEnabled(good_crx)); @@ -3510,7 +3609,7 @@ TEST_F(ExtensionServiceTest, ProcessSyncDataVersionCheck) { *(service_->GetExtensionById(good_crx, true)->version()); // Should do nothing if extension version == sync version. - service_->ProcessSyncData(extension_sync_data, &AlwaysInstall); + service_->ProcessSyncData(extension_sync_data, &AllExtensions); EXPECT_FALSE(service_->updater()->WillCheckSoon()); // Should do nothing if extension version > sync version (but see @@ -3518,7 +3617,7 @@ TEST_F(ExtensionServiceTest, ProcessSyncDataVersionCheck) { { scoped_ptr<Version> version(Version::GetVersionFromString("0.0.0.0")); extension_sync_data.version = *version; - service_->ProcessSyncData(extension_sync_data, &AlwaysInstall); + service_->ProcessSyncData(extension_sync_data, &AllExtensions); EXPECT_FALSE(service_->updater()->WillCheckSoon()); } @@ -3526,7 +3625,7 @@ TEST_F(ExtensionServiceTest, ProcessSyncDataVersionCheck) { { scoped_ptr<Version> version(Version::GetVersionFromString("9.9.9.9")); extension_sync_data.version = *version; - service_->ProcessSyncData(extension_sync_data, &AlwaysInstall); + service_->ProcessSyncData(extension_sync_data, &AllExtensions); EXPECT_TRUE(service_->updater()->WillCheckSoon()); } @@ -3545,7 +3644,7 @@ TEST_F(ExtensionServiceTest, ProcessSyncDataNotInstalled) { extension_sync_data.version = *version; } - service_->ProcessSyncData(extension_sync_data, &AlwaysInstall); + service_->ProcessSyncData(extension_sync_data, &AllExtensions); EXPECT_TRUE(service_->updater()->WillCheckSoon()); PendingExtensionInfo info; diff --git a/chrome/browser/extensions/extension_sync_data.h b/chrome/browser/extensions/extension_sync_data.h index 3a29d22..c454946 100644 --- a/chrome/browser/extensions/extension_sync_data.h +++ b/chrome/browser/extensions/extension_sync_data.h @@ -29,6 +29,9 @@ struct ExtensionSyncData { // version of the currenty-installed extension matches |version|). Version version; GURL update_url; + + // Used only for debugging. + std::string name; }; #endif // CHROME_BROWSER_EXTENSIONS_EXTENSION_SYNC_DATA_H_ diff --git a/chrome/browser/extensions/extension_updater_unittest.cc b/chrome/browser/extensions/extension_updater_unittest.cc index 277329c..817de10 100644 --- a/chrome/browser/extensions/extension_updater_unittest.cc +++ b/chrome/browser/extensions/extension_updater_unittest.cc @@ -4,6 +4,7 @@ #include <map> +#include "base/compiler_specific.h" #include "base/file_util.h" #include "base/memory/scoped_ptr.h" #include "base/stl_util-inl.h" @@ -15,8 +16,9 @@ #include "base/version.h" #include "chrome/browser/extensions/extension_error_reporter.h" #include "chrome/browser/extensions/extension_updater.h" -#include "chrome/browser/extensions/extension_service.h" +#include "chrome/browser/extensions/extension_sync_data.h" #include "chrome/browser/extensions/test_extension_prefs.h" +#include "chrome/browser/extensions/test_extension_service.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/common/extensions/extension.h" #include "chrome/common/extensions/extension_constants.h" @@ -48,97 +50,18 @@ const ManifestFetchData::PingData kNeverPingedData( } // namespace // Base class for further specialized test classes. -class MockService : public ExtensionServiceInterface { +class MockService : public TestExtensionService { public: MockService() : pending_extension_manager_(ALLOW_THIS_IN_INITIALIZER_LIST(*this)) {} virtual ~MockService() {} - virtual const ExtensionList* extensions() const { - ADD_FAILURE(); - return NULL; - } - - virtual const ExtensionList* disabled_extensions() const { - ADD_FAILURE(); - return NULL; - } - - virtual void UpdateExtension(const std::string& id, - const FilePath& path, - const GURL& download_url) { - FAIL(); - } - - virtual const Extension* GetExtensionById(const std::string& id, - bool include_disabled) const { - ADD_FAILURE(); - return NULL; - } - - virtual bool UninstallExtension(const std::string& extension_id, - bool external_uninstall, - std::string* error) { - ADD_FAILURE(); - return false; - } - - virtual bool IsExtensionEnabled(const std::string& extension_id) const { - ADD_FAILURE(); - return false; - } - - virtual bool IsExternalExtensionUninstalled( - const std::string& extension_id) const { - ADD_FAILURE(); - return false; - } - - virtual void EnableExtension(const std::string& extension_id) { - FAIL(); - } - - virtual void DisableExtension(const std::string& extension_id) { - FAIL(); - } - - - virtual void UpdateExtensionBlacklist( - const std::vector<std::string>& blacklist) { - FAIL(); - } - - virtual void CheckAdminBlacklist() { - FAIL(); - } - - virtual bool IsIncognitoEnabled(const std::string& id) const { - ADD_FAILURE(); - return false; - } - - virtual void SetIsIncognitoEnabled(const std::string& id, - bool enabled) { - FAIL(); - } - - virtual void CheckForUpdatesSoon() { - FAIL(); - } - - virtual PendingExtensionManager* pending_extension_manager() { + virtual PendingExtensionManager* pending_extension_manager() OVERRIDE { ADD_FAILURE() << "Subclass should override this if it will " << "be accessed by a test."; return &pending_extension_manager_; } - virtual void ProcessSyncData( - const ExtensionSyncData& extension_sync_data, - PendingExtensionInfo::ShouldAllowInstallPredicate - should_allow_install) { - FAIL(); - } - Profile* profile() { return &profile_; } ExtensionPrefs* extension_prefs() { return prefs_.prefs(); } @@ -228,8 +151,8 @@ class ServiceForManifestTests : public MockService { virtual ~ServiceForManifestTests() {} - virtual const Extension* GetExtensionById(const std::string& id, - bool include_disabled) const { + virtual const Extension* GetExtensionById( + const std::string& id, bool include_disabled) const OVERRIDE { for (ExtensionList::const_iterator iter = extensions_.begin(); iter != extensions_.end(); ++iter) { if ((*iter)->id() == id) { @@ -239,9 +162,11 @@ class ServiceForManifestTests : public MockService { return NULL; } - virtual const ExtensionList* extensions() const { return &extensions_; } + virtual const ExtensionList* extensions() const OVERRIDE { + return &extensions_; + } - virtual PendingExtensionManager* pending_extension_manager() { + virtual PendingExtensionManager* pending_extension_manager() OVERRIDE { return &pending_extension_manager_; } @@ -263,11 +188,12 @@ class ServiceForDownloadTests : public MockService { download_url_ = download_url; } - virtual PendingExtensionManager* pending_extension_manager() { + virtual PendingExtensionManager* pending_extension_manager() OVERRIDE { return &pending_extension_manager_; } - virtual const Extension* GetExtensionById(const std::string& id, bool) const { + virtual const Extension* GetExtensionById( + const std::string& id, bool) const OVERRIDE { last_inquired_extension_id_ = id; return NULL; } @@ -298,7 +224,7 @@ class ServiceForBlacklistTests : public MockService { processed_blacklist_(false) { } virtual void UpdateExtensionBlacklist( - const std::vector<std::string>& blacklist) { + const std::vector<std::string>& blacklist) OVERRIDE { processed_blacklist_ = true; return; } diff --git a/chrome/browser/extensions/mock_extension_service.cc b/chrome/browser/extensions/mock_extension_service.cc deleted file mode 100644 index d26b02a..0000000 --- a/chrome/browser/extensions/mock_extension_service.cc +++ /dev/null @@ -1,9 +0,0 @@ -// Copyright (c) 2011 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/mock_extension_service.h" - -MockExtensionService::MockExtensionService() {} - -MockExtensionService::~MockExtensionService() {} diff --git a/chrome/browser/extensions/mock_extension_service.h b/chrome/browser/extensions/mock_extension_service.h deleted file mode 100644 index 68325a5..0000000 --- a/chrome/browser/extensions/mock_extension_service.h +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright (c) 2011 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. - -#ifndef CHROME_BROWSER_EXTENSIONS_MOCK_EXTENSION_SERVICE_H_ -#define CHROME_BROWSER_EXTENSIONS_MOCK_EXTENSION_SERVICE_H_ -#pragma once - -#include <string> -#include <vector> - -#include "chrome/browser/extensions/extension_service.h" -// Needed to keep gmock happy. -#include "chrome/browser/extensions/extension_sync_data.h" -#include "testing/gmock/include/gmock/gmock.h" - -class MockExtensionService : public ExtensionServiceInterface { - public: - MockExtensionService(); - virtual ~MockExtensionService(); - - MOCK_CONST_METHOD0(extensions, const ExtensionList*()); - MOCK_CONST_METHOD0(disabled_extensions, const ExtensionList*()); - MOCK_METHOD0(pending_extension_manager, PendingExtensionManager*()); - MOCK_METHOD3(UpdateExtension, void(const std::string&, - const FilePath&, - const GURL&)); - MOCK_CONST_METHOD2(GetExtensionById, - const Extension*(const std::string&, bool)); - MOCK_METHOD3(UninstallExtension, - bool(const std::string&, bool, std::string*)); - MOCK_CONST_METHOD1(IsExtensionEnabled, bool(const std::string&)); - MOCK_CONST_METHOD1(IsExternalExtensionUninstalled, - bool(const std::string&)); - MOCK_METHOD1(EnableExtension, void(const std::string&)); - MOCK_METHOD1(DisableExtension, void(const std::string&)); - MOCK_METHOD1(UpdateExtensionBlacklist, - void(const std::vector<std::string>&)); - MOCK_METHOD0(CheckAdminBlacklist, void()); - MOCK_CONST_METHOD1(IsIncognitoEnabled, bool(const std::string&)); - MOCK_METHOD2(SetIsIncognitoEnabled, void(const std::string&, bool)); - MOCK_METHOD0(CheckForUpdatesSoon, void()); - MOCK_METHOD2(ProcessSyncData, - void(const ExtensionSyncData&, - PendingExtensionInfo::ShouldAllowInstallPredicate - should_allow_install)); -}; - -#endif // CHROME_BROWSER_EXTENSIONS_MOCK_EXTENSION_SERVICE_H_ diff --git a/chrome/browser/extensions/test_extension_service.cc b/chrome/browser/extensions/test_extension_service.cc new file mode 100644 index 0000000..46e7544 --- /dev/null +++ b/chrome/browser/extensions/test_extension_service.cc @@ -0,0 +1,75 @@ +// Copyright (c) 2011 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_sync_data.h" +#include "chrome/browser/extensions/test_extension_service.h" +#include "testing/gtest/include/gtest/gtest.h" + +TestExtensionService::~TestExtensionService() {} + +const ExtensionList* TestExtensionService::extensions() const { + ADD_FAILURE(); + return NULL; +} + +PendingExtensionManager* TestExtensionService::pending_extension_manager() { + ADD_FAILURE(); + return NULL; +} + +void TestExtensionService::UpdateExtension(const std::string& id, + const FilePath& path, + const GURL& download_url) { + ADD_FAILURE(); +} + +const Extension* TestExtensionService::GetExtensionById( + const std::string& id, bool include_disabled) const { + ADD_FAILURE(); + return NULL; +} + +bool TestExtensionService::IsExtensionEnabled( + const std::string& extension_id) const { + ADD_FAILURE(); + return false; +} + +bool TestExtensionService::IsExternalExtensionUninstalled( + const std::string& extension_id) const { + ADD_FAILURE(); + return false; +} + +void TestExtensionService::UpdateExtensionBlacklist( + const std::vector<std::string>& blacklist) { + ADD_FAILURE(); +} + +void TestExtensionService::CheckAdminBlacklist() { + ADD_FAILURE(); +} + +void TestExtensionService::CheckForUpdatesSoon() { + ADD_FAILURE(); +} + +bool TestExtensionService::GetSyncData( + const std::string& id, ExtensionFilter filter, + ExtensionSyncData* extension_sync_data) const { + ADD_FAILURE(); + return false; +} + +std::vector<ExtensionSyncData> TestExtensionService::GetSyncDataList( + ExtensionFilter filter) const { + ADD_FAILURE(); + return std::vector<ExtensionSyncData>(); +} + +void TestExtensionService::ProcessSyncData( + const ExtensionSyncData& extension_sync_data, + ExtensionFilter filter) { + ADD_FAILURE(); +} diff --git a/chrome/browser/extensions/test_extension_service.h b/chrome/browser/extensions/test_extension_service.h new file mode 100644 index 0000000..9f18b16 --- /dev/null +++ b/chrome/browser/extensions/test_extension_service.h @@ -0,0 +1,48 @@ +// Copyright (c) 2011 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. + +#ifndef CHROME_BROWSER_EXTENSIONS_TEST_EXTENSION_SERVICE_H_ +#define CHROME_BROWSER_EXTENSIONS_TEST_EXTENSION_SERVICE_H_ +#pragma once + +#include <string> +#include <vector> + +#include "chrome/browser/extensions/extension_service.h" + +// Implemention of ExtensionServiceInterface with default +// implementations for methods that add failures. You should subclass +// this and override the methods you care about. +class TestExtensionService : public ExtensionServiceInterface { + public: + virtual ~TestExtensionService(); + + // ExtensionServiceInterface implementation. + virtual const ExtensionList* extensions() const OVERRIDE; + virtual PendingExtensionManager* pending_extension_manager() OVERRIDE; + virtual void UpdateExtension(const std::string& id, + const FilePath& path, + const GURL& download_url) OVERRIDE; + virtual const Extension* GetExtensionById( + const std::string& id, bool include_disabled) const OVERRIDE; + virtual bool IsExtensionEnabled( + const std::string& extension_id) const OVERRIDE; + virtual bool IsExternalExtensionUninstalled( + const std::string& extension_id) const OVERRIDE; + + virtual void UpdateExtensionBlacklist( + const std::vector<std::string>& blacklist) OVERRIDE; + virtual void CheckAdminBlacklist() OVERRIDE; + virtual void CheckForUpdatesSoon() OVERRIDE; + virtual bool GetSyncData( + const std::string& id, ExtensionFilter filter, + ExtensionSyncData* extension_sync_data) const OVERRIDE; + virtual std::vector<ExtensionSyncData> GetSyncDataList( + ExtensionFilter filter) const OVERRIDE; + virtual void ProcessSyncData( + const ExtensionSyncData& extension_sync_data, + ExtensionFilter filter) OVERRIDE; +}; + +#endif // CHROME_BROWSER_EXTENSIONS_TEST_EXTENSION_SERVICE_H_ diff --git a/chrome/browser/sync/glue/extension_change_processor.cc b/chrome/browser/sync/glue/extension_change_processor.cc index dd0da34..3600c55 100644 --- a/chrome/browser/sync/glue/extension_change_processor.cc +++ b/chrome/browser/sync/glue/extension_change_processor.cc @@ -8,13 +8,11 @@ #include <string> #include "base/logging.h" -#include "base/stl_util-inl.h" #include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_sync_data.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/sync/glue/extension_sync.h" #include "chrome/browser/sync/glue/extension_util.h" -#include "chrome/browser/sync/profile_sync_service.h" #include "chrome/browser/sync/protocol/extension_specifics.pb.h" #include "chrome/common/extensions/extension.h" #include "content/browser/browser_thread.h" @@ -29,8 +27,7 @@ ExtensionChangeProcessor::ExtensionChangeProcessor( : ChangeProcessor(error_handler), traits_(traits), profile_(NULL), - extension_service_(NULL), - user_share_(NULL) { + extension_service_(NULL) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); DCHECK(error_handler); } @@ -49,9 +46,7 @@ void ExtensionChangeProcessor::Observe(NotificationType type, DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); DCHECK(running()); DCHECK(profile_); - if ((type != NotificationType::EXTENSION_INSTALLED) && - (type != NotificationType::EXTENSION_UNINSTALLED) && - (type != NotificationType::EXTENSION_LOADED) && + if ((type != NotificationType::EXTENSION_LOADED) && (type != NotificationType::EXTENSION_UPDATE_DISABLED) && (type != NotificationType::EXTENSION_UNLOADED)) { LOG(DFATAL) << "Received unexpected notification of type " @@ -59,36 +54,36 @@ void ExtensionChangeProcessor::Observe(NotificationType type, return; } + // Filter out unhandled extensions first. DCHECK_EQ(Source<Profile>(source).ptr(), profile_); - if (type == NotificationType::EXTENSION_UNINSTALLED) { - const UninstalledExtensionInfo* uninstalled_extension_info = - Details<UninstalledExtensionInfo>(details).ptr(); - CHECK(uninstalled_extension_info); - if (traits_.should_handle_extension_uninstall( - *uninstalled_extension_info)) { - const std::string& id = uninstalled_extension_info->extension_id; + const Extension& extension = + (type == NotificationType::EXTENSION_UNLOADED) ? + *Details<UnloadedExtensionInfo>(details)->extension : + *Details<const Extension>(details).ptr(); + if (!traits_.is_valid_and_syncable(extension)) { + return; + } + + const std::string& id = extension.id(); + + // Then handle extension uninstalls. + if (type == NotificationType::EXTENSION_UNLOADED) { + const UnloadedExtensionInfo& info = + *Details<UnloadedExtensionInfo>(details).ptr(); + if (info.reason == UnloadedExtensionInfo::UNINSTALL) { VLOG(1) << "Removing server data for uninstalled extension " << id - << " of type " << uninstalled_extension_info->extension_type; - RemoveServerData(traits_, id, user_share_); - } - } else { - const Extension* extension = NULL; - if (type == NotificationType::EXTENSION_UNLOADED) { - extension = Details<UnloadedExtensionInfo>(details)->extension; - } else { - extension = Details<const Extension>(details).ptr(); - } - CHECK(extension); - VLOG(1) << "Updating server data for extension " << extension->id() - << " (notification type = " << type.value << ")"; - if (!traits_.is_valid_and_syncable(*extension)) { + << " of type " << info.extension->GetType(); + RemoveServerData(traits_, id, share_handle()); return; } - std::string error; - if (!UpdateServerData(traits_, *extension, *extension_service_, - user_share_, &error)) { - error_handler()->OnUnrecoverableError(FROM_HERE, error); - } + } + + VLOG(1) << "Updating server data for extension " << id + << " (notification type = " << type.value << ")"; + std::string error; + if (!UpdateServerData(traits_, id, *extension_service_, + share_handle(), &error)) { + error_handler()->OnUnrecoverableError(FROM_HERE, error); } } @@ -131,7 +126,7 @@ void ExtensionChangeProcessor::ApplyChangesFromSyncModel( } } ExtensionSyncData sync_data; - if (!GetExtensionSyncData(specifics, &sync_data)) { + if (!SpecificsToSyncData(specifics, &sync_data)) { // TODO(akalin): Should probably recover or drop. std::string error = std::string("Invalid server specifics: ") + @@ -152,10 +147,8 @@ void ExtensionChangeProcessor::StartImpl(Profile* profile) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); profile_ = profile; extension_service_ = profile_->GetExtensionService(); - user_share_ = profile_->GetProfileSyncService()->GetUserShare(); DCHECK(profile_); DCHECK(extension_service_); - DCHECK(user_share_); StartObserving(); } @@ -164,18 +157,11 @@ void ExtensionChangeProcessor::StopImpl() { StopObserving(); profile_ = NULL; extension_service_ = NULL; - user_share_ = NULL; } void ExtensionChangeProcessor::StartObserving() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); DCHECK(profile_); - notification_registrar_.Add( - this, NotificationType::EXTENSION_INSTALLED, - Source<Profile>(profile_)); - notification_registrar_.Add( - this, NotificationType::EXTENSION_UNINSTALLED, - Source<Profile>(profile_)); notification_registrar_.Add( this, NotificationType::EXTENSION_LOADED, diff --git a/chrome/browser/sync/glue/extension_change_processor.h b/chrome/browser/sync/glue/extension_change_processor.h index cf75a2c..2c05732 100644 --- a/chrome/browser/sync/glue/extension_change_processor.h +++ b/chrome/browser/sync/glue/extension_change_processor.h @@ -61,7 +61,6 @@ class ExtensionChangeProcessor : public ChangeProcessor, // Non-NULL iff |running()| is true. Profile* profile_; ExtensionServiceInterface* extension_service_; - sync_api::UserShare* user_share_; DISALLOW_COPY_AND_ASSIGN(ExtensionChangeProcessor); }; diff --git a/chrome/browser/sync/glue/extension_sync.cc b/chrome/browser/sync/glue/extension_sync.cc index 1aedc4a..1d26a50 100644 --- a/chrome/browser/sync/glue/extension_sync.cc +++ b/chrome/browser/sync/glue/extension_sync.cc @@ -61,57 +61,25 @@ ExtensionData* SetOrCreateExtensionData( return extension_data; } -// Reads the client data for each extension in |extensions| to be -// synced and updates |extension_data_map|. Puts all unsynced -// extensions in |unsynced_extensions|. -void ReadClientDataFromExtensionList( - const ExtensionList& extensions, - IsValidAndSyncablePredicate is_valid_and_syncable, - const ExtensionServiceInterface& extensions_service, - std::set<std::string>* unsynced_extensions, - ExtensionDataMap* extension_data_map) { - for (ExtensionList::const_iterator it = extensions.begin(); - it != extensions.end(); ++it) { - CHECK(*it); - const Extension& extension = **it; - if (is_valid_and_syncable(extension)) { - sync_pb::ExtensionSpecifics client_specifics; - GetExtensionSpecifics(extension, extensions_service, - &client_specifics); - DcheckIsExtensionSpecificsValid(client_specifics); - const ExtensionData& extension_data = - *SetOrCreateExtensionData( - extension_data_map, ExtensionData::CLIENT, - true, client_specifics); - DcheckIsExtensionSpecificsValid(extension_data.merged_data()); - // Assumes this is called before any server data is read. - DCHECK(extension_data.NeedsUpdate(ExtensionData::SERVER)); - DCHECK(!extension_data.NeedsUpdate(ExtensionData::CLIENT)); - } else { - unsynced_extensions->insert(extension.id()); - } - } -} - -// Simply calls ReadClientDataFromExtensionList() on the list of -// enabled and disabled extensions from |extensions_service|. +// Fills in |extension_data_map| with data from +// extension_service.GetSyncDataList(). void SlurpClientData( IsValidAndSyncablePredicate is_valid_and_syncable, - const ExtensionServiceInterface& extensions_service, - std::set<std::string>* unsynced_extensions, + const ExtensionServiceInterface& extension_service, ExtensionDataMap* extension_data_map) { - const ExtensionList* extensions = extensions_service.extensions(); - CHECK(extensions); - ReadClientDataFromExtensionList( - *extensions, is_valid_and_syncable, extensions_service, - unsynced_extensions, extension_data_map); - - const ExtensionList* disabled_extensions = - extensions_service.disabled_extensions(); - CHECK(disabled_extensions); - ReadClientDataFromExtensionList( - *disabled_extensions, is_valid_and_syncable, extensions_service, - unsynced_extensions, extension_data_map); + std::vector<ExtensionSyncData> sync_data_list = + extension_service.GetSyncDataList(is_valid_and_syncable); + for (std::vector<ExtensionSyncData>::const_iterator it = + sync_data_list.begin(); + it != sync_data_list.end(); ++it) { + sync_pb::ExtensionSpecifics client_specifics; + SyncDataToSpecifics(*it, &client_specifics); + const ExtensionData& extension_data = + *SetOrCreateExtensionData( + extension_data_map, ExtensionData::CLIENT, + true, client_specifics); + DcheckIsExtensionSpecificsValid(extension_data.merged_data()); + } } // Gets the boilerplate error message for not being able to find a @@ -131,7 +99,6 @@ std::string GetRootNodeDoesNotExistError(const char* root_node_tag) { bool SlurpServerData( const char* root_node_tag, const ExtensionSpecificsGetter extension_specifics_getter, - const std::set<std::string>& unsynced_extensions, sync_api::UserShare* user_share, ExtensionDataMap* extension_data_map) { sync_api::WriteTransaction trans(user_share); @@ -154,19 +121,12 @@ bool SlurpServerData( LOG(ERROR) << "Invalid extensions specifics for id " << id; return false; } - // Don't process server data for extensions we know are - // unsyncable. This doesn't catch everything, as if we don't - // have the extension already installed we can't check, but we - // also check at extension install time. - if (unsynced_extensions.find(server_data.id()) == - unsynced_extensions.end()) { - // Pass in false for merge_user_properties so client user - // settings always take precedence. - const ExtensionData& extension_data = - *SetOrCreateExtensionData( - extension_data_map, ExtensionData::SERVER, false, server_data); - DcheckIsExtensionSpecificsValid(extension_data.merged_data()); - } + // Pass in false for merge_user_properties so client user + // settings always take precedence. + const ExtensionData& extension_data = + *SetOrCreateExtensionData( + extension_data_map, ExtensionData::SERVER, false, server_data); + DcheckIsExtensionSpecificsValid(extension_data.merged_data()); id = sync_node.GetSuccessorId(); } return true; @@ -175,20 +135,17 @@ bool SlurpServerData( } // namespace bool SlurpExtensionData(const ExtensionSyncTraits& traits, - const ExtensionServiceInterface& extensions_service, + const ExtensionServiceInterface& extension_service, sync_api::UserShare* user_share, ExtensionDataMap* extension_data_map) { - std::set<std::string> unsynced_extensions; - - // Read client-side data first so server data takes precedence, and - // also so we have an idea of which extensions are unsyncable. + // Read client-side data first so server data takes precedence. SlurpClientData( - traits.is_valid_and_syncable, extensions_service, - &unsynced_extensions, extension_data_map); + traits.is_valid_and_syncable, extension_service, + extension_data_map); if (!SlurpServerData( traits.root_node_tag, traits.extension_specifics_getter, - unsynced_extensions, user_share, extension_data_map)) { + user_share, extension_data_map)) { return false; } return true; @@ -239,7 +196,7 @@ bool UpdateServer( bool FlushExtensionData(const ExtensionSyncTraits& traits, const ExtensionDataMap& extension_data_map, - ExtensionServiceInterface* extensions_service, + ExtensionServiceInterface* extension_service, sync_api::UserShare* user_share) { sync_api::WriteTransaction trans(user_share); sync_api::ReadNode root(&trans); @@ -262,34 +219,33 @@ bool FlushExtensionData(const ExtensionSyncTraits& traits, } DCHECK(!extension_data.NeedsUpdate(ExtensionData::SERVER)); ExtensionSyncData sync_data; - if (!GetExtensionSyncData(extension_data.merged_data(), &sync_data)) { + if (!SpecificsToSyncData(extension_data.merged_data(), &sync_data)) { // TODO(akalin): Should probably recover or drop. NOTREACHED(); return false; } - extensions_service->ProcessSyncData(sync_data, - traits.is_valid_and_syncable); + extension_service->ProcessSyncData(sync_data, + traits.is_valid_and_syncable); } return true; } bool UpdateServerData(const ExtensionSyncTraits& traits, - const Extension& extension, - const ExtensionServiceInterface& extensions_service, + const std::string& id, + const ExtensionServiceInterface& extension_service, sync_api::UserShare* user_share, std::string* error) { - const std::string& id = extension.id(); - if (!traits.is_valid_and_syncable(extension)) { + ExtensionSyncData data; + if (!extension_service.GetSyncData( + id, traits.is_valid_and_syncable, &data)) { *error = std::string("UpdateServerData() called for invalid or " "unsyncable extension ") + id; LOG(DFATAL) << *error; return false; } - sync_pb::ExtensionSpecifics client_data; - GetExtensionSpecifics(extension, extensions_service, - &client_data); + SyncDataToSpecifics(data, &client_data); DcheckIsExtensionSpecificsValid(client_data); ExtensionData extension_data = ExtensionData::FromData(ExtensionData::CLIENT, client_data); diff --git a/chrome/browser/sync/glue/extension_sync.h b/chrome/browser/sync/glue/extension_sync.h index e1f3f15..78ab5a2 100644 --- a/chrome/browser/sync/glue/extension_sync.h +++ b/chrome/browser/sync/glue/extension_sync.h @@ -12,7 +12,6 @@ #include <map> #include <string> -class Extension; class ExtensionServiceInterface; class Profile; class ProfileSyncService; @@ -47,7 +46,7 @@ bool RootNodeHasChildren(const char* tag, // extensions to be synced. Returns true iff this was successful; if // unsuccessful, the contents of |extension_data_map| are undefined. bool SlurpExtensionData(const ExtensionSyncTraits& traits, - const ExtensionServiceInterface& extensions_service, + const ExtensionServiceInterface& extension_service, sync_api::UserShare* user_share, ExtensionDataMap* extension_data_map); @@ -59,15 +58,15 @@ bool SlurpExtensionData(const ExtensionSyncTraits& traits, // function is returned is that the updates were successfully started. bool FlushExtensionData(const ExtensionSyncTraits& traits, const ExtensionDataMap& extension_data_map, - ExtensionServiceInterface* extensions_service, + ExtensionServiceInterface* extension_service, sync_api::UserShare* user_share); // Updates the server data for the given extension. Returns true iff // this was successful; if unsuccessful, an error string is put into // |error|. bool UpdateServerData(const ExtensionSyncTraits& traits, - const Extension& extension, - const ExtensionServiceInterface& extensions_service, + const std::string& id, + const ExtensionServiceInterface& extension_service, sync_api::UserShare* user_share, std::string* error); diff --git a/chrome/browser/sync/glue/extension_util.cc b/chrome/browser/sync/glue/extension_util.cc index 5578f89..72401f1 100644 --- a/chrome/browser/sync/glue/extension_util.cc +++ b/chrome/browser/sync/glue/extension_util.cc @@ -8,9 +8,7 @@ #include "base/logging.h" #include "base/memory/scoped_ptr.h" -#include "base/stl_util-inl.h" #include "base/version.h" -#include "chrome/browser/extensions/extension_prefs.h" #include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_sync_data.h" #include "chrome/browser/sync/protocol/extension_specifics.pb.h" @@ -142,22 +140,6 @@ bool AreExtensionSpecificsNonUserPropertiesEqual( a_non_user_properties, b_non_user_properties); } -void GetExtensionSpecifics(const Extension& extension, - const ExtensionServiceInterface& extension_service, - sync_pb::ExtensionSpecifics* specifics) { - DCHECK(IsExtensionValid(extension)); - const std::string& id = extension.id(); - bool enabled = extension_service.IsExtensionEnabled(id); - bool incognito_enabled = extension_service.IsIncognitoEnabled(id); - specifics->set_id(id); - specifics->set_version(extension.VersionString()); - specifics->set_update_url(extension.update_url().spec()); - specifics->set_enabled(enabled); - specifics->set_incognito_enabled(incognito_enabled); - specifics->set_name(extension.name()); - DcheckIsExtensionSpecificsValid(*specifics); -} - void MergeExtensionSpecifics( const sync_pb::ExtensionSpecifics& specifics, bool merge_user_properties, @@ -182,7 +164,7 @@ void MergeExtensionSpecifics( } } -bool GetExtensionSyncData( +bool SpecificsToSyncData( const sync_pb::ExtensionSpecifics& specifics, ExtensionSyncData* sync_data) { if (!Extension::IdIsValid(specifics.id())) { @@ -206,7 +188,21 @@ bool GetExtensionSyncData( sync_data->version = *version; sync_data->enabled = specifics.enabled(); sync_data->incognito_enabled = specifics.incognito_enabled(); + sync_data->name = specifics.name(); return true; } +void SyncDataToSpecifics( + const ExtensionSyncData& sync_data, + sync_pb::ExtensionSpecifics* specifics) { + DCHECK(Extension::IdIsValid(sync_data.id)); + DCHECK(!sync_data.uninstalled); + specifics->set_id(sync_data.id); + specifics->set_update_url(sync_data.update_url.spec()); + specifics->set_version(sync_data.version.GetString()); + specifics->set_enabled(sync_data.enabled); + specifics->set_incognito_enabled(sync_data.incognito_enabled); + specifics->set_name(sync_data.name); +} + } // namespace browser_sync diff --git a/chrome/browser/sync/glue/extension_util.h b/chrome/browser/sync/glue/extension_util.h index d914a75..7f16cfe 100644 --- a/chrome/browser/sync/glue/extension_util.h +++ b/chrome/browser/sync/glue/extension_util.h @@ -12,10 +12,8 @@ #include <string> class Extension; -class ExtensionPrefs; class ExtensionServiceInterface; struct ExtensionSyncData; -struct UninstalledExtensionInfo; namespace sync_pb { class ExtensionSpecifics; @@ -78,13 +76,6 @@ bool AreExtensionSpecificsNonUserPropertiesEqual( const sync_pb::ExtensionSpecifics& a, const sync_pb::ExtensionSpecifics& b); -// Fills |specifics| with information taken from |extension|, which -// must be a syncable extension. |specifics| will be valid after this -// function is called. -void GetExtensionSpecifics(const Extension& extension, - const ExtensionServiceInterface& extension_service, - sync_pb::ExtensionSpecifics* specifics); - // Merge |specifics| into |merged_specifics|. Both must be valid and // have the same ID. The merge policy is currently to copy the // non-user properties of |specifics| into |merged_specifics| (and the @@ -97,10 +88,15 @@ void MergeExtensionSpecifics( // Fills |sync_data| with the data from |specifics|. Returns true iff // succesful. -bool GetExtensionSyncData( +bool SpecificsToSyncData( const sync_pb::ExtensionSpecifics& specifics, ExtensionSyncData* sync_data); +// Fills |specifics| with the data from |sync_data|. +void SyncDataToSpecifics( + const ExtensionSyncData& sync_data, + sync_pb::ExtensionSpecifics* specifics); + } // namespace browser_sync #endif // CHROME_BROWSER_SYNC_GLUE_EXTENSION_UTIL_H_ diff --git a/chrome/browser/sync/glue/extension_util_unittest.cc b/chrome/browser/sync/glue/extension_util_unittest.cc index 35a4065..e7494c4 100644 --- a/chrome/browser/sync/glue/extension_util_unittest.cc +++ b/chrome/browser/sync/glue/extension_util_unittest.cc @@ -6,7 +6,7 @@ #include "base/file_path.h" #include "base/values.h" -#include "chrome/browser/extensions/mock_extension_service.h" +#include "chrome/browser/extensions/extension_sync_data.h" #include "chrome/browser/sync/protocol/extension_specifics.pb.h" #include "chrome/common/extensions/extension.h" #include "chrome/common/extensions/extension_constants.h" @@ -17,10 +17,6 @@ namespace browser_sync { namespace { -using ::testing::_; -using ::testing::Return; -using ::testing::StrictMock; - #if defined(OS_WIN) const FilePath::CharType kExtensionFilePath[] = FILE_PATH_LITERAL("c:\\foo"); #elif defined(OS_POSIX) @@ -381,48 +377,6 @@ TEST_F(ExtensionUtilTest, AreExtensionSpecificsNonUserPropertiesEqual) { EXPECT_TRUE(AreExtensionSpecificsNonUserPropertiesEqual(a, b)); } -scoped_refptr<Extension> MakeSyncableExtension( - const std::string& version_string, - const std::string& update_url_spec, - const std::string& name, - const FilePath& extension_path) { - DictionaryValue source; - source.SetString(extension_manifest_keys::kVersion, version_string); - source.SetString(extension_manifest_keys::kUpdateURL, update_url_spec); - source.SetString(extension_manifest_keys::kName, name); - std::string error; - scoped_refptr<Extension> extension = Extension::Create( - extension_path, Extension::INTERNAL, source, - Extension::STRICT_ERROR_CHECKS, &error); - EXPECT_TRUE(extension); - EXPECT_EQ("", error); - return extension; -} - -TEST_F(ExtensionUtilTest, GetExtensionSpecifics) { - FilePath file_path(kExtensionFilePath); - StrictMock<MockExtensionService> mock_extension_service; - EXPECT_CALL(mock_extension_service, IsExtensionEnabled(_)) - .WillOnce(Return(true)); - EXPECT_CALL(mock_extension_service, IsIncognitoEnabled(_)) - .WillOnce(Return(false)); - - scoped_refptr<Extension> extension( - MakeSyncableExtension( - kValidVersion, kValidUpdateUrl1, kName, file_path)); - sync_pb::ExtensionSpecifics specifics; - GetExtensionSpecifics(*extension, mock_extension_service, &specifics); - EXPECT_EQ(extension->id(), specifics.id()); - EXPECT_EQ(extension->VersionString(), kValidVersion); - EXPECT_EQ(extension->update_url().spec(), kValidUpdateUrl1); - EXPECT_TRUE(specifics.enabled()); - EXPECT_FALSE(specifics.incognito_enabled()); - EXPECT_EQ(kName, specifics.name()); -} - -// TODO(akalin): Make ExtensionService/ExtensionUpdater testable -// enough to be able to write a unittest for SetExtensionProperties(). - TEST_F(ExtensionUtilTest, MergeExtensionSpecificsWithUserProperties) { sync_pb::ExtensionSpecifics merged_specifics; merged_specifics.set_id(kValidId); @@ -483,6 +437,47 @@ TEST_F(ExtensionUtilTest, MergeExtensionSpecificsWithUserProperties) { } } +TEST_F(ExtensionUtilTest, SpecificsToSyncData) { + sync_pb::ExtensionSpecifics specifics; + specifics.set_id(kValidId); + specifics.set_update_url(kValidUpdateUrl2); + specifics.set_enabled(false); + specifics.set_incognito_enabled(true); + specifics.set_version(kVersion1); + specifics.set_name(kName); + + ExtensionSyncData sync_data; + EXPECT_TRUE(SpecificsToSyncData(specifics, &sync_data)); + EXPECT_EQ(specifics.id(), sync_data.id); + EXPECT_EQ(specifics.version(), sync_data.version.GetString()); + EXPECT_EQ(specifics.update_url(), sync_data.update_url.spec()); + EXPECT_EQ(specifics.enabled(), sync_data.enabled); + EXPECT_EQ(specifics.incognito_enabled(), sync_data.incognito_enabled); + EXPECT_EQ(specifics.name(), sync_data.name); +} + +TEST_F(ExtensionUtilTest, SyncDataToSpecifics) { + ExtensionSyncData sync_data; + sync_data.id = kValidId; + sync_data.update_url = GURL(kValidUpdateUrl2); + sync_data.enabled = true; + sync_data.incognito_enabled = false; + { + scoped_ptr<Version> version(Version::GetVersionFromString(kVersion1)); + sync_data.version = *version; + } + sync_data.name = kName; + + sync_pb::ExtensionSpecifics specifics; + SyncDataToSpecifics(sync_data, &specifics); + EXPECT_EQ(sync_data.id, specifics.id()); + EXPECT_EQ(sync_data.update_url.spec(), specifics.update_url()); + EXPECT_EQ(sync_data.enabled, specifics.enabled()); + EXPECT_EQ(sync_data.incognito_enabled, specifics.incognito_enabled()); + EXPECT_EQ(sync_data.version.GetString(), specifics.version()); + EXPECT_EQ(sync_data.name, specifics.name()); +} + } // namespace } // namespace browser_sync diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index a954ff9..3f76afd 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -56,13 +56,13 @@ 'browser/autofill/autofill_common_test.h', 'browser/autofill/data_driven_test.cc', 'browser/autofill/data_driven_test.h', - 'browser/extensions/mock_extension_service.cc', - 'browser/extensions/mock_extension_service.h', 'browser/automation/mock_tab_event_observer.cc', 'browser/automation/mock_tab_event_observer.h', # The only thing used from browser is Browser::Type. 'browser/extensions/test_extension_prefs.cc', 'browser/extensions/test_extension_prefs.h', + 'browser/extensions/test_extension_service.cc', + 'browser/extensions/test_extension_service.h', 'browser/mock_browsing_data_appcache_helper.cc', 'browser/mock_browsing_data_appcache_helper.h', 'browser/mock_browsing_data_database_helper.cc', diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h index e2309c3..31a8f0d 100644 --- a/chrome/common/extensions/extension.h +++ b/chrome/common/extensions/extension.h @@ -960,6 +960,10 @@ struct ExtensionInfo { // Struct used for the details of the EXTENSION_UNINSTALLED // notification. +// +// TODO(akalin): Now that sync doesn't need to listen to +// EXTENSION_UNINSTALLED, everything but |extension_id| can be +// removed. struct UninstalledExtensionInfo { explicit UninstalledExtensionInfo(const Extension& extension); ~UninstalledExtensionInfo(); |