Add metrics for calculating precision/recall of link navigation pre-connect triggers.

chrome/browser/net/predictor.h

 // Copyright (c) 2012 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.
 
 // A Predictor object is instantiated once in the browser process, and manages
 // both preresolution of hostnames, as well as TCP/IP preconnection to expected
 // subresources.
 // Most hostname lists are provided by the renderer processes, and include URLs
 // that *might* be used in the near future by the browsing user.  One goal of
 // this class is to cause the underlying DNS structure to lookup a hostname
@@ skipping 220 matching lines @@
   // May be called from either the IO or UI thread and will PostTask
   // to the IO thread if necessary.
   void PreconnectUrl(const GURL& url, const GURL& first_party_for_cookies,
                      UrlInfo::ResolutionMotivation motivation, int count);
 
   void PreconnectUrlOnIOThread(const GURL& url,
                                const GURL& first_party_for_cookies,
                                UrlInfo::ResolutionMotivation motivation,
                                int count);
 
-  void RecordPreconnectNavigationStats(const GURL& url);
+  // May be called from either the IO or UI thread and will PostTask

[jar (doing other things), 2013/08/01 22:56:47]

nit: (personal?) I think it is a bad pattern to protect methods with a callback
to the right thread if you happen to be on the wrong thread when calling.  

I guess you could argue that you compact some amount of code (the post-task
call), but I think that folks should know which thread they are on, and not be
blind to the implications of such calls.

I'll ask others... but my personal preference is to avoid this pattern.  

If you really have a bunch of folks calling Foo() from both the IO thread and
from the UI thread (and want to save code?), you *could* have a
FooFromUIThread() vs a FooFromIOThread().  That would at least hint to the
reader that you might be on the wrong thread.. and this call might need to be
posted etc.

[kouhei (in TOK), 2013/08/02 06:18:03]

I changed so that callers PostTask to IOThread.

+  // to the IO thread if necessary.
+  void RecordPreconnectTrigger(const GURL& url);
+
+  void RecordPreconnectNavigationStat(const std::vector<GURL>& url_chain,
+                                      bool is_subresource);
+
+  // May be called from either the IO or UI thread and will PostTask

[jar (doing other things), 2013/08/01 22:56:47]

nit: same comment on pattern as above.

[kouhei (in TOK), 2013/08/02 06:18:03]

Done.

+  // to the IO thread if necessary.
+  void RecordLinkNavigation(const GURL& url);
 
   // ------------- End IO thread methods.
 
   // The following methods may be called on either the IO or UI threads.
 
   // Instigate pre-connection to any URLs, or pre-resolution of related host,
   // that we predict will be needed after this navigation (typically
   // more-embedded resources on a page).  This method will actually post a task
   // to do the actual work, so as not to jump ahead of the frame navigation that
   // instigated this activity.
@@ skipping 194 matching lines @@
   void LoadUrlsForTrimming();
 
   // Posts a task to do additional incremental trimming of referrers_.
   void PostIncrementalTrimTask();
 
   // Calls Trim() on some or all of urls_being_trimmed_.
   // If it does not process all the URLs in that vector, it posts a task to
   // continue with them shortly (i.e., it yeilds and continues).
   void IncrementalTrimReferrers(bool trim_all_now);
 
+  void RecordPreconnectTriggerOnIOThread(const GURL& url);
+
+  void RecordLinkNavigationOnIOThread(const GURL& url);

[jar (doing other things), 2013/08/01 22:56:47]

So you have this method... it is just IMO nicer to expose it, let callers use
their knowledge to "call the right flavor," and then DCHECK that it was done
correctly.  This also means we don't waste time doing the check in production
code needlessly.

[kouhei (in TOK), 2013/08/02 06:18:03]

Done.

+
   // ------------- End IO thread methods.
 
   scoped_ptr<InitialObserver> initial_observer_;
 
   // Reference to URLRequestContextGetter from the Profile which owns the
   // predictor. Used by Preconnect.
   scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
 
   // Status of speculative DNS resolution and speculative TCP/IP connection
   // feature.
@@ skipping 36 matching lines @@
   // The time when the last preresolve was done for last_omnibox_host_.
   base::TimeTicks last_omnibox_preresolve_;
 
   // The number of consecutive requests to AnticipateOmniboxUrl() that suggested
   // preconnecting (because it was to a search service).
   int consecutive_omnibox_preconnect_count_;
 
   // The time when the last preconnection was requested to a search service.
   base::TimeTicks last_omnibox_preconnect_;
 
-  TimedCache recent_preconnects_;
+  class PreconnectUsage;
+  scoped_ptr<PreconnectUsage> preconnect_usage_;
 
   // For each URL that we might navigate to (that we've "learned about")
   // we have a Referrer list. Each Referrer list has all hostnames we might
   // need to pre-resolve or pre-connect to when there is a navigation to the
   // orginial hostname.
   Referrers referrers_;
 
   // List of URLs in referrers_ currently being trimmed (scaled down to
   // eventually be aged out of use).
   std::vector<GURL> urls_being_trimmed_;
@@ skipping 16 matching lines @@
       PrefService* user_prefs,
       PrefService* local_state,
       IOThread* io_thread,
       net::URLRequestContextGetter* getter) OVERRIDE;
   virtual void ShutdownOnUIThread(PrefService* user_prefs) OVERRIDE;
 };
 
 }  // namespace chrome_browser_net
 
 #endif  // CHROME_BROWSER_NET_PREDICTOR_H_