Skip to content

Commit

Permalink
Clear page load tokens on cookies deleted.
Browse files Browse the repository at this point in the history
As per feedback from privacy, page load tokens should be cleared when
cookies are deleted.

Bug: 1268158
Change-Id: I613ac67b74cf5c76c6b7df59b3154316146ec3e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3562369
Reviewed-by: Daniel Rubery <drubery@chromium.org>
Reviewed-by: Joshua Bell <jsbell@chromium.org>
Commit-Queue: Xinghui Lu <xinghuilu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988249}
  • Loading branch information
Xinghui Lu authored and Chromium LUCI CQ committed Apr 2, 2022
1 parent 8f50819 commit 419817a
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 1 deletion.
1 change: 1 addition & 0 deletions chrome/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2218,6 +2218,7 @@ static_library("browser") {
"//components/safe_browsing/content/browser/web_ui",
"//components/safe_browsing/core/browser",
"//components/safe_browsing/core/browser:safe_browsing_metrics_collector",
"//components/safe_browsing/core/browser:verdict_cache_manager",
"//components/safe_browsing/core/browser/db:database_manager",
"//components/safe_browsing/core/browser/realtime:policy_engine",
"//components/safe_browsing/core/browser/realtime:url_lookup_service",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
#include "chrome/browser/profiles/keep_alive/scoped_profile_keep_alive.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/safe_browsing/safe_browsing_service.h"
#include "chrome/browser/safe_browsing/verdict_cache_manager_factory.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "chrome/browser/share/share_history.h"
#include "chrome/browser/share/share_ranking.h"
Expand Down Expand Up @@ -116,6 +117,7 @@
#include "components/permissions/permission_decision_auto_blocker.h"
#include "components/prefs/pref_service.h"
#include "components/privacy_sandbox/privacy_sandbox_settings.h"
#include "components/safe_browsing/core/browser/verdict_cache_manager.h"
#include "components/search_engines/template_url_service.h"
#include "components/web_cache/browser/web_cache_manager.h"
#include "components/webrtc_logging/browser/log_cleanup.h"
Expand Down Expand Up @@ -628,6 +630,8 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
base::BindOnce(
&ChromeBrowsingDataRemoverDelegate::CreateTaskCompletionClosure,
base::Unretained(this), TracingDataType::kCookies));
safe_browsing::VerdictCacheManagerFactory::GetForProfile(profile_)
->OnCookiesDeleted();
}

if (filter_builder->GetMode() ==
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
#include "chrome/browser/permissions/permission_decision_auto_blocker_factory.h"
#include "chrome/browser/privacy_sandbox/privacy_sandbox_settings_factory.h"
#include "chrome/browser/safe_browsing/safe_browsing_service.h"
#include "chrome/browser/safe_browsing/verdict_cache_manager_factory.h"
#include "chrome/browser/signin/chrome_signin_client_factory.h"
#include "chrome/browser/signin/test_signin_client_builder.h"
#include "chrome/browser/spellchecker/spellcheck_custom_dictionary.h"
Expand Down Expand Up @@ -3308,3 +3309,23 @@ TEST_F(ChromeBrowsingDataRemoverDelegateTest,
BlockUntilBrowsingDataRemoved(AnHourAgo(), base::Time::Max(),
constants::DATA_TYPE_PASSWORDS, false);
}

// Verify that clearing cookies will also clear page load tokens.
TEST_F(ChromeBrowsingDataRemoverDelegateTest,
PageLoadTokenClearedOnCookieDeleted) {
GURL url("https://www.example.com/path");
safe_browsing::VerdictCacheManager* sb_cache_manager =
safe_browsing::VerdictCacheManagerFactory::GetForProfile(GetProfile());
sb_cache_manager->CreatePageLoadToken(url);
safe_browsing::ChromeUserPopulation::PageLoadToken token =
sb_cache_manager->GetPageLoadToken(url);
ASSERT_TRUE(token.has_token_value());

BlockUntilBrowsingDataRemoved(base::Time(), base::Time::Max(),
content::BrowsingDataRemover::DATA_TYPE_COOKIES,
false);

token = sb_cache_manager->GetPageLoadToken(url);
// Token is not found because cookies are deleted.
ASSERT_FALSE(token.has_token_value());
}
Original file line number Diff line number Diff line change
Expand Up @@ -766,6 +766,10 @@ void VerdictCacheManager::HistoryServiceBeingDeleted(
history_service_observation_.Reset();
}

void VerdictCacheManager::OnCookiesDeleted() {
CleanUpAllPageLoadTokens(ClearReason::kCookiesDeleted);
}

bool VerdictCacheManager::RemoveExpiredPhishGuardVerdicts(
LoginReputationClientRequest::TriggerType trigger_type,
base::Value* cache_dictionary) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ class VerdictCacheManager : public history::HistoryServiceObserver,
void HistoryServiceBeingDeleted(
history::HistoryService* history_service) override;

// Called by browsing data remover.
void OnCookiesDeleted();

// Returns true if an artificial unsafe URL has been provided using
// command-line flags.
static bool has_artificial_unsafe_url();
Expand Down Expand Up @@ -129,8 +132,9 @@ class VerdictCacheManager : public history::HistoryServiceObserver,
// histograms. Entries must not be removed or reordered.
enum class ClearReason {
kSafeBrowsingStateChanged = 0,
kCookiesDeleted = 1,

kMaxValue = kSafeBrowsingStateChanged
kMaxValue = kCookiesDeleted
};

void ScheduleNextCleanUpAfterInterval(base::TimeDelta interval);
Expand Down
1 change: 1 addition & 0 deletions tools/metrics/histograms/enums.xml
Original file line number Diff line number Diff line change
Expand Up @@ -79165,6 +79165,7 @@ https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_2.7.1.pdf

<enum name="SafeBrowsingPageLoadTokenClearReason">
<int value="0" label="Safe Browsing state changed"/>
<int value="1" label="Cookies deleted"/>
</enum>

<enum name="SafeBrowsingParseV4HashResult">
Expand Down

0 comments on commit 419817a

Please sign in to comment.