Make HttpNetworkSession::host_mapping_rules no longer a pointer.

net/base/host_mapping_rules.cc

 // Copyright (c) 2010 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 "net/base/host_mapping_rules.h"
 
 #include "base/logging.h"
 #include "base/strings/pattern.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_tokenizer.h"
@@ skipping 10 matching lines @@
   std::string replacement_hostname;
   int replacement_port;
 };
 
 struct HostMappingRules::ExclusionRule {
   std::string hostname_pattern;
 };
 
 HostMappingRules::HostMappingRules() {}
 
+HostMappingRules::HostMappingRules(const HostMappingRules& host_mapping_rules) =
+    default;
+
 HostMappingRules::~HostMappingRules() {}
 
+HostMappingRules& HostMappingRules::operator=(
+    const HostMappingRules& host_mapping_rules) = default;
+
 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;
-  }
-

[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.

   // 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
     //     www.foo.com:1234
     //     *.foo.com:1234
     // First, we'll check for a match just on hostname.
     // If that fails, we'll check for a match with both hostname and port.
     if (!base::MatchPattern(host_port->host(), rule.hostname_pattern)) {
       std::string host_port_string = host_port->ToString();
       if (!base::MatchPattern(host_port_string, rule.hostname_pattern))
         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));
     return true;
   }
 
   return false;
 }
 
 bool HostMappingRules::AddRuleFromString(const std::string& rule_string) {
@@ skipping 31 matching lines @@
   map_rules_.clear();
 
   base::StringTokenizer rules(rules_string, ",");
   while (rules.GetNext()) {
     bool ok = AddRuleFromString(rules.token());
     LOG_IF(ERROR, !ok) << "Failed parsing rule: " << rules.token();
   }
 }
 
 }  // namespace net