Replace the WAS_INSTALLED_BY_CUSTODIAN creation flag with a pref

chrome/browser/extensions/extension_sync_service.cc

Index: chrome/browser/extensions/extension_sync_service.cc
diff --git a/chrome/browser/extensions/extension_sync_service.cc b/chrome/browser/extensions/extension_sync_service.cc
index 86824c6673779da2f85b79f15c1ccf6b72c6b467..359390fd7f9cd7839fe82b7632f6b2229c9475f1 100644
--- a/chrome/browser/extensions/extension_sync_service.cc
+++ b/chrome/browser/extensions/extension_sync_service.cc
@@ -34,6 +34,7 @@
 #include "sync/api/sync_error_factory.h"
 
 #if defined(ENABLE_SUPERVISED_USERS)
+#include "chrome/browser/extensions/extension_util.h"

[Marc Treib, 2016/06/17 08:56:54]

This needs to go out of the #ifdef

[mamir, 2016/06/17 16:12:54]

Done.

 #include "chrome/browser/supervised_user/supervised_user_service.h"
 #include "chrome/browser/supervised_user/supervised_user_service_factory.h"
 #endif
@@ -280,18 +281,21 @@ ExtensionSyncData ExtensionSyncService::CreateSyncData(
       extension_prefs->HasDisableReason(id, Extension::DISABLE_REMOTE_INSTALL);
   ExtensionSyncData::OptionalBoolean allowed_on_all_url =
       GetAllowedOnAllUrlsOptionalBoolean(id, profile_);
+  bool installed_by_custodian =
+      extensions::util::WasInstalledByCustodian(id, profile_);
   AppSorting* app_sorting = ExtensionSystem::Get(profile_)->app_sorting();
 
   ExtensionSyncData result = extension.is_app()
       ? ExtensionSyncData(
             extension, enabled, disable_reasons, incognito_enabled,
             remote_install, allowed_on_all_url,
+            installed_by_custodian,
             app_sorting->GetAppLaunchOrdinal(id),
             app_sorting->GetPageOrdinal(id),
             extensions::GetLaunchTypePrefValue(extension_prefs, id))
       : ExtensionSyncData(
             extension, enabled, disable_reasons, incognito_enabled,
-            remote_install, allowed_on_all_url);
+            remote_install, allowed_on_all_url, installed_by_custodian);
 
   // If there's a pending update, send the new version to sync instead of the
   // installed one.
@@ -315,6 +319,11 @@ void ExtensionSyncService::ApplySyncData(
   // sync data, so that we don't end up notifying ourselves.
   base::AutoReset<bool> ignore_updates(&ignore_updates_, true);
 
+  if (extension_sync_data.installed_by_custodian()) {

[Marc Treib, 2016/06/17 08:56:54]

Hm. Now that I see this, I kinda think we should trust the data coming from
Sync, and clear the pref if necessary. Otherwise, it's impossible to ever remove
the flag from Sync, because the Chrome client will always add it back.

Consider:
- Extension is custodian-installed.
- Custodian uninstalls it.
- Another machine gets the uninstall.
- On that other machine, SU initiates an install of the same extension.
- Back on this machine, we get a Sync update saying that it's now not installed
by custodian anymore, but Chrome ignores that, and adds the flag back to Sync.

So, would it break anything to clear the pref here if Sync says so?

[mamir, 2016/06/17 16:12:55]

I agree.
No thing should break because this flag is always set to true on the server not
the client.
Done!

+    extensions::util::SetWasInstalledByCustodian(extension_sync_data.id(),
+                                                 profile_);
+  }
+
   syncer::ModelType type = extension_sync_data.is_app() ? syncer::APPS
                                                         : syncer::EXTENSIONS;
   const std::string& id = extension_sync_data.id();
@@ -503,8 +512,7 @@ void ExtensionSyncService::ApplySyncData(
             extension_sync_data.update_url(),
             extension_sync_data.version(),
             ShouldAllowInstall,
-            extension_sync_data.remote_install(),
-            extension_sync_data.installed_by_custodian())) {
+            extension_sync_data.remote_install())) {
       LOG(WARNING) << "Could not add pending extension for " << id;
       // This means that the extension is already pending installation, with a
       // non-INTERNAL location.  Add to pending_sync_data, even though it will