Move download attribution right after CheckDownloadUrl

chrome/browser/safe_browsing/download_protection_service.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.
 //
 // Helper class which handles communication with the SafeBrowsing servers for
 // improved binary download protection.
 
 #ifndef CHROME_BROWSER_SAFE_BROWSING_DOWNLOAD_PROTECTION_SERVICE_H_
 #define CHROME_BROWSER_SAFE_BROWSING_DOWNLOAD_PROTECTION_SERVICE_H_
 
@@ skipping 100 matching lines @@
   // is finished and written to disk.
   virtual void CheckClientDownload(
       content::DownloadItem* item,
       const CheckDownloadCallback& callback);
 
   // Checks whether any of the URLs in the redirect chain of the
   // download match the SafeBrowsing bad binary URL list.  The result is
   // delivered asynchronously via the given callback.  This method must be
   // called on the UI thread, and the callback will also be invoked on the UI
   // thread.  Pre-condition: !info.download_url_chain.empty().
-  virtual void CheckDownloadUrl(const content::DownloadItem& item,
+  virtual void CheckDownloadUrl(content::DownloadItem* item,
                                 const CheckDownloadCallback& callback);
 
   // Returns true iff the download specified by |info| should be scanned by
   // CheckClientDownload() for malicious content.
   virtual bool IsSupportedDownload(
       const content::DownloadItem& item,
       const base::FilePath& target_path) const;
 
   virtual void CheckPPAPIDownloadRequest(
       const GURL& requestor_url,
@@ skipping 84 matching lines @@
     REASON_LOCAL_FILE = 24,
     REASON_REMOTE_FILE = 25,
     REASON_SAMPLED_UNSUPPORTED_FILE = 26,
     REASON_VERDICT_UNKNOWN = 27,
     REASON_MAX  // Always add new values before this one.
   };
 
  private:
   class CheckClientDownloadRequest;
   class PPAPIDownloadRequest;
+  friend class DownloadSBClient;
   friend class DownloadProtectionServiceTest;
   friend class DownloadDangerPromptTest;
 
   FRIEND_TEST_ALL_PREFIXES(DownloadProtectionServiceTest,
                            CheckClientDownloadWhitelistedUrlWithoutSampling);
   FRIEND_TEST_ALL_PREFIXES(DownloadProtectionServiceTest,
                            CheckClientDownloadWhitelistedUrlWithSampling);
   FRIEND_TEST_ALL_PREFIXES(DownloadProtectionServiceTest,
                            CheckClientDownloadValidateRequest);
   FRIEND_TEST_ALL_PREFIXES(DownloadProtectionServiceTest,
@@ skipping 16 matching lines @@
                            PPAPIDownloadRequest_InvalidResponse);
   FRIEND_TEST_ALL_PREFIXES(DownloadProtectionServiceTest,
                            PPAPIDownloadRequest_Timeout);
   FRIEND_TEST_ALL_PREFIXES(DownloadProtectionServiceFlagTest,
                            CheckClientDownloadOverridenByFlag);
 
   static const char kDownloadRequestUrl[];
 
   static const void* const kDownloadPingTokenKey;
 
+  static const void* const kDownloadReferrerChainDataKey;

[Nathan Parker, 2017/01/24 22:46:46]

nit: could this be in a anonymous namespace in the .cc file instead?  This and
the others here.  If they're used in tests, then never mind.

[Jialiu Lin, 2017/01/25 00:11:20]

Done.
kDownloadPingTokenKey is used in a browser test. 
So I only put kDownloadReferrerChainDataKey in the anonymous namespace.

+
   // Helper class for easy setting and getting token string.
   class DownloadPingToken : public base::SupportsUserData::Data {
    public:
     explicit DownloadPingToken(const std::string& token)
        : token_string_(token) {}
 
     std::string token_string() {
       return token_string_;
     }
    private:
@@ skipping 16 matching lines @@
   // list of strings that need to be checked against the download whitelist to
   // determine whether the certificate is whitelisted.
   static void GetCertificateWhitelistStrings(
       const net::X509Certificate& certificate,
       const net::X509Certificate& issuer,
       std::vector<std::string>* whitelist_strings);
 
   // Returns the URL that will be used for download requests.
   static GURL GetDownloadRequestUrl();
 
-  // If kDownloadAttribution feature is enabled, identify and add referrer chain
-  // info of a download to ClientDownloadRequest proto. This function also
-  // records UMA stats of download attribution result.
-  void AddReferrerChainToClientDownloadRequest(
+  // If kDownloadAttribution feature is enabled, identify referrer chain info of
+  // a download. This function also records UMA stats of download attribution
+  // result.
+  void IdentifyReferrerChain(
     const GURL& download_url,
     content::WebContents* web_contents,
-    ClientDownloadRequest* out_request);
+    ReferrerChain* out_referrer_chain);
 
   // If kDownloadAttribution feature is enabled, identify referrer chain of the
   // PPAPI download based on the frame URL where the download is initiated.
   // Then add referrer chain info to ClientDownloadRequest proto. This function
   // also records UMA stats of download attribution result.
   void AddReferrerChainToPPAPIClientDownloadRequest(
     const GURL& initiating_frame_url,
     int tab_id,
     bool has_user_gesture,
     ClientDownloadRequest* out_request);
@@ skipping 39 matching lines @@
   std::set<std::string> manual_blacklist_hashes_;
 
   // Rate of whitelisted downloads we sample to send out download ping.
   double whitelist_sample_rate_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadProtectionService);
 };
 }  // namespace safe_browsing
 
 #endif  // CHROME_BROWSER_SAFE_BROWSING_DOWNLOAD_PROTECTION_SERVICE_H_