Remove migration code for domain scoped settings.

This CL removes the domain to origin scoped migration code added for
crbug.com/604612. The pref used for the migration is now cleared on
startup to ensure that dead data is not retained.

BUG=621398

Change-Id: I053091ae9e6e5f816a5e5ece84bd7f46882904fa
Reviewed-on: https://chromium-review.googlesource.com/765554Reviewed-by: Timothy Loh <timloh@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516089}

chrome/browser/content_settings/host_content_settings_map_factory.cc

@@ -7,14 +7,12 @@
 #include <utility>
 
 #include "base/feature_list.h"
-#include "chrome/browser/prefs/pref_service_syncable_util.h"
 #include "chrome/browser/profiles/off_the_record_profile_impl.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_features.h"
 #include "components/content_settings/core/browser/content_settings_pref_provider.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
-#include "components/sync_preferences/pref_service_syncable.h"
 #include "content/public/browser/browser_thread.h"
 #include "extensions/features/features.h"
 
@@ -89,14 +87,6 @@ scoped_refptr<RefcountedKeyedService>
       profile->GetProfileType() == Profile::GUEST_PROFILE,
       store_last_modified));
 
-  sync_preferences::PrefServiceSyncable* pref_service =
-      PrefServiceSyncableFromProfile(profile);
-  if (pref_service) {
-    pref_service->RegisterMergeDataFinishedCallback(
-        base::Bind(&HostContentSettingsMap::MigrateDomainScopedSettings,
-                   settings_map->GetWeakPtr(), true /* after_sync */));
-  }
-
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   // These must be registered before before the HostSettings are passed over to
   // the IOThread.  Simplest to do this on construction.

components/content_settings/core/browser/content_settings_pref_provider.cc

@@ -37,6 +37,9 @@ namespace content_settings {
 namespace {
 
 // These settings are no longer used, and should be deleted on profile startup.
+const char kObsoleteDomainToOriginMigrationStatus[] =
+    "profile.content_settings.domain_to_origin_migration_status";
+
 #if !defined(OS_IOS)
 const char kObsoleteFullscreenExceptionsPref[] =
     "profile.content_settings.exceptions.fullscreen";
@@ -70,6 +73,7 @@ void PrefProvider::RegisterProfilePrefs(
 
   // These prefs have been removed, but need to be registered so they can
   // be deleted on startup.
+  registry->RegisterIntegerPref(kObsoleteDomainToOriginMigrationStatus, 0);
 #if !defined(OS_IOS)
   registry->RegisterDictionaryPref(
       kObsoleteFullscreenExceptionsPref,
@@ -221,6 +225,9 @@ void PrefProvider::Notify(
 void PrefProvider::DiscardObsoletePreferences() {
   if (is_incognito_)
     return;
+
+  prefs_->ClearPref(kObsoleteDomainToOriginMigrationStatus);
+
   // These prefs were never stored on iOS/Android so they don't need to be
   // deleted.
 #if !defined(OS_IOS)

components/content_settings/core/browser/host_content_settings_map.cc

@@ -66,23 +66,6 @@ const ProviderNamesSourceMapEntry kProviderNamesSourceMap[] = {
     {"tests_other", content_settings::SETTING_SOURCE_USER},
 };
 
-// Enum describing the status of domain to origin migration of content settings.
-// Migration will be done twice: once upon construction of the
-// HostContentSettingsMap (before syncing any content settings) and once after
-// sync has finished. We always migrate before sync to ensure that settings will
-// get migrated even if a user doesn't have sync enabled. We migrate after sync
-// to ensure that any sync'd settings will be migrated. Once these events have
-// occurred, we won't perform migration again.
-enum DomainToOriginMigrationStatus {
-  // Haven't been migrated at all.
-  NOT_MIGRATED,
-  // Have done migration in the constructor of HostContentSettingsMap.
-  MIGRATED_BEFORE_SYNC,
-  // Have done migration both in HostContentSettingsMap construction and and
-  // after sync is finished. No migration will happen after this point.
-  MIGRATED_AFTER_SYNC,
-};
-
 static_assert(
     arraysize(kProviderNamesSourceMap) ==
         HostContentSettingsMap::NUM_PROVIDER_TYPES,
@@ -220,7 +203,6 @@ HostContentSettingsMap::HostContentSettingsMap(PrefService* prefs,
   default_provider->AddObserver(this);
   content_settings_providers_[DEFAULT_PROVIDER] = std::move(default_provider);
 
-  MigrateDomainScopedSettings(false);
   RecordExceptionMetrics();
 }
 
@@ -231,8 +213,6 @@ void HostContentSettingsMap::RegisterProfilePrefs(
   content_settings::ContentSettingsRegistry::GetInstance();
 
   registry->RegisterIntegerPref(prefs::kContentSettingsWindowLastTabIndex, 0);
-  registry->RegisterIntegerPref(prefs::kDomainToOriginMigrationStatus,
-                                NOT_MIGRATED);
 
   // Register the prefs for the content settings providers.
   content_settings::DefaultProvider::RegisterProfilePrefs(registry);
@@ -516,89 +496,6 @@ void HostContentSettingsMap::SetContentSettingDefaultScope(
                                resource_identifier, setting);
 }
 
-void HostContentSettingsMap::MigrateDomainScopedSettings(bool after_sync) {
-  DomainToOriginMigrationStatus status =
-      static_cast<DomainToOriginMigrationStatus>(
-          prefs_->GetInteger(prefs::kDomainToOriginMigrationStatus));
-  if (status == MIGRATED_AFTER_SYNC)
-    return;
-  if (status == MIGRATED_BEFORE_SYNC && !after_sync)
-    return;
-  DCHECK(status != NOT_MIGRATED || !after_sync);
-
-  const ContentSettingsType kDomainScopedTypes[] = {
-      CONTENT_SETTINGS_TYPE_IMAGES,
-      CONTENT_SETTINGS_TYPE_PLUGINS,
-      CONTENT_SETTINGS_TYPE_JAVASCRIPT,
-      CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS,
-      CONTENT_SETTINGS_TYPE_POPUPS};
-  for (const ContentSettingsType& type : kDomainScopedTypes) {
-    if (!content_settings::ContentSettingsRegistry::GetInstance()->Get(type))
-      continue;
-    ContentSettingsForOneType settings;
-    GetSettingsForOneType(type, std::string(), &settings);
-
-    for (const ContentSettingPatternSource& setting_entry : settings) {
-      // Migrate user preference settings only.
-      if (setting_entry.source != "preference")
-        continue;
-      // Migrate ALLOW settings only.
-      if (setting_entry.GetContentSetting() != CONTENT_SETTING_ALLOW)
-        continue;
-      // Skip default settings.
-      if (setting_entry.primary_pattern == ContentSettingsPattern::Wildcard())
-        continue;
-
-      if (setting_entry.secondary_pattern !=
-          ContentSettingsPattern::Wildcard()) {
-        NOTREACHED();
-        continue;
-      }
-
-      ContentSettingsPattern origin_pattern;
-      if (!ContentSettingsPattern::MigrateFromDomainToOrigin(
-              setting_entry.primary_pattern, &origin_pattern)) {
-        continue;
-      }
-
-      if (!origin_pattern.IsValid())
-        continue;
-
-      GURL origin(origin_pattern.ToString());
-      DCHECK(origin.is_valid());
-
-      // Ensure that the current resolved content setting for this origin is
-      // allowed. Otherwise we may be overriding some narrower setting which is
-      // set to block.
-      ContentSetting origin_setting =
-          GetContentSetting(origin, origin, type, std::string());
-
-      // Remove the domain scoped pattern. If |origin_setting| is not
-      // CONTENT_SETTING_ALLOW it implies there is some narrower pattern in
-      // effect, so it's still safe to remove the domain-scoped pattern.
-      SetContentSettingCustomScope(setting_entry.primary_pattern,
-                                   setting_entry.secondary_pattern, type,
-                                   std::string(), CONTENT_SETTING_DEFAULT);
-
-      // If the current resolved content setting is allowed it's safe to set the
-      // origin-scoped pattern.
-      if (origin_setting == CONTENT_SETTING_ALLOW)
-        SetContentSettingCustomScope(
-            ContentSettingsPattern::FromURLNoWildcard(origin),
-            ContentSettingsPattern::Wildcard(), type, std::string(),
-            CONTENT_SETTING_ALLOW);
-    }
-  }
-
-  if (status == NOT_MIGRATED) {
-    prefs_->SetInteger(prefs::kDomainToOriginMigrationStatus,
-                       MIGRATED_BEFORE_SYNC);
-  } else if (status == MIGRATED_BEFORE_SYNC) {
-    prefs_->SetInteger(prefs::kDomainToOriginMigrationStatus,
-                       MIGRATED_AFTER_SYNC);
-  }
-}
-
 base::WeakPtr<HostContentSettingsMap> HostContentSettingsMap::GetWeakPtr() {
   return weak_ptr_factory_.GetWeakPtr();
 }

components/content_settings/core/browser/host_content_settings_map.h

@@ -284,17 +284,6 @@ class HostContentSettingsMap : public content_settings::Observer,
   // Schedules any pending lossy website settings to be written to disk.
   void FlushLossyWebsiteSettings();
 
-  // Migrate old domain scoped ALLOW settings to be origin scoped for
-  // ContentSettingsTypes which are domain scoped. Only narrow down ALLOW
-  // domain settings to origins so that this will not cause privacy/security
-  // issues.
-  //
-  // |after_sync| will be false when called upon construction of this object and
-  // true when called by the sync layer after sync is completed.
-  // TODO(lshang): https://crbug.com/621398 Remove this when clients have
-  // migrated (~M56).
-  void MigrateDomainScopedSettings(bool after_sync);
-
   base::WeakPtr<HostContentSettingsMap> GetWeakPtr();
 
   // Injects a clock into the PrefProvider to allow control over the
@@ -308,12 +297,6 @@ class HostContentSettingsMap : public content_settings::Observer,
 
  private:
   friend class base::RefCountedThreadSafe<HostContentSettingsMap>;
-
-  FRIEND_TEST_ALL_PREFIXES(HostContentSettingsMapTest,
-                           DomainToOriginMigrationStatus);
-  FRIEND_TEST_ALL_PREFIXES(HostContentSettingsMapTest,
-                           MigrateDomainScopedSettings);
-
   friend class content_settings::TestUtils;
 
   ~HostContentSettingsMap() override;

components/content_settings/core/common/content_settings_pattern.cc

@@ -401,58 +401,6 @@ ContentSettingsPattern ContentSettingsPattern::FromString(
   return builder.Build();
 }
 
-// static
-bool ContentSettingsPattern::MigrateFromDomainToOrigin(
-    const ContentSettingsPattern& domain_pattern,
-    ContentSettingsPattern* origin_pattern) {
-  DCHECK(origin_pattern);
-
-  // Generated patterns with ::FromURL (which we want to migrate) must either
-  // have a scheme wildcard or be https.
-  if (domain_pattern.parts_.scheme != url::kHttpsScheme &&
-      !domain_pattern.parts_.is_scheme_wildcard) {
-    return false;
-  }
-
-  // Generated patterns using ::FromURL with the HTTPs scheme can not have a
-  // port wildcard.
-  if (domain_pattern.parts_.is_port_wildcard &&
-      domain_pattern.parts_.scheme == url::kHttpsScheme) {
-    return false;
-  }
-
-  // Patterns generated with ::FromURL will always have a domain wildcard. Those
-  // generated with ::FromURLNoWildcard don't.
-  if (!domain_pattern.parts_.has_domain_wildcard)
-    return false;
-
-  // Generated patterns with ::FromURL will always have a host.
-  if (domain_pattern.parts_.host.empty())
-    return false;
-
-  ContentSettingsPattern::Builder builder;
-  if (domain_pattern.parts_.is_scheme_wildcard)
-    builder.WithScheme(url::kHttpScheme);
-  else
-    builder.WithScheme(domain_pattern.parts_.scheme);
-
-  builder.WithHost(domain_pattern.parts_.host);
-
-  if (domain_pattern.parts_.is_port_wildcard) {
-    if (domain_pattern.parts_.scheme == url::kHttpsScheme) {
-      builder.WithPort(GetDefaultPort(url::kHttpsScheme));
-    } else {
-      builder.WithPort(GetDefaultPort(url::kHttpScheme));
-    }
-  } else {
-    builder.WithPort(domain_pattern.parts_.port);
-  }
-
-  *origin_pattern = builder.Build();
-
-  return true;
-}
-
 // static
 void ContentSettingsPattern::SetNonWildcardDomainNonPortSchemes(
     const char* const* schemes,

components/content_settings/core/common/content_settings_pattern.h

@@ -151,7 +151,6 @@ class ContentSettingsPattern {
 
   // Returns a pattern that matches the scheme and host of this URL, as well as
   // all subdomains and ports.
-  // TODO(lshang): Remove this when crbug.com/604612 is done.
   static ContentSettingsPattern FromURL(const GURL& url);
 
   // Returns a pattern that matches exactly this URL.
@@ -168,13 +167,6 @@ class ContentSettingsPattern {
   //   - [a:b:c:d:e:f:g:h] (matches an exact IPv6 ip)
   static ContentSettingsPattern FromString(const std::string& pattern_spec);
 
-  // Migrate domain scoped settings generated using FromURL() to be origin
-  // scoped. Return false if domain_pattern is not generated using FromURL().
-  // TODO(lshang): Remove this when migration is done. https://crbug.com/604612
-  static bool MigrateFromDomainToOrigin(
-      const ContentSettingsPattern& domain_pattern,
-      ContentSettingsPattern* origin_pattern);
-
   // Sets schemes that do not support domain wildcards and ports.
   // Needs to be called by the embedder before using ContentSettingsPattern.
   // |schemes| can't be NULL, and the pointed to strings must remain alive

components/content_settings/core/common/content_settings_pattern_unittest.cc

@@ -740,98 +740,6 @@ TEST(ContentSettingsPatternTest, CanonicalizePattern_Legacy) {
   EXPECT_STREQ("", Pattern("\xC4\x87ira.*").ToString().c_str());
 }
 
-TEST(ContentSettingsPatternTest, MigrateFromDomainToOrigin) {
-  ContentSettingsPattern origin_pattern;
-  // Http scheme patterns.
-  EXPECT_FALSE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("http://[*.]example.com"),
-      &origin_pattern));
-  EXPECT_FALSE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("http://[*.]example.com:80"),
-      &origin_pattern));
-
-  // Https patterns with port wildcard.
-  EXPECT_FALSE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("https://www.google.com"),
-      &origin_pattern));
-  EXPECT_FALSE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("https://[*.]google.com"),
-      &origin_pattern));
-
-  EXPECT_FALSE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("www.google.com"), &origin_pattern));
-
-  // Patterns with no domain wildcard.
-  EXPECT_FALSE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("*://www.google.com:8080"),
-      &origin_pattern));
-  EXPECT_FALSE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("www.example.com:8080"),
-      &origin_pattern));
-  EXPECT_FALSE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("www.google.com/*"), &origin_pattern));
-  EXPECT_FALSE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("google"), &origin_pattern));
-  EXPECT_FALSE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("https://www.google.com:443"),
-      &origin_pattern));
-
-  // Patterns with empty host.
-  EXPECT_FALSE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("*"), &origin_pattern));
-  EXPECT_FALSE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("[*.]"), &origin_pattern));
-  EXPECT_FALSE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("http://*"), &origin_pattern));
-  EXPECT_FALSE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("http://*:8080"), &origin_pattern));
-
-  // Other schemes and IP address patterns won't be migrated.
-  EXPECT_FALSE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("192.168.0.1"), &origin_pattern));
-  EXPECT_FALSE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("https://127.0.0.1"),
-      &origin_pattern));
-  EXPECT_FALSE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("http://[::1]"), &origin_pattern));
-  EXPECT_FALSE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("[::1]"), &origin_pattern));
-  EXPECT_FALSE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("file:///foo/bar.html"),
-      &origin_pattern));
-  EXPECT_FALSE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString(
-          "filesystem:http://www.google.com/temporary/"),
-      &origin_pattern));
-  EXPECT_FALSE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString(
-          "chrome-extension://peoadpeiejnhkmpaakpnompolbglelel/"),
-      &origin_pattern));
-  EXPECT_FALSE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("chrome-search://local-ntp/"),
-      &origin_pattern));
-
-  // These are pattern styles which might be generated using FromURL().
-  EXPECT_TRUE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("[*.]example.com"), &origin_pattern));
-  EXPECT_EQ("http://example.com:80", origin_pattern.ToString());
-
-  EXPECT_TRUE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("[*.]google.com:80"),
-      &origin_pattern));
-  EXPECT_EQ("http://google.com:80", origin_pattern.ToString());
-
-  EXPECT_TRUE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("[*.]example.com:123"),
-      &origin_pattern));
-  EXPECT_EQ("http://example.com:123", origin_pattern.ToString());
-
-  EXPECT_TRUE(ContentSettingsPattern::MigrateFromDomainToOrigin(
-      ContentSettingsPattern::FromString("https://[*.]google.com:443"),
-      &origin_pattern));
-  EXPECT_EQ("https://google.com:443", origin_pattern.ToString());
-}
-
 TEST(ContentSettingsPatternTest, Schemes) {
   EXPECT_EQ(ContentSettingsPattern::SCHEME_HTTP,
             Pattern("http://www.example.com").GetScheme());
@@ -857,4 +765,4 @@ TEST(ContentSettingsPatternTest, FileSchemeHasPath) {
   EXPECT_TRUE(Pattern("file:///foo").HasPath());
   EXPECT_TRUE(Pattern("file:///foo/bar/").HasPath());
   EXPECT_TRUE(Pattern("file:///foo/bar/test.html").HasPath());
-}
\ No newline at end of file
+}

components/content_settings/core/common/pref_names.cc

@@ -18,12 +18,6 @@ const char kContentSettingsVersion[] = "profile.content_settings.pref_version";
 const char kContentSettingsWindowLastTabIndex[] =
     "content_settings_window.last_tab_index";
 
-// Integer that indicates the status of migrating domain scoped settings to
-// origin scoped settings.
-// TODO(lshang): Remove this when all migration is done. See crbug.com/621398.
-const char kDomainToOriginMigrationStatus[] =
-    "profile.content_settings.domain_to_origin_migration_status";
-
 // Preferences that are exclusively used to store managed values for default
 // content settings.
 const char kManagedDefaultCookiesSetting[] =

components/content_settings/core/common/pref_names.h

@@ -16,7 +16,6 @@ namespace prefs {
 extern const char kBlockThirdPartyCookies[];
 extern const char kContentSettingsVersion[];
 extern const char kContentSettingsWindowLastTabIndex[];
-extern const char kDomainToOriginMigrationStatus[];
 
 extern const char kManagedDefaultCookiesSetting[];
 extern const char kManagedDefaultImagesSetting[];