Sync the browser test host resolver with the network process before a test runs.

net/dns/mock_host_resolver.h

Index: net/dns/mock_host_resolver.h
diff --git a/net/dns/mock_host_resolver.h b/net/dns/mock_host_resolver.h
index cca300acbc69c90e93bfa0772f27646533f0670f..37eab83301cf56e9ab26f90ff09bffea8ce9affa 100644
--- a/net/dns/mock_host_resolver.h
+++ b/net/dns/mock_host_resolver.h
@@ -212,6 +212,9 @@ class RuleBasedHostResolverProc : public HostResolverProc {
   // Deletes all the rules that have been added.
   void ClearRules();
 
+  // Asserts if any changes are made to the rules after this call.

[mmenke, 2017/04/25 16:37:53]

Could you clarify that this makes calls that add/remove/modify rules assert?  As
opposed to this method itself asserting.

[jam, 2017/04/25 17:04:40]

"after this call" isn't clear enough?

[mmenke, 2017/04/25 17:44:44]

Oops, forgot to respond to this.  My feeling is that for clarity, methods should
have comments along the lines what they do when called, not what they will do in
the future (Or worse, what they cause other, unspecified methods to do in the
future).

So I think it's best to prefer something like "Causes/tells/makes class to
assert if <....>", for instance, to more clearly say what the method does when
called.  This also more closely matches format of other method descriptions.

[mmenke, 2017/04/25 18:10:08]

A better explanation:  This method does not "Assert if any changes are made to
the rules after this call".  It makes other methods assert if changes are made
after it is called.

[jam, 2017/04/25 18:19:37]

Done.

+  void Lock();

[mmenke, 2017/04/25 16:37:55]

Add a TODO to remove this, once it's no longer used, and to make Rule private
again when that happens?  I assume we won't need either of those, once we're
using a Mojo interface for this.

[mmenke, 2017/04/25 16:37:55]

Can we not call this lock?  Commit() or Initialize(), maybe?  (I don't like lock
because this doesn't have anything to do with a base::Lock objects)

[jam, 2017/04/25 17:04:40]

Done.

[jam, 2017/04/25 17:04:40]

Done.

+
   // HostResolverProc methods:
   int Resolve(const std::string& host,
               AddressFamily address_family,
@@ -219,10 +222,36 @@ class RuleBasedHostResolverProc : public HostResolverProc {
               AddressList* addrlist,
               int* os_error) override;
 
- private:
-  struct Rule;
+  struct Rule {
+    enum ResolverType {
+      kResolverTypeFail,
+      kResolverTypeSystem,
+      kResolverTypeIPLiteral,
+    };
+
+    ResolverType resolver_type;
+    std::string host_pattern;
+    AddressFamily address_family;
+    HostResolverFlags host_resolver_flags;
+    std::string replacement;
+    std::string canonical_name;

[mmenke, 2017/04/25 16:37:54]

include <string> (Should have been included already)

[jam, 2017/04/25 17:04:41]

Done.

+    int latency_ms;  // In milliseconds.
+
+    Rule(ResolverType resolver_type,
+         const std::string& host_pattern,
+         AddressFamily address_family,
+         HostResolverFlags host_resolver_flags,
+         const std::string& replacement,
+         const std::string& canonical_name,
+         int latency_ms);
+    Rule(const Rule& other);

[mmenke, 2017/04/25 16:37:56]

Per style guide, constructor/destructor should go before member variables.

[jam, 2017/04/25 17:04:40]

Done.

+  };
+
   typedef std::list<Rule> RuleList;
 
+  RuleList GetRules();
+
+ private:
   ~RuleBasedHostResolverProc() override;
 
   void AddRuleInternal(const Rule& rule);
@@ -231,6 +260,9 @@ class RuleBasedHostResolverProc : public HostResolverProc {
 
   // Must be obtained before writing to or reading from |rules_|.
   base::Lock rule_lock_;
+
+  // Whether changes are allowed.
+  bool locked_;

[mmenke, 2017/04/25 16:37:55]

Hrm...Should this be protected by the lock as well?  Currently, it's actually
safe to add a rule on any thread.  This CL changes that.  That's probably not an
expected use-case of this class, but I'm not sure whether or not any test is
actually doing it.

[jam, 2017/04/25 17:04:40]

this will be modified before any threading happens by definition (since if there
are races with locking and new rules being added, then the network service would
miss some rules)

[mmenke, 2017/04/25 17:29:46]

SGTM (Particularly with the above TODO)

 };
 
 // Create rules that map all requests to localhost.