Make HttpNetworkSession::host_mapping_rules no longer a pointer.

net/base/host_mapping_rules.cc

Index: net/base/host_mapping_rules.cc
diff --git a/net/base/host_mapping_rules.cc b/net/base/host_mapping_rules.cc
index 3c24b245942319b069d18f7b5f791fc04e8baa09..b8fef178e7a7f8f9c2751b2fa8b63917fed823eb 100644
--- a/net/base/host_mapping_rules.cc
+++ b/net/base/host_mapping_rules.cc
@@ -28,22 +28,17 @@ struct HostMappingRules::ExclusionRule {
 
 HostMappingRules::HostMappingRules() {}
 
+HostMappingRules::HostMappingRules(const HostMappingRules& host_mapping_rules) =
+    default;
+
 HostMappingRules::~HostMappingRules() {}
 
-bool HostMappingRules::RewriteHost(HostPortPair* host_port) const {
-  // Check if the hostname was excluded.
-  for (ExclusionRuleList::const_iterator it = exclusion_rules_.begin();
-       it != exclusion_rules_.end(); ++it) {
-    const ExclusionRule& rule = *it;
-    if (base::MatchPattern(host_port->host(), rule.hostname_pattern))
-      return false;
-  }
+HostMappingRules& HostMappingRules::operator=(
+    const HostMappingRules& host_mapping_rules) = default;
 

[Randy Smith (Not in Mondays), 2017/05/25 17:50:48]

The movement of the exclusion check looks like a behavior change to me, though I
presume the mapping rules are just used for testing (?) so we're free to make
that change if we want.  Is it not a behavior change/what was the motivation for
it if it was?

[mmenke, 2017/05/25 18:07:10]

I don't think this is a behavior change.  This was just done for very modest
perf reasons - I was concerned we'd go from never running this code, since the
rules was nullptr, to always using it.  Which shouldn't matter, but I'm paranoid
(Turns out it's generally not nullptr, anyways, except in CroNet, or other
implementations).

Code was basically:

if (<exclude>)
  return false;

if (<include>) {
  DoStuff();
  return true;
}

return false;

It's now:

if (<include>) {
  if (<exclude>)
    return false;
  DoStuff();
  return true;
}

return false;

So we do less when there are no lists, and when there are lists, we only check
the exclude list if there's an include list hit.  Exclude list I assume would
generally be more specific, so in both cases, it's generally a modest per
improvement.  Or am I missing something?  I can certainly go back to the old
behavior without much concern, just wanted to minimize any impact on perf
(Which, when there are no patterns, would no doubt be trivial anyways).

[Randy Smith (Not in Mondays), 2017/05/25 18:17:06]

I'm sorry, I was reading too fast, and read this as an exchange of position, not
a moving of one check inside another.  Comment retracted.

+bool HostMappingRules::RewriteHost(HostPortPair* host_port) const {
   // Check if the hostname was remapped.
-  for (MapRuleList::const_iterator it = map_rules_.begin();
-       it != map_rules_.end(); ++it) {
-    const MapRule& rule = *it;
-
+  for (const auto& rule : map_rules_) {
     // The rule's hostname_pattern will be something like:
     //     www.foo.com
     //     *.foo.com
@@ -57,6 +52,12 @@ bool HostMappingRules::RewriteHost(HostPortPair* host_port) const {
         continue;  // This rule doesn't apply.
     }
 
+    // Check if the hostname was excluded.
+    for (const auto& rule : exclusion_rules_) {
+      if (base::MatchPattern(host_port->host(), rule.hostname_pattern))
+        return false;
+    }
+
     host_port->set_host(rule.replacement_hostname);
     if (rule.replacement_port != -1)
       host_port->set_port(static_cast<uint16_t>(rule.replacement_port));