Commit ef69dc9a authored by Jialiu Lin's avatar Jialiu Lin Committed by Commit Bot

Fix missing download referrer chain

This is a regression introduced by a refactoring CL. SupportUserData key
should be unique, but due to this refactoring, one of the data key
losts its sameness. This make SetUserData() and GetUserData()
consume different keys. This CL fix this problem.

Also move the logging of "SafeBrowsing.ReferrerURLChainSize.
DownloadAttribution" UMA metric right before report sending to help
catch this kind of regression in the future.

Bug: 789217
Change-Id: I778926b2d2ede69ad2b7c8327ce835104faa7852
Reviewed-on: https://chromium-review.googlesource.com/794512
Commit-Queue: Jialiu Lin <jialiul@chromium.org>
Reviewed-by: default avatarNathan Parker <nparker@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519881}
parent 08c69bc2
...@@ -860,11 +860,14 @@ void CheckClientDownloadRequest::SendRequest() { ...@@ -860,11 +860,14 @@ void CheckClientDownloadRequest::SendRequest() {
request.set_download_type(type_); request.set_download_type(type_);
ReferrerChainData* referrer_chain_data = static_cast<ReferrerChainData*>( ReferrerChainData* referrer_chain_data = static_cast<ReferrerChainData*>(
item_->GetUserData(kDownloadReferrerChainDataKey)); item_->GetUserData(ReferrerChainData::kDownloadReferrerChainDataKey));
if (referrer_chain_data && if (referrer_chain_data &&
!referrer_chain_data->GetReferrerChain()->empty()) { !referrer_chain_data->GetReferrerChain()->empty()) {
request.mutable_referrer_chain()->Swap( request.mutable_referrer_chain()->Swap(
referrer_chain_data->GetReferrerChain()); referrer_chain_data->GetReferrerChain());
UMA_HISTOGRAM_COUNTS_100(
"SafeBrowsing.ReferrerURLChainSize.DownloadAttribution",
request.referrer_chain().size());
if (type_ == ClientDownloadRequest::SAMPLED_UNSUPPORTED_FILE) if (type_ == ClientDownloadRequest::SAMPLED_UNSUPPORTED_FILE)
SafeBrowsingNavigationObserverManager::SanitizeReferrerChain( SafeBrowsingNavigationObserverManager::SanitizeReferrerChain(
request.mutable_referrer_chain()); request.mutable_referrer_chain());
......
...@@ -387,9 +387,6 @@ std::unique_ptr<ReferrerChain> DownloadProtectionService::IdentifyReferrerChain( ...@@ -387,9 +387,6 @@ std::unique_ptr<ReferrerChain> DownloadProtectionService::IdentifyReferrerChain(
referrer_chain.get()); referrer_chain.get());
} }
UMA_HISTOGRAM_COUNTS_100(
"SafeBrowsing.ReferrerURLChainSize.DownloadAttribution",
referrer_chain->size());
UMA_HISTOGRAM_ENUMERATION( UMA_HISTOGRAM_ENUMERATION(
"SafeBrowsing.ReferrerAttributionResult.DownloadAttribution", result, "SafeBrowsing.ReferrerAttributionResult.DownloadAttribution", result,
SafeBrowsingNavigationObserverManager::ATTRIBUTION_FAILURE_TYPE_MAX); SafeBrowsingNavigationObserverManager::ATTRIBUTION_FAILURE_TYPE_MAX);
......
...@@ -13,9 +13,6 @@ ...@@ -13,9 +13,6 @@
namespace safe_browsing { namespace safe_browsing {
const void* const kDownloadReferrerChainDataKey =
&kDownloadReferrerChainDataKey;
enum class DownloadCheckResult { enum class DownloadCheckResult {
UNKNOWN, UNKNOWN,
SAFE, SAFE,
......
...@@ -129,7 +129,7 @@ void DownloadUrlSBClient::IdentifyReferrerChain() { ...@@ -129,7 +129,7 @@ void DownloadUrlSBClient::IdentifyReferrerChain() {
if (!item_) if (!item_)
return; return;
item_->SetUserData(kDownloadReferrerChainDataKey, item_->SetUserData(ReferrerChainData::kDownloadReferrerChainDataKey,
base::MakeUnique<ReferrerChainData>( base::MakeUnique<ReferrerChainData>(
service_->IdentifyReferrerChain(*item_))); service_->IdentifyReferrerChain(*item_)));
} }
......
...@@ -84,6 +84,11 @@ static const int kNavigationRecordMaxSize = 100; ...@@ -84,6 +84,11 @@ static const int kNavigationRecordMaxSize = 100;
static const int kReferrerChainMaxLength = 10; static const int kReferrerChainMaxLength = 10;
// -------------------------ReferrerChainData----------------------- // -------------------------ReferrerChainData-----------------------
// String value of kDownloadReferrerChainDataKey is not used.
const char ReferrerChainData::kDownloadReferrerChainDataKey[] =
"referrer_chain_data_key";
ReferrerChainData::ReferrerChainData( ReferrerChainData::ReferrerChainData(
std::unique_ptr<ReferrerChain> referrer_chain) std::unique_ptr<ReferrerChain> referrer_chain)
: referrer_chain_(std::move(referrer_chain)) {} : referrer_chain_(std::move(referrer_chain)) {}
......
...@@ -31,6 +31,10 @@ class ReferrerChainData : public base::SupportsUserData::Data { ...@@ -31,6 +31,10 @@ class ReferrerChainData : public base::SupportsUserData::Data {
~ReferrerChainData() override; ~ReferrerChainData() override;
ReferrerChain* GetReferrerChain(); ReferrerChain* GetReferrerChain();
// Unique user data key used to get and set referrer chain data in
// DownloadItem.
static const char kDownloadReferrerChainDataKey[];
private: private:
std::unique_ptr<ReferrerChain> referrer_chain_; std::unique_ptr<ReferrerChain> referrer_chain_;
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment