Extensions: Remove temporary cleanup code from crbug.com/558299

In the past, there was a bug where some themes incorrectly got synced
into the EXTENSIONS data type, so we added cleanup code to remove the
bad data. That was long ago and all bad data should be long gone now,
so let's get rid of the cleanup code.

Bug: none
Change-Id: I53fcc8ecb208e9fce6565cda70ec57cc960f9b6c
Reviewed-on: https://chromium-review.googlesource.com/1124680Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572255}

chrome/browser/extensions/extension_service.cc

@@ -1400,10 +1400,6 @@ void ExtensionService::OnExtensionInstalled(
       pending_extension_manager()->GetById(id);
   if (pending_extension_info) {
     if (!pending_extension_info->ShouldAllowInstall(extension)) {
-      // Hack for crbug.com/558299, see comment on DeleteThemeDoNotUse.
-      if (extension->is_theme() && pending_extension_info->is_from_sync())
-        ExtensionSyncService::Get(profile_)->DeleteThemeDoNotUse(*extension);
-
       pending_extension_manager()->Remove(id);
 
       LOG(WARNING) << "ShouldAllowInstall() returned false for " << id

chrome/browser/extensions/extension_sync_service.cc

@@ -57,11 +57,11 @@ bool IsCorrectSyncType(const Extension& extension, syncer::ModelType type) {
 }
 
 // Predicate for PendingExtensionManager.
-// TODO(treib,devlin): The !is_theme check shouldn't be necessary anymore after
-// all the bad data from crbug.com/558299 has been cleaned up, after M52 or so.
 bool ShouldAllowInstall(const Extension* extension) {
-  return !extension->is_theme() &&
-         extensions::sync_helper::IsSyncable(extension);
+  // Note: In the past, there was a bug where some themes incorrectly ended up
+  // here, see https://crbug.com/558299. Make sure that doesn't happen anymore.
+  CHECK(!extension->is_theme());
+  return extensions::sync_helper::IsSyncable(extension);
 }
 
 syncer::SyncDataList ToSyncerSyncDataList(
@@ -493,11 +493,8 @@ void ExtensionSyncService::ApplySyncData(
     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())) {
+            id, extension_sync_data.update_url(), extension_sync_data.version(),
+            ShouldAllowInstall, 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
@@ -563,12 +560,6 @@ void ExtensionSyncService::SetSyncStartFlareForTesting(
   flare_ = flare;
 }
 
-void ExtensionSyncService::DeleteThemeDoNotUse(const Extension& theme) {
-  DCHECK(theme.is_theme());
-  GetSyncBundle(syncer::EXTENSIONS)->PushSyncDeletion(
-      theme.id(), CreateSyncData(theme).GetSyncData());
-}
-
 extensions::ExtensionService* ExtensionSyncService::extension_service() const {
   return ExtensionSystem::Get(profile_)->extension_service();
 }

chrome/browser/extensions/extension_sync_service.h

@@ -67,12 +67,6 @@ class ExtensionSyncService : public syncer::SyncableService,
   void SetSyncStartFlareForTesting(
       const syncer::SyncableService::StartSyncFlare& flare);
 
-  // Special hack: There was a bug where themes incorrectly ended up in the
-  // syncer::EXTENSIONS type. This is for cleaning up the data. crbug.com/558299
-  // DO NOT USE FOR ANYTHING ELSE!
-  // TODO(treib,devlin): Remove this after M52 or so.
-  void DeleteThemeDoNotUse(const extensions::Extension& theme);
-
  private:
   FRIEND_TEST_ALL_PREFIXES(TwoClientAppsSyncTest, UnexpectedLaunchType);
   FRIEND_TEST_ALL_PREFIXES(ExtensionDisabledGlobalErrorTest,