predictors: Fix duplicate content type detection.

chrome/browser/predictors/resource_prefetch_predictor.h

 // Copyright 2014 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.
 
 #ifndef CHROME_BROWSER_PREDICTORS_RESOURCE_PREFETCH_PREDICTOR_H_
 #define CHROME_BROWSER_PREDICTORS_RESOURCE_PREFETCH_PREDICTOR_H_
 
 #include <stddef.h>
 
 #include <map>
@@ skipping 73 matching lines @@
 
     NavigationID navigation_id;
     GURL resource_url;
     content::ResourceType resource_type;
     net::RequestPriority priority;
 
     // Only for responses.
     std::string mime_type;
     bool was_cached;
     GURL redirect_url;  // Empty unless request was redirected to a valid url.
+
+    // Initializes a |URLRequestSummary| from a |URLRequest| response.
+    static bool SummarizeResponse(const net::URLRequest* request,

[pasko, 2016/08/09 17:08:54]

const net::URLRequest&

(yes, the const pointer is a majority, not sure why, it is against style guide,
here it is certainly not necessary to be a pointer)

[Benoit L, 2016/08/09 17:22:56]

Unfortunately, it is:
CONTENT_EXPORT static const ResourceRequestInfo* ForRequest(const
net::URLRequest* request);

[pasko, 2016/08/10 09:08:46]

Why would it be inconvenient to do the following?

content::ResourceRequestInfo::ForRequest(&request)

The content export is not beautiful, but let's try to make it easier to change
for better.

[Benoit L, 2016/08/10 14:08:47]

Done.

+                                  URLRequestSummary* summary);

[pasko, 2016/08/09 17:08:54]

Just returning a unique_ptr (instead of, ugh, output parameter) would make the
code more readable without comments and will make it easier to later remove the
evil default constructor for NavigationID.

Up to you for this small change, either to update the comment to say what this
function returns or to change the declaration.

[Benoit L, 2016/08/09 17:22:56]

The rest of the code uses URLRequestSummary by value, and output parameters are
common in Chrome.
Added a comment.

   };
 
   ResourcePrefetchPredictor(const ResourcePrefetchPredictorConfig& config,
                             Profile* profile);
   ~ResourcePrefetchPredictor() override;
 
   // Thread safe.
   static bool ShouldRecordRequest(net::URLRequest* request,
                                   content::ResourceType resource_type);
   static bool ShouldRecordResponse(net::URLRequest* response);
@@ skipping 229 matching lines @@
 
   ScopedObserver<history::HistoryService, history::HistoryServiceObserver>
       history_service_observer_;
 
   DISALLOW_COPY_AND_ASSIGN(ResourcePrefetchPredictor);
 };
 
 }  // namespace predictors
 
 #endif  // CHROME_BROWSER_PREDICTORS_RESOURCE_PREFETCH_PREDICTOR_H_