Add migration code to change existing domain scoped content settings to be origin scoped

components/content_settings/core/common/content_settings_pattern.cc

 // Copyright (c) 2012 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 "components/content_settings/core/common/content_settings_pattern.h"
 
 #include <stddef.h>
 
 #include <memory>
 #include <vector>
@@ skipping 421 matching lines @@
 ContentSettingsPattern ContentSettingsPattern::FromString(
     const std::string& pattern_spec) {
   std::unique_ptr<ContentSettingsPattern::BuilderInterface> builder(
       ContentSettingsPattern::CreateBuilder(false));
   content_settings::PatternParser::Parse(pattern_spec,
                                          builder.get());
   return builder->Build();
 }
 
 // static
+bool ContentSettingsPattern::MigrateFromDomainToOrigin(
+    const ContentSettingsPattern& domain_pattern,
+    ContentSettingsPattern* origin_pattern) {
+  DCHECK(origin_pattern);
+
+  // Generated patterns 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 will either have a port wildcard or be https (but not

[msramek, 2016/06/30 16:16:22]

Not sure if this is just me, but this sentence reads to me like they should have
one XOR the other. That is not the case, because the HTTP scheme may or may not
have a port wildcard, which is also captured in the tests.

Meanwhile, the code says one NAND the other. Can we change the comment to e.g.
"Generated patterns with the HTTPs scheme can not have a port wildcard.", or
"Only generated patterns with a scheme wildcard can have a port wildcard", or
something like that?

[lshang, 2016/07/04 02:46:01]

Done.

+  // both).
+  if (domain_pattern.parts_.is_port_wildcard &&
+      domain_pattern.parts_.scheme == url::kHttpsScheme) {
+    return false;
+  }
+
+  // Generated patterns will always have a domain wildcard.

[msramek, 2016/06/30 16:16:22]

nit: Patterns generated with ::FromUrl (which we want to migrate). Those
generated with ::FromUrlNoWildcard don't.

[lshang, 2016/07/04 02:46:01]

Done.

+  if (!domain_pattern.parts_.has_domain_wildcard)
+    return false;
+
+  // Generated patterns will always have a host.
+  if (domain_pattern.parts_.host.empty())
+    return false;
+
+  *origin_pattern = domain_pattern;

[msramek, 2016/06/30 16:16:22]

Would it make sense to use Builder here instead of these in-place changes? If we
ever needed to change something in how a pattern is built, there would only be
one place to change.

[lshang, 2016/07/04 02:46:01]

Done.

+
+  if (domain_pattern.parts_.is_scheme_wildcard) {
+    (*origin_pattern).parts_.scheme = url::kHttpScheme;

[msramek, 2016/06/30 16:16:22]

The scheme wildcard exception was valid for both http and https. Shouldn't we
add both exceptions?

(although actually there aren't many sites that use both schemes, so this would
probably just pollute HCSM with duplicate entries)

[lshang, 2016/07/04 02:46:01]

That's a good point!

Patterns generated from permission request prompts for http sites will have
scheme wildcards, while for https sites the generated patterns will always have
a https scheme explicitly. 

I think for scheme wildcard patterns, even though the pattern system will also
make it take effect on https sites, the decision was made originally for http
sites, so it will be reasonable we now only add a http exception and make it
only affect http sites. 

And yes for the other reason you pointed out, this will avoid polluting HCSM and
confuse users (imagine how users will feel if there suddenly pop out a lot
duplicate entries and some of them don't even exist).

The drawback of this is that if a user added a scheme wildcard pattern (if a
pattern is like [*.]example.com, we can't actually distinguish if it's from the
prompt or user manually input) and wanted it to affect both schemes, after the
migration it will only affect http scheme. 

Does this sounds reasonable to you?

[msramek, 2016/07/07 12:19:24]

Yup, makes sense :)

+    (*origin_pattern).parts_.is_scheme_wildcard = false;
+  }
+
+  (*origin_pattern).parts_.has_domain_wildcard = false;
+
+  if (domain_pattern.parts_.is_port_wildcard) {
+    (*origin_pattern).parts_.port =
+        GetDefaultPort((*origin_pattern).parts_.scheme);
+    (*origin_pattern).parts_.is_port_wildcard = false;
+  }
+  return true;
+}
+
+// static
 void ContentSettingsPattern::SetNonWildcardDomainNonPortScheme(
     const char* scheme) {
   DCHECK(scheme);
   DCHECK(!non_port_non_domain_wildcard_scheme ||
          non_port_non_domain_wildcard_scheme == scheme);
   non_port_non_domain_wildcard_scheme = scheme;
 }
 
 // static
 bool ContentSettingsPattern::IsNonWildcardDomainNonPortScheme(
@@ skipping 285 matching lines @@
   if (!parts.is_path_wildcard && other_parts.is_path_wildcard)
     return ContentSettingsPattern::PREDECESSOR;
 
   int result = parts.path.compare(other_parts.path);
   if (result == 0)
     return ContentSettingsPattern::IDENTITY;
   if (result > 0)
     return ContentSettingsPattern::DISJOINT_ORDER_PRE;
   return ContentSettingsPattern::DISJOINT_ORDER_POST;
 }