Add a browsertest suite for net predictor

chrome/browser/net/predictor.h

Index: chrome/browser/net/predictor.h
diff --git a/chrome/browser/net/predictor.h b/chrome/browser/net/predictor.h
index 2e6f811811e3ca1c36e414b44146f375d47ccd0b..5238f26b73e6498ce9d5d16c924ace356a1fc709 100644
--- a/chrome/browser/net/predictor.h
+++ b/chrome/browser/net/predictor.h
@@ -38,6 +38,7 @@
 #include "chrome/browser/net/url_info.h"
 #include "components/network_hints/common/network_hints_common.h"
 #include "net/base/host_port_pair.h"
+#include "url/gurl.h"
 
 class IOThread;
 class PrefService;
@@ -75,7 +76,9 @@ class PredictorObserver {
   virtual void OnPreconnectUrl(const GURL& original_url,
                                const GURL& first_party_for_cookies,
                                UrlInfo::ResolutionMotivation motivation,
-                               int count) = 0;
+                               int count) {}
+  virtual void OnLearnFromNavigation(const GURL& referring_url,
+                                     const GURL& target_url) {}
 };
 
 // Predictor is constructed during Profile construction (on the UI thread),
@@ -300,6 +303,7 @@ class Predictor {
   }
 
   bool preconnect_enabled() const {

[mmenke, 2016/05/12 16:30:23]

Naming scheme should be changed, and method de-inlined since we're grabbing a
lock.

[mmenke, 2016/05/12 16:30:23]

optional:  Could use an atomic instead of a lock here.  Doesn't really matter, I
suppose.  (Naming scheme should still change, even if we switch)

[Charlie Harrison, 2016/05/12 20:17:03]

Done.

[Charlie Harrison, 2016/05/12 20:17:03]

Kept the lock for simplicity.

+    base::AutoLock lock(lock_);
     return preconnect_enabled_;
   }
 
@@ -307,6 +311,16 @@ class Predictor {
     return predictor_enabled_;
   }
 
+  // Used only for testing. Overrides command line flag to disable preconnect,
+  // which is added in the browser test fixture.
+  void set_preconnect_enabled_for_test(bool preconnect_enabled) {

[mmenke, 2016/05/12 16:30:22]

Again, naming scheme should be changed, and should be de-inlined.

[Charlie Harrison, 2016/05/12 20:17:03]

Done.

+    base::AutoLock lock(lock_);
+    preconnect_enabled_ = preconnect_enabled;
+  }
+
+  net::URLRequestContextGetter* url_request_context_getter_for_test() {
+    return url_request_context_getter_.get();
+  }
 
  private:
   FRIEND_TEST_ALL_PREFIXES(PredictorTest, BenefitLookupTest);
@@ -564,7 +578,7 @@ class Predictor {
   // Are we currently using preconnection, rather than just DNS resolution, for
   // subresources and omni-box search URLs.
   // This is false if and only if disabled by a command line switch.
-  const bool preconnect_enabled_;
+  bool preconnect_enabled_;

[mmenke, 2016/05/12 16:30:23]

Should mention it's protected by the lock.  Or make it atomic, per above
suggestions.

[Charlie Harrison, 2016/05/12 20:17:03]

Done.

 
   // Most recent suggestion from Omnibox provided via AnticipateOmniboxUrl().
   std::string last_omnibox_host_;
@@ -597,6 +611,8 @@ class Predictor {
 
   std::unique_ptr<base::WeakPtrFactory<Predictor>> weak_factory_;
 
+  mutable base::Lock lock_;

[mmenke, 2016/05/12 16:30:23]

Should mention what this protects, and could even call it
preconnect_enabled_lock_.

[Charlie Harrison, 2016/05/12 20:17:03]

Done.

+
   DISALLOW_COPY_AND_ASSIGN(Predictor);
 };