Apps/Extensions Sync refactoring -- delete most of the old glue, implement new sync API.

chrome/browser/extensions/extension_service.cc

 // 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_service.h"
 
 #include <algorithm>
 #include <set>
 
 #include "base/basictypes.h"
@@ skipping 35 matching lines @@
 #include "chrome/browser/extensions/extension_web_ui.h"
 #include "chrome/browser/extensions/extension_webnavigation_api.h"
 #include "chrome/browser/extensions/external_extension_provider_impl.h"
 #include "chrome/browser/extensions/external_extension_provider_interface.h"
 #include "chrome/browser/extensions/pending_extension_manager.h"
 #include "chrome/browser/net/chrome_url_request_context.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search_engines/template_url_service.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
+#include "chrome/browser/sync/api/sync_change.h"
 #include "chrome/browser/themes/theme_service.h"
 #include "chrome/browser/themes/theme_service_factory.h"
 #include "chrome/browser/ui/webui/chrome_url_data_manager.h"
 #include "chrome/browser/ui/webui/favicon_source.h"
 #include "chrome/browser/ui/webui/ntp/shown_sections_handler.h"
 #include "chrome/common/child_process_logging.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
@@ skipping 78 matching lines @@
   return NOT_NEEDED;
 }
 
 static void ForceShutdownPlugin(const FilePath& plugin_path) {
   PluginProcessHost* plugin =
       PluginService::GetInstance()->FindNpapiPluginProcess(plugin_path);
   if (plugin)
     plugin->ForceShutdown();
 }
 
+static bool IsSyncableExtension(const Extension& extension) {
+  return extension.GetSyncType() == Extension::SYNC_TYPE_EXTENSION;
+}
+
+static bool IsSyncableApp(const Extension& extension) {
+  return extension.GetSyncType() == Extension::SYNC_TYPE_APP;
+}
+
 // Manages an ExtensionInstallUI for a particular extension.
 class SimpleExtensionLoadPrompt : public ExtensionInstallUI::Delegate {
  public:
   SimpleExtensionLoadPrompt(Profile* profile,
                             base::WeakPtr<ExtensionService> extension_service,
                             const Extension* extension);
   ~SimpleExtensionLoadPrompt();
 
   void ShowPrompt();
 
@@ skipping 346 matching lines @@
 const Extension* ExtensionService::GetInstalledAppForRenderer(
     int renderer_child_id) {
   InstalledAppMap::iterator i = installed_app_hosts_.find(renderer_child_id);
   if (i == installed_app_hosts_.end())
     return NULL;
   return i->second;
 }
 
 // static
 // This function is used to implement the command-line switch
-// --uninstall-extension.  The LOG statements within this function are used to
-// inform the user if the uninstall cannot be done.
+// --uninstall-extension, and to uninstall an extension via sync.  The LOG
+// statements within this function are used to inform the user if the uninstall
+// cannot be done.
 bool ExtensionService::UninstallExtensionHelper(
     ExtensionService* extensions_service,
     const std::string& extension_id) {
 
   const Extension* extension =
       extensions_service->GetInstalledExtension(extension_id);
 
   // We can't call UninstallExtension with an invalid extension ID.
   if (!extension) {
     LOG(WARNING) << "Attempted uninstallation of non-existent extension with "
@@ skipping 316 matching lines @@
     NotificationService::current()->Notify(
         chrome::NOTIFICATION_EXTENSION_UNINSTALL_NOT_ALLOWED,
         Source<Profile>(profile_),
         Details<const Extension>(extension));
     if (error != NULL) {
       *error = errors::kCannotUninstallManagedExtension;
     }
     return false;
   }
 
+  // Extract the data we need for sync now, but don't actually sync until we've
+  // completed the uninstallation.
+  SyncBundle* sync_bundle = GetSyncBundleForExtension(*extension);
+
+  SyncChange sync_change;
+  if (sync_bundle) {
+    ExtensionSyncData extension_sync_data(*extension,
+                                          IsExtensionEnabled(extension_id),
+                                          IsIncognitoEnabled(extension_id));
+    sync_change = extension_sync_data.GetSyncChange(SyncChange::ACTION_DELETE);
+  }
+
   UninstalledExtensionInfo uninstalled_extension_info(*extension);
 
   UMA_HISTOGRAM_ENUMERATION("Extensions.UninstallType",
                             extension->GetType(), 100);
   RecordPermissionMessagesHistogram(
       extension, "Extensions.Permissions_Uninstall");
 
   TemplateURLService* url_service =
       TemplateURLServiceFactory::GetForProfile(profile_);
   if (url_service)
@@ skipping 19 matching lines @@
 
   ClearExtensionData(extension_url);
   UntrackTerminatedExtension(extension_id);
 
   // Notify interested parties that we've uninstalled this extension.
   NotificationService::current()->Notify(
       chrome::NOTIFICATION_EXTENSION_UNINSTALLED,
       Source<Profile>(profile_),
       Details<UninstalledExtensionInfo>(&uninstalled_extension_info));
 
+  if (sync_bundle && sync_bundle->HasExtensionId(extension_id)) {
+    SyncChangeList sync_change_list(1);
+    sync_change_list[0] = sync_change;
+    sync_bundle->sync_processor->ProcessSyncChanges(
+        FROM_HERE, sync_change_list);
+    sync_bundle->synced_extensions.erase(extension_id);
+  }
+
   return true;
 }
 
 void ExtensionService::ClearExtensionData(const GURL& extension_url) {
   scoped_refptr<ExtensionDataDeleter> deleter(
       new ExtensionDataDeleter(profile_, extension_url));
   deleter->StartDeleting();
 }
 
 bool ExtensionService::IsExtensionEnabled(
@@ skipping 38 matching lines @@
   extensions_.push_back(make_scoped_refptr(extension));
   ExtensionList::iterator iter = std::find(disabled_extensions_.begin(),
                                            disabled_extensions_.end(),
                                            extension);
   disabled_extensions_.erase(iter);
 
   // Make sure any browser action contained within it is not hidden.
   extension_prefs_->SetBrowserActionVisibility(extension, true);
 
   NotifyExtensionLoaded(extension);
+
+  SyncExtensionUpdateIfNeeded(*extension);
 }
 
 void ExtensionService::DisableExtension(const std::string& extension_id) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // The extension may have been disabled already.
   if (!IsExtensionEnabled(extension_id))
     return;
 
   const Extension* extension = GetInstalledExtension(extension_id);
@@ skipping 16 matching lines @@
   if (iter != extensions_.end()) {
     extensions_.erase(iter);
   } else {
     iter = std::find(terminated_extensions_.begin(),
                      terminated_extensions_.end(),
                      extension);
     terminated_extensions_.erase(iter);
   }
 
   NotifyExtensionUnloaded(extension, UnloadedExtensionInfo::DISABLE);
+
+  SyncExtensionUpdateIfNeeded(*extension);
 }
 
 void ExtensionService::GrantPermissions(const Extension* extension) {
   CHECK(extension);
 
   // We only maintain the granted permissions prefs for extensions that can't
   // silently increase their permissions.
   if (extension->CanSilentlyIncreasePermissions())
     return;
 
@@ skipping 610 matching lines @@
 }
 
 void ExtensionService::CheckForUpdatesSoon() {
   if (updater()) {
     updater()->CheckSoon();
   } else {
     LOG(WARNING) << "CheckForUpdatesSoon() called with auto-update turned off";
   }
 }
 
-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 Extension& extension,
-    ExtensionFilter filter,
-    ExtensionSyncData* extension_sync_data) const {
-  if (!(*filter)(extension)) {
-    return false;
-  }
-  *extension_sync_data = GetSyncDataHelper(extension);
-  return true;
+void ExtensionService::SyncExtensionUpdateIfNeeded(const Extension& extension) {
+  SyncBundle* sync_bundle = GetSyncBundleForExtension(extension);
+  if (sync_bundle) {
+    SyncChangeList sync_change_list(1);
+    ExtensionSyncData extension_sync_data(extension,
+                                          IsExtensionEnabled(extension.id()),
+                                          IsIncognitoEnabled(extension.id()));
+
+    sync_change_list[0] = extension_sync_data.GetSyncChange(
+        sync_bundle->HasExtensionId(extension.id()) ?
+            SyncChange::ACTION_UPDATE : SyncChange::ACTION_ADD);
+    sync_bundle->sync_processor->ProcessSyncChanges(
+        FROM_HERE, sync_change_list);
+    sync_bundle->synced_extensions.insert(extension.id());
+    sync_bundle->pending_sync_data.erase(extension.id());
+  }
+}
+
+ExtensionService::SyncBundle* ExtensionService::GetSyncBundleForExtension(
+    const Extension& extension) {
+  if (app_sync_bundle_.filter(extension))
+    return &app_sync_bundle_;
+  else if (extension_sync_bundle_.filter(extension))
+    return &extension_sync_bundle_;
+  else
+    return NULL;
+}
+
+ExtensionService::SyncBundle*
+    ExtensionService::GetSyncBundleForExtensionSyncData(
+    const ExtensionSyncData& extension_sync_data) {
+  switch (extension_sync_data.type()) {
+    case Extension::SYNC_TYPE_APP:
+      return &app_sync_bundle_;
+    case Extension::SYNC_TYPE_EXTENSION:
+      return &extension_sync_bundle_;
+    default:
+      NOTREACHED();
+      return NULL;
+  }
+}
+
+const ExtensionService::SyncBundle* ExtensionService::GetSyncBundleForModelType(
+    syncable::ModelType type) const {
+  switch (type) {
+    case syncable::APPS:
+      return &app_sync_bundle_;
+    case syncable::EXTENSIONS:
+      return &extension_sync_bundle_;
+    default:
+      NOTREACHED();
+      return NULL;
+  }
+}
+
+ExtensionService::SyncBundle* ExtensionService::GetSyncBundleForModelType(
+    syncable::ModelType type) {
+  switch (type) {
+    case syncable::APPS:
+      return &app_sync_bundle_;
+    case syncable::EXTENSIONS:
+      return &extension_sync_bundle_;
+    default:
+      NOTREACHED();
+      return NULL;

[asargent_no_longer_on_chrome, 2011/08/12 20:50:39]

nit: Looks like these two have the same body. Maybe use a macro or templates or
something to eliminate the duplication?

+  }
+}
+
+SyncError ExtensionService::MergeDataAndStartSyncing(
+    syncable::ModelType type,
+    const SyncDataList& initial_sync_data,
+    SyncChangeProcessor* sync_processor) {
+
+  SyncBundle* bundle = NULL;
+
+  switch (type) {
+    case syncable::EXTENSIONS:
+      bundle = &extension_sync_bundle_;
+      bundle->filter = IsSyncableExtension;
+      break;
+
+    case syncable::APPS:
+      bundle = &app_sync_bundle_;
+      bundle->filter = IsSyncableApp;
+      break;
+
+    default:
+      DLOG(ERROR) << "Got " << type << " ModelType";
+      NOTREACHED();

[akalin, 2011/08/12 03:35:02]

may as well just do NOTREACHED() << "Got " ...;
Also return a suitable SyncError() here

[asargent_no_longer_on_chrome, 2011/08/12 20:50:39]

consider LOG(FATAL) 

+  }
+
+  bundle->sync_processor = sync_processor;

[asargent_no_longer_on_chrome, 2011/08/12 20:50:39]

consider adding a CHECK(sync_processor) right above this

+
+  for (SyncDataList::const_iterator i = initial_sync_data.begin();
+       i != initial_sync_data.end();
+       ++i) {
+    ExtensionSyncData extension_sync_data = ExtensionSyncData(*i);
+    bundle->synced_extensions.insert(extension_sync_data.id());
+    ProcessExtensionSyncData(extension_sync_data, *bundle);
+  }
+
+  SyncDataList sync_data_list = GetAllSyncData(type);
+  SyncChangeList sync_change_list;
+  for (SyncDataList::const_iterator i = sync_data_list.begin();
+       i != sync_data_list.end();
+       ++i) {
+    if (bundle->HasExtensionId(i->GetTag()))
+      sync_change_list.push_back(SyncChange(SyncChange::ACTION_UPDATE, *i));

[asargent_no_longer_on_chrome, 2011/08/12 20:50:39]

I was a bit surprised not to see you check here whether the local data about
this extension differs from what sync has, and if not then avoid adding an entry
to sync_change_list. Does the sync system skip pushing unnecessary changes up to
the server for you?

[Ben Olmstead, 2011/08/15 20:20:12]

According to Fred, yes it does.  I considered the case, but did not want to add
more code if the sync engine already handled it.

+    else
+      sync_change_list.push_back(SyncChange(SyncChange::ACTION_ADD, *i));
+  }
+  bundle->sync_processor->ProcessSyncChanges(FROM_HERE, sync_change_list);
+
+  return SyncError();
+}
+
+void ExtensionService::StopSyncing(syncable::ModelType type) {
+  SyncBundle* bundle = GetSyncBundleForModelType(type);
+  DCHECK(bundle);

[akalin, 2011/08/12 03:35:02]

change to CHECK, since you're dereferencing in next line anyway

+  // This is the simplest way to clear out the bundle.
+  *bundle = SyncBundle();
+}
+
+SyncDataList ExtensionService::GetAllSyncData(syncable::ModelType type) const {
+  const SyncBundle* bundle = GetSyncBundleForModelType(type);
+  DCHECK(bundle);

[akalin, 2011/08/12 03:35:02]

CHECK

+  std::vector<ExtensionSyncData> extension_sync_data = GetSyncDataList(*bundle);
+  SyncDataList rv(extension_sync_data.size());

[asargent_no_longer_on_chrome, 2011/08/12 20:50:39]

naming nit: "rv" is too abbreviated - maybe use "result" or something like that.

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#General_Naming...

+  for (int i = 0; i < static_cast<int>(extension_sync_data.size()); ++i) {

[akalin, 2011/08/12 03:35:02]

may as well use iterators here, too

+    rv[i] = extension_sync_data[i].GetSyncData();
+  }
+  return rv;
+}
+
+SyncError ExtensionService::ProcessSyncChanges(
+    const tracked_objects::Location& from_here,
+    const SyncChangeList& change_list) {
+  for (SyncChangeList::const_iterator i = change_list.begin();
+      i != change_list.end();
+      ++i) {
+    ExtensionSyncData extension_sync_data = ExtensionSyncData(*i);
+    SyncBundle* bundle = GetSyncBundleForExtensionSyncData(extension_sync_data);
+    DCHECK(bundle);

[akalin, 2011/08/12 03:35:02]

CHECK

+
+    if (extension_sync_data.uninstalled())
+      bundle->synced_extensions.erase(extension_sync_data.id());
+    else
+      bundle->synced_extensions.insert(extension_sync_data.id());
+    ProcessExtensionSyncData(extension_sync_data, *bundle);
+  }
+
+  return SyncError();
 }
 
 void ExtensionService::GetSyncDataListHelper(
     const ExtensionList& extensions,
-    ExtensionFilter filter,
+    const SyncBundle& bundle,
     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));
+    if (bundle.filter(extension) &&
+        // If we have pending extension data for this extension, then this
+        // version is out of date.  We'll sync back the version we got from
+        // sync.
+        !bundle.HasPendingExtensionId(extension.id())) {
+      sync_data_list->push_back(
+          ExtensionSyncData(extension,
+                            IsExtensionEnabled(extension.id()),
+                            IsIncognitoEnabled(extension.id())));
     }
   }
 }
 
 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);
-  GetSyncDataListHelper(terminated_extensions_, filter, &sync_data_list);
-  return sync_data_list;
-}
-
-void ExtensionService::ProcessSyncData(
+    const SyncBundle& bundle) const {
+  std::vector<ExtensionSyncData> extension_sync_list;
+  GetSyncDataListHelper(extensions_, bundle, &extension_sync_list);
+  GetSyncDataListHelper(disabled_extensions_, bundle, &extension_sync_list);
+  GetSyncDataListHelper(terminated_extensions_, bundle, &extension_sync_list);
+
+  for (std::map<std::string, ExtensionSyncData>::const_iterator i =
+           bundle.pending_sync_data.begin();
+       i != bundle.pending_sync_data.end();
+       ++i) {
+    extension_sync_list.push_back(i->second);
+  }
+
+  return extension_sync_list;
+}
+
+void ExtensionService::ProcessExtensionSyncData(
     const ExtensionSyncData& extension_sync_data,
-    ExtensionFilter filter) {
-  const std::string& id = extension_sync_data.id;
+    SyncBundle& bundle) {
+  const std::string& id = extension_sync_data.id();
 
   // Handle uninstalls first.
-  if (extension_sync_data.uninstalled) {
+  if (extension_sync_data.uninstalled()) {
     std::string error;
     if (!UninstallExtensionHelper(this, id)) {
       LOG(WARNING) << "Could not uninstall extension " << id
                    << " for sync";
     }
     return;
   }
 
   // Set user settings.
-  if (extension_sync_data.enabled) {
+  if (extension_sync_data.enabled()) {
     EnableExtension(id);
   } else {
     DisableExtension(id);
   }
-  SetIsIncognitoEnabled(id, extension_sync_data.incognito_enabled);
+  SetIsIncognitoEnabled(id, extension_sync_data.incognito_enabled());
 
   const Extension* extension = GetInstalledExtension(id);
   if (extension) {
     // If the extension is already installed, check if it's outdated.
-    int result = extension->version()->CompareTo(extension_sync_data.version);
+    int result = extension->version()->CompareTo(extension_sync_data.version());
     if (result < 0) {
       // Extension is outdated.
+      bundle.pending_sync_data[extension_sync_data.id()] = extension_sync_data;
       CheckForUpdatesSoon();
-    } else if (result > 0) {
-      // Sync version is outdated.  Do nothing for now, as sync code
-      // in other places will eventually update the sync data.
-      //
-      // TODO(akalin): Move that code here.
     }
   } else {
     // TODO(akalin): Replace silent update with a list of enabled
     // permissions.
+    //
+    // TODO(bolms): Install disabled if it came in from sync disabled.

[akalin, 2011/08/12 03:35:02]

i believe the DisableExtension(id) call above should do exactly that already

[Ben Olmstead, 2011/08/15 20:20:12]

You are correct--just verified.

     const bool kInstallSilently = true;
     if (!pending_extension_manager()->AddFromSync(
             id,
-            extension_sync_data.update_url,
-            filter,
+            extension_sync_data.update_url(),
+            bundle.filter,
             kInstallSilently)) {
       LOG(WARNING) << "Could not add pending extension for " << id;

[akalin, 2011/08/12 03:35:02]

From the comments re. when AddFromSync returns false, I think we can conclude
the extension is already pending, so maybe returning is the wrong thing here.

[Ben Olmstead, 2011/08/15 20:20:12]

Looking at the code, it looks like this can only happen in fairly strange cases.
 The most likely scenario is a new extension coming in both through enterprise
policy and sync on Chrome startup.  Thinking through the cases, it does look
like it should get added to pending_sync_data, although it will stay there
forever.

       return;
     }
+    // Track pending extensions so that we can return them in GetAllSyncData().
+    bundle.pending_sync_data[extension_sync_data.id()] = extension_sync_data;
     CheckForUpdatesSoon();
   }
 }
 
 bool ExtensionService::IsIncognitoEnabled(
     const std::string& extension_id) const {
   // If this is an existing component extension we always allow it to
   // work in incognito mode.
   const Extension* extension = GetInstalledExtension(extension_id);
   if (extension && extension->location() == Extension::COMPONENT)
@@ skipping 21 matching lines @@
 
   extension_prefs_->SetIsIncognitoEnabled(extension_id, enabled);
 
   // If the extension is enabled (and not terminated), unload and
   // reload it to update UI.
   const Extension* enabled_extension = GetExtensionById(extension_id, false);
   if (enabled_extension) {
     NotifyExtensionUnloaded(enabled_extension, UnloadedExtensionInfo::DISABLE);
     NotifyExtensionLoaded(enabled_extension);
   }
+
+  if (extension)
+    SyncExtensionUpdateIfNeeded(*extension);
 }
 
 bool ExtensionService::CanCrossIncognito(const Extension* extension) {
   // We allow the extension to see events and data from another profile iff it
   // uses "spanning" behavior and it has incognito access. "split" mode
   // extensions only see events for a matching profile.
   return IsIncognitoEnabled(extension->id()) &&
       !extension->incognito_split_mode();
 }
 
@@ skipping 249 matching lines @@
   bool disabled = extension_prefs_->IsExtensionDisabled(extension->id());
   if (disabled) {
     disabled_extensions_.push_back(scoped_extension);
     // TODO(aa): This seems dodgy. It seems that AddExtension() could get called
     // with a disabled extension for other reasons other than that an update was
     // disabled.
     NotificationService::current()->Notify(
         chrome::NOTIFICATION_EXTENSION_UPDATE_DISABLED,
         Source<Profile>(profile_),
         Details<const Extension>(extension));
+    SyncExtensionUpdateIfNeeded(*extension);
     return;
   }
 
   extensions_.push_back(scoped_extension);
+  SyncExtensionUpdateIfNeeded(*extension);
   NotifyExtensionLoaded(extension);
 }
 
 void ExtensionService::InitializePermissions(const Extension* extension) {
   // If the extension has used the optional permissions API, it will have a
   // custom set of active permissions defined in the extension prefs. Here,
   // we update the extension's active permissions based on the prefs.
   scoped_refptr<ExtensionPermissionSet> active_permissions =
       extension_prefs()->GetActivePermissions(extension->id());
 
@@ skipping 557 matching lines @@
 
 ExtensionService::NaClModuleInfoList::iterator
     ExtensionService::FindNaClModule(const GURL& url) {
   for (NaClModuleInfoList::iterator iter = nacl_module_list_.begin();
        iter != nacl_module_list_.end(); ++iter) {
     if (iter->url == url)
       return iter;
   }
   return nacl_module_list_.end();
 }