Extension syncing: Introduce a NeedsSync pref

chrome/browser/extensions/extension_sync_service.h

Index: chrome/browser/extensions/extension_sync_service.h
diff --git a/chrome/browser/extensions/extension_sync_service.h b/chrome/browser/extensions/extension_sync_service.h
index 88e2c3c0f0dc309d1139a638332692fbdd8af367..5e1f83c6888965600821d512456376a3951f7f5f 100644
--- a/chrome/browser/extensions/extension_sync_service.h
+++ b/chrome/browser/extensions/extension_sync_service.h
@@ -8,13 +8,13 @@
 #include <string>
 #include <vector>
 
-#include "chrome/browser/extensions/pending_enables.h"
 #include "chrome/browser/extensions/sync_bundle.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/common/extension.h"
 #include "sync/api/syncable_service.h"
 
+class ExtensionService;
 class Profile;
 
 namespace extensions {
@@ -43,11 +43,9 @@ class ExtensionSyncService : public syncer::SyncableService,
   // Convenience function to get the ExtensionSyncService for a BrowserContext.
   static ExtensionSyncService* Get(content::BrowserContext* context);
 
+  // Notifies Sync that the given |extension| has been uninstalled.
   void SyncUninstallExtension(const extensions::Extension& extension);
 
-  void SyncEnableExtension(const extensions::Extension& extension);
-  void SyncDisableExtension(const extensions::Extension& extension);
-
   void SyncOrderingChange(const std::string& extension_id);
 
   // Notifies Sync (if needed) of a newly-installed extension or a change to
@@ -66,38 +64,41 @@ class ExtensionSyncService : public syncer::SyncableService,
       const tracked_objects::Location& from_here,
       const syncer::SyncChangeList& change_list) override;
 
-  // Creates the ExtensionSyncData for the given app/extension.
-  extensions::ExtensionSyncData CreateSyncData(
-      const extensions::Extension& extension) const;
-
   // Applies the change specified passed in by either ExtensionSyncData to the

[not at google - send to devlin, 2015/07/16 18:25:47]

This applies a change *from the sync server* right, not some locally generated
chane? Could you clarify that?

[Marc Treib, 2015/07/17 10:24:06]

Yes, "apply" = server -> local, "push" = local -> server. Comment improved,
hopefully. I also removed the "to be tried again" part, since that doesn't
happen.
(Also, with some of the SyncBundle changes, this doesn't need to be public
anymore. It's not supposed to be part of the public interface, it was just
called by the SyncBundle.)

   // current system.
   // Returns false if the changes were not completely applied and were added
   // to the pending list to be tried again.
   bool ApplySyncData(const extensions::ExtensionSyncData& extension_sync_data);
 
+  // Returns whether the extension with the given |extension_id| has local
+  // changes that need to be synced. This can only be true before sync has
+  // started, i.e. before MergeDataAndStartSyncing has been called.
+  bool ExtensionNeedsSync(const std::string& extension_id) const;
+
   // |flare| provides a StartSyncFlare to the SyncableService. See
   // sync_start_util for more. Public for testing.
   void SetSyncStartFlare(const syncer::SyncableService::StartSyncFlare& flare);
 
  private:
-  // Whether the given extension has been enabled before sync has started.
-  bool IsPendingEnable(const std::string& extension_id) const;
-
   // Gets the SyncBundle for the given |type|.
   extensions::SyncBundle* GetSyncBundle(syncer::ModelType type);
   const extensions::SyncBundle* GetSyncBundle(syncer::ModelType type) const;
 
-  // Gets the PendingEnables for apps/extensions.
-  extensions::PendingEnables* GetPendingEnables(bool for_apps);
+  // Creates the ExtensionSyncData for the given app/extension.
+  extensions::ExtensionSyncData CreateSyncData(
+      const extensions::Extension& extension) const;
 
-  // Gets the ExtensionSyncData for all apps or extensions.
-  std::vector<extensions::ExtensionSyncData> GetSyncDataList(
-      syncer::ModelType type) const;
+  // Collects the ExtensionSyncData for all installed apps or extensions.
+  // If |only_with_needs_sync| is true, includes only extensions that have the
+  // NeedsSync pref set, i.e. which have local changes that need to be pushed.
+  std::vector<extensions::ExtensionSyncData> GetLocalSyncDataList(
+      syncer::ModelType type, bool only_with_needs_sync) const;
 
+  // Helper for GetLocalSyncDataList.
   void FillSyncDataList(
       const extensions::ExtensionSet& extensions,
       syncer::ModelType type,
+      bool only_with_needs_sync,
       std::vector<extensions::ExtensionSyncData>* sync_data_list) const;
 
   // Handles applying the extension specific values in |extension_sync_data| to
@@ -123,12 +124,6 @@ class ExtensionSyncService : public syncer::SyncableService,
   extensions::SyncBundle app_sync_bundle_;
   extensions::SyncBundle extension_sync_bundle_;
 
-  // Set of extensions/apps that have been enabled before sync has started.
-  // TODO(treib,kalman): This seems wrong. Why are enables special, as opposed
-  // to disables, or any other changes?
-  extensions::PendingEnables pending_app_enables_;
-  extensions::PendingEnables pending_extension_enables_;
-
   // Run()ning tells sync to try and start soon, because syncable changes
   // have started happening. It will cause sync to call us back
   // asynchronously via MergeDataAndStartSyncing as soon as possible.