Replace the WAS_INSTALLED_BY_CUSTODIAN creation flag with a pref

chrome/browser/extensions/extension_sync_service.cc

 // Copyright 2013 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_service.h"
 
 #include <utility>
 
 #include "base/auto_reset.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 16 matching lines @@
 #include "extensions/browser/uninstall_reason.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_set.h"
 #include "extensions/common/image_util.h"
 #include "extensions/common/permissions/permission_message_provider.h"
 #include "extensions/common/permissions/permissions_data.h"
 #include "sync/api/sync_change.h"
 #include "sync/api/sync_error_factory.h"
 
 #if defined(ENABLE_SUPERVISED_USERS)
+#include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/supervised_user/supervised_user_service.h"
 #include "chrome/browser/supervised_user/supervised_user_service_factory.h"
 #endif
 
 using extensions::AppSorting;
 using extensions::Extension;
 using extensions::ExtensionPrefs;
 using extensions::ExtensionRegistry;
 using extensions::ExtensionSet;
 using extensions::ExtensionSyncData;
@@ skipping 145 matching lines @@
       << "Got " << type << " ModelType";
 
   SyncBundle* bundle = GetSyncBundle(type);
   bundle->StartSyncing(std::move(sync_processor));
 
   // Apply the initial sync data, filtering out any items where we have more
   // recent local changes. Also tell the SyncBundle the extension IDs.
   for (const syncer::SyncData& sync_data : initial_sync_data) {
     std::unique_ptr<ExtensionSyncData> extension_sync_data(
         ExtensionSyncData::CreateFromSyncData(sync_data));
+

[Marc Treib, 2016/06/13 09:37:14]

Please avoid unnecessary changes like this.

[mamir, 2016/06/13 11:30:16]

Sorry.
Overlooked it after removing a LOG statement.
Done!

     // If the extension has local state that needs to be synced, ignore this
     // change (we assume the local state is more recent).
     if (extension_sync_data &&
         !ExtensionPrefs::Get(profile_)->NeedsSync(extension_sync_data->id())) {
       ApplySyncData(*extension_sync_data);
     }
   }
 
   // Now push the local state to sync.
   // Note: We'd like to only send out changes for extensions which have
@@ skipping 96 matching lines @@
   }
   return result;
 }
 
 void ExtensionSyncService::ApplySyncData(
     const ExtensionSyncData& extension_sync_data) {
   // Ignore any pref change notifications etc. while we're applying incoming
   // sync data, so that we don't end up notifying ourselves.
   base::AutoReset<bool> ignore_updates(&ignore_updates_, true);
 
+#if defined(ENABLE_SUPERVISED_USERS)

[Marc Treib, 2016/06/13 09:37:14]

I don't think this needs an #ifdef. All the other code that deals with that flag
isn't #ifdef'd either.

[mamir, 2016/06/13 11:30:16]

Done.

+  if (extension_sync_data.installed_by_custodian()) {
+    extensions::util::SetIsInstalledByCustodian(extension_sync_data.id(),
+                                                profile_, true);
+  }
+#endif
+
   syncer::ModelType type = extension_sync_data.is_app() ? syncer::APPS
                                                         : syncer::EXTENSIONS;
   const std::string& id = extension_sync_data.id();
   SyncBundle* bundle = GetSyncBundle(type);
   DCHECK(bundle->IsSyncing());
   // Note: |extension| may be null if it hasn't been installed yet.
   const Extension* extension =
       ExtensionRegistry::Get(profile_)->GetInstalledExtension(id);
   if (extension && !IsCorrectSyncType(*extension, type)) {
     // The installed item isn't the same type as the sync data item, so we need
@@ skipping 169 matching lines @@
     pending_updates_[id] =
         PendingUpdate(extension_sync_data.version(), reenable_after_update);
     check_for_updates = true;
   } else if (state == NOT_INSTALLED) {
     if (!extension_service()->pending_extension_manager()->AddFromSync(
             id,
             extension_sync_data.update_url(),
             extension_sync_data.version(),
             ShouldAllowInstall,
             extension_sync_data.remote_install(),
             extension_sync_data.installed_by_custodian())) {

[Marc Treib, 2016/06/13 09:37:14]

Is it still necessary to pass the installed_by_custodian flag here? If you set
the pref above, it should get applied during installation anyway, right?

[mamir, 2016/06/13 11:30:16]

It not necessary anymore, you are right.
Done!

       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
       // never be removed (we'll never install a syncable version of the
       // extension), so that GetAllSyncData() continues to send it.
     }
     // Track pending extensions so that we can return them in GetAllSyncData().
     bundle->AddPendingExtensionData(extension_sync_data);
     check_for_updates = true;
   }
@@ skipping 185 matching lines @@
       sync_data_list->push_back(CreateSyncData(*extension));
     }
   }
 }
 
 bool ExtensionSyncService::ShouldSync(const Extension& extension) const {
   // Themes are handled by the ThemeSyncableService.
   return extensions::util::ShouldSync(&extension, profile_) &&
          !extension.is_theme();
 }