[Sync] Refactor ExtensionService for sync

Move sync-related methods in ExtensionService to the interface
ExtensionServiceSyncInterface.  Add methods GetSyncData() and
GetSyncDataList() for sync to query.

Make extension/app sync code use new interface and methods.  Stop
listening to EXTENSION_INSTALLED notification as it is unneeded;
EXTENSION_LOADED/EXTENSION_UPDATE_DISABLED is called afterwards.

Clean up now-unneeded methods in ExtensionServiceInterface.

Stop keeping track of unsynced extensions in SlurpClientData().  We check it in SlurpServerData(), but we shouldn't be sending that data to the server in the first place.

Add OVERRIDE annotations to subclasses of ExtensionService* subclasses.

BUG=77995
TEST=

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

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

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();