Commit 6c977f87 authored by Yao Xiao's avatar Yao Xiao Committed by Commit Bot

Recompute floc when history is deleted

Add a new enum value HISTORY_DELETE to the
FlocIdComputed_EventTrigger enum. When history is deleted, recompute
the floc and log the event accordingly.

Bug: 1062736
Change-Id: I236df7cc3246cef8f836de752d405e5c209e2395
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2364208
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Reviewed-by: default avatarJosh Karlin <jkarlin@chromium.org>
Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800167}
parent 26653ee7
...@@ -193,6 +193,17 @@ class FlocIdProviderWithCustomizedServicesBrowserTest ...@@ -193,6 +193,17 @@ class FlocIdProviderWithCustomizedServicesBrowserTest
run_loop.Run(); run_loop.Run();
} }
void ExpireHistoryBefore(base::Time end_time) {
base::RunLoop run_loop;
base::CancelableTaskTracker tracker;
HistoryServiceFactory::GetForProfile(browser()->profile(),
ServiceAccessType::EXPLICIT_ACCESS)
->ExpireHistoryBeforeForTesting(
end_time, base::BindLambdaForTesting([&]() { run_loop.Quit(); }),
&tracker);
run_loop.Run();
}
history::HistoryService* history_service() { history::HistoryService* history_service() {
return HistoryServiceFactory::GetForProfile( return HistoryServiceFactory::GetForProfile(
browser()->profile(), ServiceAccessType::IMPLICIT_ACCESS); browser()->profile(), ServiceAccessType::IMPLICIT_ACCESS);
...@@ -324,4 +335,46 @@ IN_PROC_BROWSER_TEST_F(FlocIdProviderWithCustomizedServicesBrowserTest, ...@@ -324,4 +335,46 @@ IN_PROC_BROWSER_TEST_F(FlocIdProviderWithCustomizedServicesBrowserTest,
ASSERT_EQ(0u, user_event_service()->GetRecordedUserEvents().size()); ASSERT_EQ(0u, user_event_service()->GetRecordedUserEvents().size());
} }
IN_PROC_BROWSER_TEST_F(FlocIdProviderWithCustomizedServicesBrowserTest,
HistoryDeleteRecomputeFloc) {
net::IPAddress::ConsiderLoopbackIPToBePubliclyRoutableForTesting();
ConfigureReplacementHostAndPortForRemotePermissionService();
std::string cookies_to_set = "/set-cookie?user_id=123";
ui_test_utils::NavigateToURL(
browser(), https_server_.GetURL(test_host(), cookies_to_set));
EXPECT_EQ(1u, GetHistoryUrls().size());
EXPECT_EQ(GetFlocId().ToDebugHeaderValue(), FlocId().ToDebugHeaderValue());
// Turn on sync-history to trigger the 1st floc computation.
sync_service()->SetActiveDataTypes(syncer::ModelTypeSet::All());
sync_service()->FireStateChanged();
FinishOutstandingRemotePermissionQueries();
FinishOutstandingHistoryQueries();
ASSERT_EQ(1u, user_event_service()->GetRecordedUserEvents().size());
ExpireHistoryBefore(base::Time::Now());
FinishOutstandingRemotePermissionQueries();
FinishOutstandingHistoryQueries();
// Expect that the 2nd FlocIdComputed event should be due to history deletion.
ASSERT_EQ(2u, user_event_service()->GetRecordedUserEvents().size());
const sync_pb::UserEventSpecifics& specifics =
user_event_service()->GetRecordedUserEvents()[1];
EXPECT_EQ(sync_pb::UserEventSpecifics::kFlocIdComputedEvent,
specifics.event_case());
const sync_pb::UserEventSpecifics_FlocIdComputed& event =
specifics.floc_id_computed_event();
EXPECT_EQ(sync_pb::UserEventSpecifics::FlocIdComputed::HISTORY_DELETE,
event.event_trigger());
EXPECT_FALSE(event.has_floc_id());
}
} // namespace federated_learning } // namespace federated_learning
...@@ -44,6 +44,7 @@ FlocIdProviderImpl::FlocIdProviderImpl( ...@@ -44,6 +44,7 @@ FlocIdProviderImpl::FlocIdProviderImpl(
floc_remote_permission_service_(floc_remote_permission_service), floc_remote_permission_service_(floc_remote_permission_service),
history_service_(history_service), history_service_(history_service),
user_event_service_(user_event_service) { user_event_service_(user_event_service) {
history_service->AddObserver(this);
sync_service_->AddObserver(this); sync_service_->AddObserver(this);
OnStateChanged(sync_service); OnStateChanged(sync_service);
} }
...@@ -61,10 +62,21 @@ void FlocIdProviderImpl::NotifyFlocUpdated(ComputeFlocTrigger trigger) { ...@@ -61,10 +62,21 @@ void FlocIdProviderImpl::NotifyFlocUpdated(ComputeFlocTrigger trigger) {
sync_pb::UserEventSpecifics_FlocIdComputed* const floc_id_computed_event = sync_pb::UserEventSpecifics_FlocIdComputed* const floc_id_computed_event =
specifics->mutable_floc_id_computed_event(); specifics->mutable_floc_id_computed_event();
sync_pb::UserEventSpecifics_FlocIdComputed_EventTrigger event_trigger = sync_pb::UserEventSpecifics_FlocIdComputed_EventTrigger event_trigger;
(trigger == ComputeFlocTrigger::kBrowserStart) switch (trigger) {
? sync_pb::UserEventSpecifics_FlocIdComputed_EventTrigger_NEW case ComputeFlocTrigger::kBrowserStart:
: sync_pb::UserEventSpecifics_FlocIdComputed_EventTrigger_REFRESHED; event_trigger =
sync_pb::UserEventSpecifics_FlocIdComputed_EventTrigger_NEW;
break;
case ComputeFlocTrigger::kScheduledUpdate:
event_trigger =
sync_pb::UserEventSpecifics_FlocIdComputed_EventTrigger_REFRESHED;
break;
case ComputeFlocTrigger::kHistoryDelete:
event_trigger = sync_pb::
UserEventSpecifics_FlocIdComputed_EventTrigger_HISTORY_DELETE;
break;
}
floc_id_computed_event->set_event_trigger(event_trigger); floc_id_computed_event->set_event_trigger(event_trigger);
...@@ -133,9 +145,22 @@ void FlocIdProviderImpl::IsSwaaNacAccountEnabled( ...@@ -133,9 +145,22 @@ void FlocIdProviderImpl::IsSwaaNacAccountEnabled(
} }
void FlocIdProviderImpl::Shutdown() { void FlocIdProviderImpl::Shutdown() {
if (sync_service_ && sync_service_->HasObserver(this)) if (sync_service_)
sync_service_->RemoveObserver(this); sync_service_->RemoveObserver(this);
sync_service_ = nullptr; sync_service_ = nullptr;
if (history_service_)
history_service_->RemoveObserver(this);
history_service_ = nullptr;
}
void FlocIdProviderImpl::OnURLsDeleted(
history::HistoryService* history_service,
const history::DeletionInfo& deletion_info) {
if (!first_floc_computation_triggered_ || !floc_id_.IsValid())
return;
ComputeFloc(ComputeFlocTrigger::kHistoryDelete);
} }
void FlocIdProviderImpl::OnStateChanged(syncer::SyncService* sync_service) { void FlocIdProviderImpl::OnStateChanged(syncer::SyncService* sync_service) {
...@@ -155,7 +180,15 @@ void FlocIdProviderImpl::ComputeFloc(ComputeFlocTrigger trigger) { ...@@ -155,7 +180,15 @@ void FlocIdProviderImpl::ComputeFloc(ComputeFlocTrigger trigger) {
DCHECK(trigger != ComputeFlocTrigger::kBrowserStart || DCHECK(trigger != ComputeFlocTrigger::kBrowserStart ||
!floc_computation_in_progress_); !floc_computation_in_progress_);
// Skip computing as long as there's one still in progress. // It's fine to skip computing as long as there's one in progress:
// 1) If the incoming computation was triggered by history deletion, then the
// in-progress one must haven't got its history query result (if it would
// reach that stage), so the history query result wouldn't contain the
// deleted entries anyway.
// 2) If the incoming computation was triggered by scheduled update and is
// ignored, we won't be losing the recomputing frequency, as the
// in-progress one only occurs sooner and when it finishes a new
// compute-floc task will be scheduled.
if (floc_computation_in_progress_) if (floc_computation_in_progress_)
return; return;
......
...@@ -39,13 +39,17 @@ class FlocRemotePermissionService; ...@@ -39,13 +39,17 @@ class FlocRemotePermissionService;
// given. // given.
// //
// The floc will be first computed after sync & sync-history are enabled. After // The floc will be first computed after sync & sync-history are enabled. After
// each computation, another computation will be scheduled 24 hours later. // each computation, another computation will be scheduled 24 hours later. In
// the event of history deletion, the floc will be recomputed immediately and
// reset the timer of any currently scheduled computation to be 24 hours later.
class FlocIdProviderImpl : public FlocIdProvider, class FlocIdProviderImpl : public FlocIdProvider,
public history::HistoryServiceObserver,
public syncer::SyncServiceObserver { public syncer::SyncServiceObserver {
public: public:
enum class ComputeFlocTrigger { enum class ComputeFlocTrigger {
kBrowserStart, kBrowserStart,
kScheduledUpdate, kScheduledUpdate,
kHistoryDelete,
}; };
using CanComputeFlocCallback = base::OnceCallback<void(bool)>; using CanComputeFlocCallback = base::OnceCallback<void(bool)>;
...@@ -77,6 +81,13 @@ class FlocIdProviderImpl : public FlocIdProvider, ...@@ -77,6 +81,13 @@ class FlocIdProviderImpl : public FlocIdProvider,
// KeyedService: // KeyedService:
void Shutdown() override; void Shutdown() override;
// history::HistoryServiceObserver
//
// On history deletion, recompute the floc if the current floc is speculated
// to be derived from the deleted history.
void OnURLsDeleted(history::HistoryService* history_service,
const history::DeletionInfo& deletion_info) override;
// syncer::SyncServiceObserver: // syncer::SyncServiceObserver:
void OnStateChanged(syncer::SyncService* sync_service) override; void OnStateChanged(syncer::SyncService* sync_service) override;
......
...@@ -104,6 +104,10 @@ class FlocIdProviderUnitTest : public testing::Test { ...@@ -104,6 +104,10 @@ class FlocIdProviderUnitTest : public testing::Test {
task_environment_.RunUntilIdle(); task_environment_.RunUntilIdle();
} }
void TearDown() override {
history_service_->RemoveObserver(floc_id_provider_.get());
}
void CheckCanComputeFloc( void CheckCanComputeFloc(
FlocIdProviderImpl::CanComputeFlocCallback callback) { FlocIdProviderImpl::CanComputeFlocCallback callback) {
floc_id_provider_->CheckCanComputeFloc(std::move(callback)); floc_id_provider_->CheckCanComputeFloc(std::move(callback));
...@@ -114,6 +118,11 @@ class FlocIdProviderUnitTest : public testing::Test { ...@@ -114,6 +118,11 @@ class FlocIdProviderUnitTest : public testing::Test {
floc_id_provider_->IsSwaaNacAccountEnabled(std::move(callback)); floc_id_provider_->IsSwaaNacAccountEnabled(std::move(callback));
} }
void OnURLsDeleted(history::HistoryService* history_service,
const history::DeletionInfo& deletion_info) {
floc_id_provider_->OnURLsDeleted(history_service, deletion_info);
}
void OnGetRecentlyVisitedURLsCompleted( void OnGetRecentlyVisitedURLsCompleted(
FlocIdProviderImpl::ComputeFlocTrigger trigger, FlocIdProviderImpl::ComputeFlocTrigger trigger,
history::QueryResults results) { history::QueryResults results) {
...@@ -125,6 +134,15 @@ class FlocIdProviderUnitTest : public testing::Test { ...@@ -125,6 +134,15 @@ class FlocIdProviderUnitTest : public testing::Test {
std::move(compute_floc_completed_callback), std::move(results)); std::move(compute_floc_completed_callback), std::move(results));
} }
void ExpireHistoryBefore(base::Time end_time) {
base::RunLoop run_loop;
base::CancelableTaskTracker tracker;
history_service_->ExpireHistoryBeforeForTesting(
end_time, base::BindLambdaForTesting([&]() { run_loop.Quit(); }),
&tracker);
run_loop.Run();
}
FlocId floc_id() const { return floc_id_provider_->floc_id_; } FlocId floc_id() const { return floc_id_provider_->floc_id_; }
void set_floc_id(const FlocId& floc_id) const { void set_floc_id(const FlocId& floc_id) const {
...@@ -258,6 +276,65 @@ TEST_F(FlocIdProviderUnitTest, UnqualifiedInitialHistory) { ...@@ -258,6 +276,65 @@ TEST_F(FlocIdProviderUnitTest, UnqualifiedInitialHistory) {
floc_id().ToDebugHeaderValue()); floc_id().ToDebugHeaderValue());
} }
TEST_F(FlocIdProviderUnitTest, HistoryDeleteAndScheduledUpdate) {
std::string domain1 = "foo.com";
std::string domain2 = "bar.com";
// Add a history entry with a timestamp exactly 7 days back from now.
history::HistoryAddPageArgs add_page_args;
add_page_args.url = GURL(base::StrCat({"https://www.", domain1}));
add_page_args.time = base::Time::Now() - base::TimeDelta::FromDays(7);
add_page_args.publicly_routable = true;
history_service_->AddPage(add_page_args);
// Add a history entry with a timestamp exactly 6 days back from now.
add_page_args.url = GURL(base::StrCat({"https://www.", domain2}));
add_page_args.time = base::Time::Now() - base::TimeDelta::FromDays(6);
history_service_->AddPage(add_page_args);
task_environment_.RunUntilIdle();
// Trigger the 1st floc computation.
test_sync_service_->SetTransportState(
syncer::SyncService::TransportState::ACTIVE);
test_sync_service_->FireStateChanged();
task_environment_.RunUntilIdle();
// Expect a floc id update notification.
ASSERT_EQ(1u, floc_id_provider_->floc_update_notification_count());
ASSERT_TRUE(floc_id().IsValid());
ASSERT_EQ(FlocId::CreateFromHistory({domain1, domain2}).ToDebugHeaderValue(),
floc_id().ToDebugHeaderValue());
// Advance the clock by 12 hours. Expect no floc id update notification.
task_environment_.FastForwardBy(base::TimeDelta::FromHours(12));
ASSERT_EQ(1u, floc_id_provider_->floc_update_notification_count());
// Expire the oldest history entry.
ExpireHistoryBefore(base::Time::Now() - base::TimeDelta::FromDays(7));
task_environment_.RunUntilIdle();
// Expect a floc id update notification as it was just recomputed due to the
// history deletion.
ASSERT_EQ(2u, floc_id_provider_->floc_update_notification_count());
ASSERT_TRUE(floc_id().IsValid());
ASSERT_EQ(FlocId::CreateFromHistory({domain2}).ToDebugHeaderValue(),
floc_id().ToDebugHeaderValue());
// Advance the clock by 23 hours. Expect no floc id update notification as the
// timer has been reset due to the recomputation from history deletion.
task_environment_.FastForwardBy(base::TimeDelta::FromHours(23));
ASSERT_EQ(2u, floc_id_provider_->floc_update_notification_count());
// Advance the clock by 1 hour. Expect an floc id update notification as the
// scheduled time is reached. Expect an invalid floc id as there is no history
// in the past 7 days.
task_environment_.FastForwardBy(base::TimeDelta::FromHours(1));
ASSERT_EQ(3u, floc_id_provider_->floc_update_notification_count());
ASSERT_FALSE(floc_id().IsValid());
}
TEST_F(FlocIdProviderUnitTest, ScheduledUpdateSameFloc_NoNotification) { TEST_F(FlocIdProviderUnitTest, ScheduledUpdateSameFloc_NoNotification) {
std::string domain = "foo.com"; std::string domain = "foo.com";
...@@ -429,6 +506,97 @@ TEST_F(FlocIdProviderUnitTest, EventLogging) { ...@@ -429,6 +506,97 @@ TEST_F(FlocIdProviderUnitTest, EventLogging) {
EXPECT_EQ(sync_pb::UserEventSpecifics::FlocIdComputed::REFRESHED, EXPECT_EQ(sync_pb::UserEventSpecifics::FlocIdComputed::REFRESHED,
event3.event_trigger()); event3.event_trigger());
EXPECT_FALSE(event3.has_floc_id()); EXPECT_FALSE(event3.has_floc_id());
set_floc_id(FlocId(555));
floc_id_provider_->NotifyFlocUpdated(
FlocIdProviderImpl::ComputeFlocTrigger::kHistoryDelete);
ASSERT_EQ(4u, fake_user_event_service_->GetRecordedUserEvents().size());
const sync_pb::UserEventSpecifics& specifics4 =
fake_user_event_service_->GetRecordedUserEvents()[3];
EXPECT_EQ(specifics4.event_time_usec(),
base::Time::Now().ToDeltaSinceWindowsEpoch().InMicroseconds());
EXPECT_EQ(sync_pb::UserEventSpecifics::kFlocIdComputedEvent,
specifics4.event_case());
const sync_pb::UserEventSpecifics_FlocIdComputed& event4 =
specifics4.floc_id_computed_event();
EXPECT_EQ(sync_pb::UserEventSpecifics::FlocIdComputed::HISTORY_DELETE,
event4.event_trigger());
EXPECT_EQ(555ULL, event4.floc_id());
}
TEST_F(FlocIdProviderUnitTest, HistoryDelete_AllHistory) {
base::Time time = base::Time::Now() - base::TimeDelta::FromDays(9);
history::URLResult url_result(GURL("https://a.test"), time);
url_result.set_publicly_routable(true);
history::QueryResults query_results;
query_results.SetURLResults({url_result});
set_first_floc_computation_triggered(true);
set_floc_computation_in_progress(true);
OnGetRecentlyVisitedURLsCompleted(
FlocIdProviderImpl::ComputeFlocTrigger::kBrowserStart,
std::move(query_results));
ASSERT_TRUE(floc_id().IsValid());
OnURLsDeleted(history_service_.get(), history::DeletionInfo::ForAllHistory());
ASSERT_FALSE(floc_id().IsValid());
}
TEST_F(FlocIdProviderUnitTest, HistoryDelete_InvalidTimeRange) {
base::Time time = base::Time::Now() - base::TimeDelta::FromDays(9);
GURL url_a = GURL("https://a.test");
history::URLResult url_result(url_a, time);
url_result.set_publicly_routable(true);
history::QueryResults query_results;
query_results.SetURLResults({url_result});
set_first_floc_computation_triggered(true);
set_floc_computation_in_progress(true);
OnGetRecentlyVisitedURLsCompleted(
FlocIdProviderImpl::ComputeFlocTrigger::kBrowserStart,
std::move(query_results));
ASSERT_TRUE(floc_id().IsValid());
OnURLsDeleted(history_service_.get(),
history::DeletionInfo::ForUrls(
{history::URLResult(url_a, base::Time())}, {}));
task_environment_.RunUntilIdle();
ASSERT_FALSE(floc_id().IsValid());
}
TEST_F(FlocIdProviderUnitTest, HistoryDelete_TimeRange) {
base::Time time = base::Time::Now() - base::TimeDelta::FromDays(9);
history::URLResult url_result(GURL("https://a.test"), time);
url_result.set_publicly_routable(true);
history::QueryResults query_results;
query_results.SetURLResults({url_result});
set_first_floc_computation_triggered(true);
set_floc_computation_in_progress(true);
OnGetRecentlyVisitedURLsCompleted(
FlocIdProviderImpl::ComputeFlocTrigger::kBrowserStart,
std::move(query_results));
ASSERT_TRUE(floc_id().IsValid());
history::DeletionInfo deletion_info(history::DeletionTimeRange(time, time),
false, {}, {},
base::Optional<std::set<GURL>>());
OnURLsDeleted(history_service_.get(), deletion_info);
task_environment_.RunUntilIdle();
ASSERT_FALSE(floc_id().IsValid());
} }
TEST_F(FlocIdProviderUnitTest, HistoryEntriesWithPrivateIP) { TEST_F(FlocIdProviderUnitTest, HistoryEntriesWithPrivateIP) {
......
...@@ -446,11 +446,12 @@ const char* ProtoEnumToString( ...@@ -446,11 +446,12 @@ const char* ProtoEnumToString(
const char* ProtoEnumToString( const char* ProtoEnumToString(
sync_pb::UserEventSpecifics::FlocIdComputed::EventTrigger trigger) { sync_pb::UserEventSpecifics::FlocIdComputed::EventTrigger trigger) {
ASSERT_ENUM_BOUNDS(sync_pb::UserEventSpecifics::FlocIdComputed, EventTrigger, ASSERT_ENUM_BOUNDS(sync_pb::UserEventSpecifics::FlocIdComputed, EventTrigger,
UNSPECIFIED, REFRESHED); UNSPECIFIED, HISTORY_DELETE);
switch (trigger) { switch (trigger) {
ENUM_CASE(sync_pb::UserEventSpecifics::FlocIdComputed, UNSPECIFIED); ENUM_CASE(sync_pb::UserEventSpecifics::FlocIdComputed, UNSPECIFIED);
ENUM_CASE(sync_pb::UserEventSpecifics::FlocIdComputed, NEW); ENUM_CASE(sync_pb::UserEventSpecifics::FlocIdComputed, NEW);
ENUM_CASE(sync_pb::UserEventSpecifics::FlocIdComputed, REFRESHED); ENUM_CASE(sync_pb::UserEventSpecifics::FlocIdComputed, REFRESHED);
ENUM_CASE(sync_pb::UserEventSpecifics::FlocIdComputed, HISTORY_DELETE);
} }
NOTREACHED(); NOTREACHED();
return ""; return "";
......
...@@ -187,8 +187,12 @@ message UserEventSpecifics { ...@@ -187,8 +187,12 @@ message UserEventSpecifics {
// Event added because the floc id is re-computed due to a long period of // Event added because the floc id is re-computed due to a long period of
// time has passed since the last computation. // time has passed since the last computation.
REFRESHED = 2; REFRESHED = 2;
// Event added because the floc id is re-computed due to history deletion.
HISTORY_DELETE = 3;
} }
optional EventTrigger event_trigger = 1; optional EventTrigger event_trigger = 1;
// If not set, it means that the floc is disabled.
optional uint64 floc_id = 2; optional uint64 floc_id = 2;
} }
......
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