Intercept navigations, not DNS resolutions, to predict startup

Transition from using a DNS observer, to using the 
TCP/IP connection interceptor (which is used for
preconneciton and pre-resolution) to acquire the
list of hosts to resolve at startup.

BUG=42694
r=mbelshe
Review URL: http://codereview.chromium.org/3169006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@56163 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 105 insertions, 128 deletions

diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc
index 21d71b3..54aaab2 100644
--- a/chrome/browser/io_thread.cc
+++ b/chrome/browser/io_thread.cc
@@ -133,7 +133,6 @@ IOThread::IOThread()
     : BrowserProcessSubThread(ChromeThread::IO),
       globals_(NULL),
       speculative_interceptor_(NULL),
-      prefetch_observer_(NULL),
       predictor_(NULL) {}
 
 IOThread::~IOThread() {
@@ -217,13 +216,6 @@ void IOThread::CleanUp() {
   delete speculative_interceptor_;
   speculative_interceptor_ = NULL;
 
-  // Not initialized in Init().  May not be initialized.
-  if (prefetch_observer_) {
-    globals_->host_resolver->RemoveObserver(prefetch_observer_);
-    delete prefetch_observer_;
-    prefetch_observer_ = NULL;
-  }
-
   // TODO(eroman): hack for http://crbug.com/15513
   if (globals_->host_resolver->GetAsHostResolverImpl()) {
     globals_->host_resolver.get()->GetAsHostResolverImpl()->Shutdown();
@@ -335,15 +327,7 @@ void IOThread::InitNetworkPredictorOnIOThread(
   DCHECK(!speculative_interceptor_);
   speculative_interceptor_ = new chrome_browser_net::ConnectInterceptor;
 
-  // TODO(jar): We can completely replace prefetch_observer with
-  // speculative_interceptor.
-  // Prefetch_observer is used to monitor initial resolutions.
-  DCHECK(!prefetch_observer_);
-  prefetch_observer_ = chrome_browser_net::CreateResolverObserver();
-  globals_->host_resolver->AddObserver(prefetch_observer_);
-
-  FinalizePredictorInitialization(
-      predictor_, prefetch_observer_, startup_urls, referral_list);
+  FinalizePredictorInitialization(predictor_, startup_urls, referral_list);
 }
 
 void IOThread::ChangedToOnTheRecordOnIOThread() {
diff --git a/chrome/browser/io_thread.h b/chrome/browser/io_thread.h
index 8ce6ecf..f2f1310 100644
--- a/chrome/browser/io_thread.h
+++ b/chrome/browser/io_thread.h
@@ -110,7 +110,6 @@ class IOThread : public BrowserProcessSubThread {
   // these observers would be used accidentally after we have begun to tear
   // down.
   chrome_browser_net::ConnectInterceptor* speculative_interceptor_;
-  net::HostResolver::Observer* prefetch_observer_;
   chrome_browser_net::Predictor* predictor_;
 
   DISALLOW_COPY_AND_ASSIGN(IOThread);
diff --git a/chrome/browser/net/connect_interceptor.cc b/chrome/browser/net/connect_interceptor.cc
index 99abf24..8b35631 100644
--- a/chrome/browser/net/connect_interceptor.cc
+++ b/chrome/browser/net/connect_interceptor.cc
@@ -18,6 +18,11 @@ ConnectInterceptor::~ConnectInterceptor() {
 }
 
 URLRequestJob* ConnectInterceptor::MaybeIntercept(URLRequest* request) {
+  // Learn what URLs are likely to be needed during next startup.
+  // Pass actual URL, rather than WithEmptyPath, as we often won't need to do
+  // the canonicalization.
+  LearnAboutInitialNavigation(request->url());
+
   bool is_subresource = !(request->load_flags() & net::LOAD_MAIN_FRAME);
   if (is_subresource && !request->referrer().empty()) {
     // Learn about our referring URL, for use in the future.
diff --git a/chrome/browser/net/predictor.cc b/chrome/browser/net/predictor.cc
index 83c6160..c698bbc 100644
--- a/chrome/browser/net/predictor.cc
+++ b/chrome/browser/net/predictor.cc
@@ -130,9 +130,10 @@ void Predictor::Resolve(const GURL& url,
 void Predictor::LearnFromNavigation(const GURL& referring_url,
                                     const GURL& target_url) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
+  DCHECK(referring_url == referring_url.GetWithEmptyPath());
+  DCHECK(target_url == target_url.GetWithEmptyPath());
   if (referring_url.has_host() &&
       referring_url != target_url) {
-    DCHECK(referring_url == referring_url.GetWithEmptyPath());
     referrers_[referring_url].SuggestHost(target_url);
   }
 }
@@ -150,11 +151,12 @@ void Predictor::PredictFrameSubresources(const GURL& url) {
   Referrers::iterator it = referrers_.find(url);
   if (referrers_.end() == it)
     return;
+  // Add one pass through the message loop to allow current navigation to
+  // proceed.
   ChromeThread::PostTask(
       ChromeThread::IO,
       FROM_HERE,
-      NewRunnableMethod(this,
-                        &Predictor::PrepareFrameSubresources, url));
+      NewRunnableMethod(this, &Predictor::PrepareFrameSubresources, url));
 }
 
 void Predictor::PrepareFrameSubresources(const GURL& url) {
diff --git a/chrome/browser/net/predictor_api.cc b/chrome/browser/net/predictor_api.cc
index 28e89f2..c55fbf4 100644
--- a/chrome/browser/net/predictor_api.cc
+++ b/chrome/browser/net/predictor_api.cc
@@ -54,6 +54,37 @@ const int PredictorInit::kMaxPrefetchQueueingDelayMs = 500;
 // we change the format so that we discard old data.
 static const int kPredictorStartupFormatVersion = 1;
 
+// There will only be one instance ever created of the following Observer class.
+// The InitialObserver lives on the IO thread, and monitors navigations made by
+// the network stack.  This is only used to identify startup time resolutions
+// (for re-resolution during our next process startup).
+// TODO(jar): Consider preconnecting at startup, which may be faster than
+// waiting for render process to start and request a connection.
+class InitialObserver {
+ public:
+  // Recording of when we observed each navigation.
+  typedef std::map<GURL, base::TimeTicks> FirstNavigations;
+
+  // Potentially add a new URL to our startup list.
+  void Append(const GURL& url);
+
+  // Get an HTML version of our current planned first_navigations_.
+  void GetFirstResolutionsHtml(std::string* output);
+
+  // Persist the current first_navigations_ for storage in a list.
+  void GetInitialDnsResolutionList(ListValue* startup_list);
+
+ private:
+  // List of the first N URL resolutions observed in this run.
+  FirstNavigations first_navigations_;
+
+  // The number of URLs we'll save for pre-resolving at next startup.
+  static const size_t kStartupResolutionCount = 10;
+};
+
+// TODO(willchan): Look at killing this global.
+static InitialObserver* g_initial_observer = NULL;
+
 //------------------------------------------------------------------------------
 // Static helper functions
 //------------------------------------------------------------------------------
@@ -89,8 +120,9 @@ static GURL CanonicalizeUrl(const GURL& url) {
 // DNS Prefetching feature.
 //------------------------------------------------------------------------------
 
-// Status of prefetch feature, controlling whether any prefetching is done.
-static bool dns_prefetch_enabled = true;
+// Status of speculative DNS resolution and speculative TCP/IP connection
+// feature.
+static bool predictor_enabled = true;
 
 // Cached inverted copy of the off_the_record pref.
 static bool on_the_record_switch = true;
@@ -98,7 +130,7 @@ static bool on_the_record_switch = true;
 // Enable/disable Dns prefetch activity (either via command line, or via pref).
 void EnablePredictor(bool enable) {
   // NOTE: this is invoked on the UI thread.
-  dns_prefetch_enabled = enable;
+  predictor_enabled = enable;
 }
 
 void OnTheRecord(bool enable) {
@@ -129,7 +161,8 @@ static Predictor* predictor = NULL;
 // includes both Page-Scan, and Link-Hover prefetching.
 // TODO(jar): Separate out link-hover prefetching, and page-scan results.
 void DnsPrefetchList(const NameList& hostnames) {
-  // TODO(jar): Push GURL transport further back into renderer.
+  // TODO(jar): Push GURL transport further back into renderer, but this will
+  // require a Webkit change in the observer :-/.
   UrlList urls;
   for (NameList::const_iterator it = hostnames.begin();
        it < hostnames.end();
@@ -146,7 +179,7 @@ static void DnsPrefetchMotivatedList(
     UrlInfo::ResolutionMotivation motivation) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI) ||
          ChromeThread::CurrentlyOn(ChromeThread::IO));
-  if (!dns_prefetch_enabled || NULL == predictor)
+  if (!predictor_enabled || NULL == predictor)
     return;
 
   if (ChromeThread::CurrentlyOn(ChromeThread::IO)) {
@@ -163,7 +196,7 @@ static void DnsPrefetchMotivatedList(
 // This API is used by the autocomplete popup box (where URLs are typed).
 void AnticipateUrl(const GURL& url, bool preconnectable) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
-  if (!dns_prefetch_enabled || NULL == predictor)
+  if (!predictor_enabled || NULL == predictor)
     return;
   if (!url.is_valid())
     return;
@@ -209,7 +242,7 @@ void AnticipateUrl(const GURL& url, bool preconnectable) {
 static void ResolveOnUIThread(const GURL& url,
                               UrlInfo::ResolutionMotivation motivation) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
-  if (!dns_prefetch_enabled || NULL == predictor || !url.has_host())
+  if (!predictor_enabled || NULL == predictor || !url.has_host())
     return;
 
   ChromeThread::PostTask(
@@ -228,14 +261,21 @@ static void ResolveOnUIThread(const GURL& url,
 
 void PredictFrameSubresources(const GURL& url) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
-  if (!dns_prefetch_enabled || NULL == predictor)
+  if (!predictor_enabled || NULL == predictor)
     return;
   predictor->PredictFrameSubresources(url);
 }
 
+void LearnAboutInitialNavigation(const GURL& url) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
+  if (!predictor_enabled || NULL == g_initial_observer )
+    return;
+  g_initial_observer->Append(url);
+}
+
 void LearnFromNavigation(const GURL& referring_url, const GURL& target_url) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
-  if (!dns_prefetch_enabled || NULL == predictor)
+  if (!predictor_enabled || NULL == predictor)
     return;
   predictor->LearnFromNavigation(referring_url, target_url);
 }
@@ -244,103 +284,49 @@ void LearnFromNavigation(const GURL& referring_url, const GURL& target_url) {
 // resolutions.  We use the following type for that map.
 typedef std::map<int, UrlInfo> ObservedResolutionMap;
 
-// There will only be one instance ever created of the following Observer
-// class. The PrefetchObserver lives on the IO thread, and intercepts DNS
-// resolutions made by the network stack.  This is only used to identify startup
-// time resolutions (for re-resolution during our next process startup).
-class PrefetchObserver : public net::HostResolver::Observer {
- public:
-  typedef std::set<GURL> FirstResolutions;
-
-  // net::HostResolver::Observer implementation:
-  virtual void OnStartResolution(
-      int request_id,
-      const net::HostResolver::RequestInfo& request_info) {}
-
-  virtual void OnFinishResolutionWithStatus(int id, bool was_resolved,
-      const net::HostResolver::RequestInfo& info);
-
-  virtual void OnCancelResolution(int id,
-      const net::HostResolver::RequestInfo& info) {}
-
-    void DnsGetFirstResolutionsHtml(std::string* output);
-  void GetInitialDnsResolutionList(ListValue* startup_list);
-
- private:
-  void StartupListAppend(const std::string& host, int port);
-
-  // List of the first N URL resolutions observed in this run.
-  FirstResolutions first_resolutions_;
-  // The number of URLs we'll save for pre-resolving at next startup.
-  static const size_t kStartupResolutionCount = 10;
-};
-
-// TODO(willchan): Look at killing this global.
-static PrefetchObserver* g_prefetch_observer = NULL;
-
 //------------------------------------------------------------------------------
-// Member definitions for above Observer class.
-
-void PrefetchObserver::OnFinishResolutionWithStatus(
-    int request_id,
-    bool was_resolved,
-    const net::HostResolver::RequestInfo& request_info) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
-  if (!was_resolved)
-    return;  // Don't remember failures.
-  if (request_info.is_speculative())
-    return;  // One of our own requests.
-  if (!request_info.hostname().length())
-    return;  // PAC scripts may create queries without a hostname.
-
-  StartupListAppend(request_info.hostname(), request_info.port());
-}
+// Member definitions for InitialObserver class.
 
-void PrefetchObserver::StartupListAppend(const std::string& host, int port) {
+void InitialObserver::Append(const GURL& url) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
 
   if (!on_the_record_switch || NULL == predictor)
     return;
-  if (kStartupResolutionCount <= first_resolutions_.size())
-    return;  // Someone just added the last item.
-
-  // TODO(jar): Switch to using speculative_interceptor_ instead of
-  // PrefetchObserver.
-  bool is_ssl(443 == port);
-  std::string url_spec = is_ssl ? "https://" : "http://";
-  url_spec += host;
-  url_spec += ":";
-  url_spec += base::IntToString(port);
-
-  GURL url(url_spec);
-  first_resolutions_.insert(url);
+  if (kStartupResolutionCount <= first_navigations_.size())
+    return;
+
+  if (url.SchemeIs("http") || url.SchemeIs("https")) {
+    const GURL url_without_path(url.GetWithEmptyPath());
+    if (first_navigations_.find(url_without_path) == first_navigations_.end())
+      first_navigations_[url_without_path] = base::TimeTicks::Now();
+  }
 }
 
-void PrefetchObserver::GetInitialDnsResolutionList(ListValue* startup_list) {
+void InitialObserver::GetInitialDnsResolutionList(ListValue* startup_list) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
   DCHECK(startup_list);
   startup_list->Clear();
   DCHECK_EQ(0u, startup_list->GetSize());
   startup_list->Append(new FundamentalValue(kPredictorStartupFormatVersion));
-  for (FirstResolutions::iterator it = first_resolutions_.begin();
-       it != first_resolutions_.end();
+  for (FirstNavigations::iterator it = first_navigations_.begin();
+       it != first_navigations_.end();
        ++it) {
-    DCHECK(*it == CanonicalizeUrl(*it));
-    startup_list->Append(new StringValue(it->spec()));
+    DCHECK(it->first == CanonicalizeUrl(it->first));
+    startup_list->Append(new StringValue(it->first.spec()));
   }
 }
 
-void PrefetchObserver::DnsGetFirstResolutionsHtml(std::string* output) {
+void InitialObserver::GetFirstResolutionsHtml(std::string* output) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
 
   UrlInfo::UrlInfoTable resolution_list;
   {
-    for (FirstResolutions::iterator it(first_resolutions_.begin());
-         it != first_resolutions_.end();
+    for (FirstNavigations::iterator it(first_navigations_.begin());
+         it != first_navigations_.end();
          it++) {
       UrlInfo info;
-      info.SetUrl(*it);
-      // TODO(jar): Need to set time of info.GetDuration();
+      info.SetUrl(it->first);
+      info.set_time(it->second);
       resolution_list.push_back(info);
     }
   }
@@ -413,16 +399,19 @@ void PredictorGetHtmlInfo(std::string* output) {
                  // We'd like the following no-cache... but it doesn't work.
                  // "<META HTTP-EQUIV=\"Pragma\" CONTENT=\"no-cache\">"
                  "</head><body>");
-  if (!dns_prefetch_enabled  || NULL == predictor) {
+  if (!predictor_enabled  || NULL == predictor) {
     output->append("Dns Prefetching is disabled.");
   } else {
     if (!on_the_record_switch) {
       output->append("Incognito mode is active in a window.");
     } else {
-      predictor->GetHtmlInfo(output);
-      if (g_prefetch_observer)
-        g_prefetch_observer->DnsGetFirstResolutionsHtml(output);
+      // List items fetched at startup.
+      if (g_initial_observer)
+        g_initial_observer->GetFirstResolutionsHtml(output);
+      // Show list of subresource predictions and stats.
       predictor->GetHtmlReferrerLists(output);
+      // Show list of prediction results.
+      predictor->GetHtmlInfo(output);
     }
   }
   output->append("</body></html>");
@@ -455,13 +444,11 @@ static void InitNetworkPredictor(TimeDelta max_dns_queue_delay,
 
 void FinalizePredictorInitialization(
     Predictor* global_predictor,
-    net::HostResolver::Observer* global_prefetch_observer,
     const UrlList& startup_urls,
     ListValue* referral_list) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
   predictor = global_predictor;
-  g_prefetch_observer =
-      static_cast<PrefetchObserver*>(global_prefetch_observer);
+  g_initial_observer = new InitialObserver();
 
   DLOG(INFO) << "DNS Prefetch service started";
 
@@ -472,14 +459,10 @@ void FinalizePredictorInitialization(
 }
 
 void FreePredictorResources() {
-  predictor = NULL;
-  g_prefetch_observer = NULL;
-}
-
-//------------------------------------------------------------------------------
-
-net::HostResolver::Observer* CreateResolverObserver() {
-  return new PrefetchObserver();
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
+  predictor = NULL;  // Owned and released by io_thread.cc.
+  delete g_initial_observer;
+  g_initial_observer = NULL;
 }
 
 //------------------------------------------------------------------------------
@@ -497,8 +480,8 @@ static void SaveDnsPrefetchStateForNextStartupAndTrimOnIOThread(
     return;
   }
 
-  if (g_prefetch_observer)
-    g_prefetch_observer->GetInitialDnsResolutionList(startup_list);
+  if (g_initial_observer)
+    g_initial_observer->GetInitialDnsResolutionList(startup_list);
 
   // TODO(jar): Trimming should be done more regularly, such as every 48 hours
   // of physical time, or perhaps after 48 hours of running (excluding time
@@ -513,7 +496,7 @@ static void SaveDnsPrefetchStateForNextStartupAndTrimOnIOThread(
 void SavePredictorStateForNextStartupAndTrim(PrefService* prefs) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
 
-  if (!dns_prefetch_enabled || predictor == NULL)
+  if (!predictor_enabled || predictor == NULL)
     return;
 
   base::WaitableEvent completion(true, false);
diff --git a/chrome/browser/net/predictor_api.h b/chrome/browser/net/predictor_api.h
index 90603a3..3c6d97b 100644
--- a/chrome/browser/net/predictor_api.h
+++ b/chrome/browser/net/predictor_api.h
@@ -20,7 +20,6 @@
 #include "base/scoped_ptr.h"
 #include "chrome/browser/autocomplete/autocomplete.h"
 #include "chrome/browser/net/predictor.h"
-#include "net/base/host_resolver.h"
 
 class PrefService;
 
@@ -29,7 +28,6 @@ namespace chrome_browser_net {
 // Deletes |referral_list| when done.
 void FinalizePredictorInitialization(
     Predictor* global_predictor,
-    net::HostResolver::Observer* global_prefetch_observer,
     const std::vector<GURL>& urls_to_prefetch,
     ListValue* referral_list);
 
@@ -37,9 +35,6 @@ void FinalizePredictorInitialization(
 // you must not call any function from this file.
 void FreePredictorResources();
 
-// Creates the HostResolver observer for the prefetching system.
-net::HostResolver::Observer* CreateResolverObserver();
-
 //------------------------------------------------------------------------------
 // Global APIs relating to predictions in browser.
 void EnablePredictor(bool enable);
@@ -64,6 +59,10 @@ void PredictorGetHtmlInfo(std::string* output);
 // sometimes SSL).  This function initiates those connections
 void PredictFrameSubresources(const GURL& url);
 
+// During startup, we learn what the first N urls visited are, and then resolve
+// the associated hosts ASAP during our next startup.
+void LearnAboutInitialNavigation(const GURL& url);
+
 // Call when we should learn from a navigation about a relationship to a
 // subresource target, and its containing frame, which was loaded as a referring
 // URL.
diff --git a/chrome/browser/net/url_info.cc b/chrome/browser/net/url_info.cc
index a92f5a2..cd065de 100644
--- a/chrome/browser/net/url_info.cc
+++ b/chrome/browser/net/url_info.cc
@@ -293,7 +293,7 @@ void UrlInfo::GetHtmlTable(const UrlInfoTable host_infos,
   }
   output->append("</table>");
 
-#ifdef DEBUG
+#ifndef NDEBUG
   StringAppendF(output,
                 "Prefetch Queue Durations: min=%d, avg=%d, max=%d<br><br>",
                 queue.minimum(), queue.average(), queue.maximum());
diff --git a/chrome/browser/net/url_info.h b/chrome/browser/net/url_info.h
index c2af6e9..eea0e6e 100644
--- a/chrome/browser/net/url_info.h
+++ b/chrome/browser/net/url_info.h
@@ -131,6 +131,11 @@ class UrlInfo {
                            const bool brief,
                            std::string* output);
 
+  // For testing, and use in printing tables of info, we sometimes need to
+  // adjust the time manually.  Usually, this value is maintained by state
+  // transition, and this call is not made.
+  void set_time(const base::TimeTicks& time) { time_ = time; }
+
  private:
   base::TimeDelta GetDuration() {
     base::TimeTicks old_time = time_;