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..9743a60a09d8a776508231b2fa845d0dad1aac12 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,44 +64,52 @@ 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
-  // 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;
+  FRIEND_TEST_ALL_PREFIXES(TwoClientAppsSyncTest, UnexpectedLaunchType);
+  FRIEND_TEST_ALL_PREFIXES(ExtensionDisabledGlobalErrorTest,
+                           HigherPermissionsFromSync);
+  FRIEND_TEST_ALL_PREFIXES(ExtensionDisabledGlobalErrorTest, RemoteInstall);
+  friend class EphemeralAppBrowserTest;
 
   // 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;
+  // Applies the given change coming in from the server to the local state.
+  // Returns false if the changes were not completely applied and were added
+  // to the pending list.

[not at google - send to devlin, 2015/07/17 15:28:50]

"... They will be applied later when..?"

[Marc Treib, 2015/07/20 09:28:22]

Nope, currently it's not applied later, ever. I'm hoping to change that in the
next CL.
(At the moment, all "pending" means is "send this cached data back to Sync,
instead of the actual local state". It's supposed to handle the case where we
get an update from Sync saying "version 3" but we have version 2 installed.
It'll take us a while to update, and we don't want Sync flip-flopping meanwhile.
The implementation is quite broken though, big surprise there.)

+  bool ApplySyncData(const extensions::ExtensionSyncData& extension_sync_data);
+
+  // Collects the ExtensionSyncData for all installed apps or extensions.
+  // If |include_everything| is true, includes all installed extensions,
+  // otherwise only those 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 include_everything) const;
 
+  // Helper for GetLocalSyncDataList.
   void FillSyncDataList(
       const extensions::ExtensionSet& extensions,
       syncer::ModelType type,
+      bool include_everything,
       std::vector<extensions::ExtensionSyncData>* sync_data_list) const;
 
   // Handles applying the extension specific values in |extension_sync_data| to
-  // the current system.
-  // Returns false if the changes were not completely applied and need to be
-  // tried again later.
+  // the local state.
+  // Returns false if the changes were not completely applied.
   bool ApplyExtensionSyncDataHelper(
       const extensions::ExtensionSyncData& extension_sync_data,
       syncer::ModelType type);
@@ -123,12 +129,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.