Server push cancellation: add NetLogs to track cache lookup transaction

net/http/http_cache_lookup_manager.h

Index: net/http/http_cache_lookup_manager.h
diff --git a/net/http/http_cache_lookup_manager.h b/net/http/http_cache_lookup_manager.h
index 45cfa009ac65d476a5c4b80c2bd1943e9135212a..811541f1f9d7856f6f4f12f87d75e4c67cc5bd93 100644
--- a/net/http/http_cache_lookup_manager.h
+++ b/net/http/http_cache_lookup_manager.h
@@ -18,11 +18,11 @@ namespace net {
 class NET_EXPORT_PRIVATE HttpCacheLookupManager : public ServerPushDelegate {
  public:
   // |http_cache| MUST outlive the HttpCacheLookupManager.
-  HttpCacheLookupManager(HttpCache* http_cache,
-                         const NetLogWithSource& net_log);
+  HttpCacheLookupManager(HttpCache* http_cache, NetLog* net_log);

[Ryan Hamilton, 2017/02/06 23:34:55]

What creates an instance of this class? I don't see any callsites changed other
than in tests.

[Zhongyi Shi, 2017/02/07 00:00:24]

It should be in the HttpCache's constructor. I have a CL working on adding one
more boolean to the HttpCache's constructor due to the cache split, which will
also include finch config. Eventually we will have

HttpCache::HttpCache(...., bool is_main_cache) {
  // ...
  if (session) {
     net_log_ = session->net_log();
     if (is_main_cache) {
       session-> session->SetServerPushDelegate(
           base::MakeUnique<HttpCacheLookupManager>(this, net_log_));
     }
     // ... set up quic server info factory ...
  ｝
} 

[Ryan Hamilton, 2017/02/07 00:10:49]

Oh, I thought this was already wired up because of the manual tests you've been
doing. But I guess this logic was part of that testing?

[Zhongyi Shi, 2017/02/07 06:01:09]

As discussed offline, this CL will have minimal effort to add NetLogs to make it
work. The other CL will then add finch plumbing and turn on the switch. I'll
upload a follow up CL to improve more granular NetLogs.

   ~HttpCacheLookupManager() override;
 
-  void OnPush(std::unique_ptr<ServerPushHelper> push_helper) override;
+  void OnPush(std::unique_ptr<ServerPushHelper> push_helper,
+              const NetLogWithSource& source) override;

[Ryan Hamilton, 2017/02/06 23:34:55]

nit: parameters of type NetLogWithSource are typically called net_log.

[Zhongyi Shi, 2017/02/07 06:01:08]

Done.

 
   // Invoked when the HttpCache::Transaction for |url| finishes to cancel the
   // server push if the response to the server push is found cached.
@@ -34,14 +34,15 @@ class NET_EXPORT_PRIVATE HttpCacheLookupManager : public ServerPushDelegate {
   // push.
   class LookupTransaction {
    public:
-    LookupTransaction(std::unique_ptr<ServerPushHelper> push_helper);
+    LookupTransaction(std::unique_ptr<ServerPushHelper> push_helper,
+                      NetLog* net_log);
     ~LookupTransaction();
 
     // Issues an HttpCache::Transaction to lookup whether the response is cached
     // without header validation.
     int StartLookup(HttpCache* cache,
                     const CompletionCallback& callback,
-                    const NetLogWithSource& net_log);
+                    const NetLogWithSource& source);
 
     void CancelPush();
 
@@ -49,10 +50,11 @@ class NET_EXPORT_PRIVATE HttpCacheLookupManager : public ServerPushDelegate {
     std::unique_ptr<ServerPushHelper> push_helper_;
     std::unique_ptr<HttpRequestInfo> request_;
     std::unique_ptr<HttpTransaction> transaction_;
+    const NetLogWithSource net_log_;
   };
 
-  NetLogWithSource net_log_;
   // HttpCache must outlive the HttpCacheLookupManager.
+  NetLog* net_log_;

[Ryan Hamilton, 2017/02/06 23:34:55]

I think this should probably not come between |http_cache_| and the comment
about ownership of it.

[Zhongyi Shi, 2017/02/07 06:01:08]

Done.

   HttpCache* http_cache_;
   std::map<GURL, std::unique_ptr<LookupTransaction>> lookup_transactions_;
   base::WeakPtrFactory<HttpCacheLookupManager> weak_factory_;