Skip to content

Commit

Permalink
[DBSC] Rename BoundSessionParams to BoundSessionThrottlerParams
Browse files Browse the repository at this point in the history
Bug: b/297165201
Change-Id: I1c0993191ace36c85c0378c5daa8b03382ac9601
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4805886
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Commit-Queue: Monica Basta <msalama@chromium.org>
Auto-Submit: Monica Basta <msalama@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1187232}
  • Loading branch information
Monica Basta authored and Chromium LUCI CQ committed Aug 23, 2023
1 parent 96f70e2 commit 1764a40
Show file tree
Hide file tree
Showing 15 changed files with 138 additions and 118 deletions.
8 changes: 4 additions & 4 deletions chrome/browser/chrome_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5622,21 +5622,21 @@ std::unique_ptr<blink::URLLoaderThrottle> CreateGoogleURLLoaderThrottle(
BoundSessionCookieRefreshServiceFactory::GetForProfile(profile);
std::unique_ptr<BoundSessionRequestThrottledListener>
bound_session_request_throttled_listener;
chrome::mojom::BoundSessionParamsPtr bound_session_params;
chrome::mojom::BoundSessionThrottlerParamsPtr bound_session_throttler_params;

if (bound_session_cookie_refresh_service) {
bound_session_request_throttled_listener =
std::make_unique<BoundSessionRequestThrottledListenerBrowserImpl>(
*bound_session_cookie_refresh_service);
bound_session_params =
bound_session_cookie_refresh_service->GetBoundSessionParams();
bound_session_throttler_params =
bound_session_cookie_refresh_service->GetBoundSessionThrottlerParams();
}
#endif

chrome::mojom::DynamicParamsPtr dynamic_params =
chrome::mojom::DynamicParams::New(
#if BUILDFLAG(ENABLE_BOUND_SESSION_CREDENTIALS)
std::move(bound_session_params),
std::move(bound_session_throttler_params),
#endif
profile->GetPrefs()->GetBoolean(
policy::policy_prefs::kForceGoogleSafeSearch),
Expand Down
18 changes: 10 additions & 8 deletions chrome/browser/profiles/renderer_updater.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,9 @@ RendererUpdater::RendererUpdater(Profile* profile)
if (bound_session_cookie_refresh_service_) {
// `base::Unretained` is safe as `this` deregister itself on destruction.
bound_session_cookie_refresh_service_
->SetRendererBoundSessionParamsUpdaterDelegate(base::BindRepeating(
&RendererUpdater::UpdateAllRenderers, base::Unretained(this)));
->SetRendererBoundSessionThrottlerParamsUpdaterDelegate(
base::BindRepeating(&RendererUpdater::UpdateAllRenderers,
base::Unretained(this)));
}
#endif

Expand Down Expand Up @@ -99,7 +100,7 @@ RendererUpdater::~RendererUpdater() {
#if BUILDFLAG(ENABLE_BOUND_SESSION_CREDENTIALS)
if (bound_session_cookie_refresh_service_) {
bound_session_cookie_refresh_service_
->SetRendererBoundSessionParamsUpdaterDelegate(
->SetRendererBoundSessionThrottlerParamsUpdaterDelegate(
base::RepeatingClosure());
}
#endif
Expand Down Expand Up @@ -212,12 +213,13 @@ void RendererUpdater::OnPrimaryAccountChanged(
}

#if BUILDFLAG(ENABLE_BOUND_SESSION_CREDENTIALS)
chrome::mojom::BoundSessionParamsPtr RendererUpdater::GetBoundSessionParams()
const {
chrome::mojom::BoundSessionThrottlerParamsPtr
RendererUpdater::GetBoundSessionThrottlerParams() const {
if (bound_session_cookie_refresh_service_) {
return bound_session_cookie_refresh_service_->GetBoundSessionParams();
return bound_session_cookie_refresh_service_
->GetBoundSessionThrottlerParams();
}
return chrome::mojom::BoundSessionParamsPtr();
return chrome::mojom::BoundSessionThrottlerParamsPtr();
}
#endif // BUILDFLAG(ENABLE_BOUND_SESSION_CREDENTIALS)

Expand All @@ -239,7 +241,7 @@ chrome::mojom::DynamicParamsPtr RendererUpdater::CreateRendererDynamicParams()
const {
return chrome::mojom::DynamicParams::New(
#if BUILDFLAG(ENABLE_BOUND_SESSION_CREDENTIALS)
GetBoundSessionParams(),
GetBoundSessionThrottlerParams(),
#endif
force_google_safesearch_.GetValue(), force_youtube_restrict_.GetValue(),
allowed_domains_for_apps_.GetValue());
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/profiles/renderer_updater.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,10 @@ class RendererUpdater : public KeyedService,
void UpdateAllRenderers();

#if BUILDFLAG(ENABLE_BOUND_SESSION_CREDENTIALS)
// Creates bound session parameters that are subset of the dynamic
// Creates bound session throttler parameters that are subset of the dynamic
// renderer parameters.
chrome::mojom::BoundSessionParamsPtr GetBoundSessionParams() const;
chrome::mojom::BoundSessionThrottlerParamsPtr GetBoundSessionThrottlerParams()
const;
#endif

// Create renderer configuration that changes at runtime.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ base::Time BoundSessionCookieController::min_cookie_expiration_time() {
->second;
}

chrome::mojom::BoundSessionParamsPtr
BoundSessionCookieController::bound_session_params() {
return chrome::mojom::BoundSessionParams::New(url().host(), url().path(),
min_cookie_expiration_time());
chrome::mojom::BoundSessionThrottlerParamsPtr
BoundSessionCookieController::bound_session_throttler_params() {
return chrome::mojom::BoundSessionThrottlerParams::New(
url().host(), url().path(), min_cookie_expiration_time());
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class BoundSessionCookieController {
// Called when the bound session parameters change, for example the minimum
// cookie expiration date changes. Cookie deletion is considered as a change
// in the expiration date to the null time.
virtual void OnBoundSessionParamsChanged() = 0;
virtual void OnBoundSessionThrottlerParamsChanged() = 0;
};

BoundSessionCookieController(
Expand All @@ -61,7 +61,8 @@ class BoundSessionCookieController {

const GURL& url() const { return url_; }
base::Time min_cookie_expiration_time();
chrome::mojom::BoundSessionParamsPtr bound_session_params();
chrome::mojom::BoundSessionThrottlerParamsPtr
bound_session_throttler_params();

protected:
const GURL url_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ void BoundSessionCookieControllerImpl::SetCookieExpirationTimeAndNotify(
}

if (min_cookie_expiration_time() != old_min_expiration_time) {
delegate_->OnBoundSessionParamsChanged();
delegate_->OnBoundSessionThrottlerParamsChanged();
MaybeScheduleCookieRotation();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@ class BoundSessionCookieControllerImplTest

base::test::TaskEnvironment* task_environment() { return &task_environment_; }

void OnBoundSessionParamsChanged() override {
on_bound_session_params_changed_call_count_++;
void OnBoundSessionThrottlerParamsChanged() override {
on_bound_session_throttler_params_changed_call_count_++;
}

void TerminateSession() override { on_terminate_session_called_ = true; }
Expand Down Expand Up @@ -219,16 +219,16 @@ class BoundSessionCookieControllerImplTest

const UnexportableKeyId& key_id() { return key_id_; }

size_t on_bound_session_params_changed_call_count() {
return on_bound_session_params_changed_call_count_;
size_t on_bound_session_throttler_params_changed_call_count() {
return on_bound_session_throttler_params_changed_call_count_;
}

bool on_cookie_refresh_persistent_failure_called() {
return on_terminate_session_called_;
}

void ResetOnBoundSessionParamsChangedCallCount() {
on_bound_session_params_changed_call_count_ = 0;
void ResetOnBoundSessionThrottlerParamsChangedCallCount() {
on_bound_session_throttler_params_changed_call_count_ = 0;
}

void ResetBoundSessionCookieController() {
Expand Down Expand Up @@ -267,7 +267,7 @@ class BoundSessionCookieControllerImplTest
bound_session_cookie_controller_;
raw_ptr<FakeBoundSessionRefreshCookieFetcher, DanglingUntriaged>
cookie_fetcher_ = nullptr;
size_t on_bound_session_params_changed_call_count_ = 0;
size_t on_bound_session_throttler_params_changed_call_count_ = 0;
bool on_terminate_session_called_ = false;
};

Expand All @@ -287,7 +287,7 @@ TEST_F(BoundSessionCookieControllerImplTest, TwoCookieObserversCreated) {

TEST_F(BoundSessionCookieControllerImplTest, CookieRefreshOnStartup) {
EXPECT_TRUE(CompletePendingRefreshRequestIfAny());
EXPECT_EQ(on_bound_session_params_changed_call_count(), 1u);
EXPECT_EQ(on_bound_session_throttler_params_changed_call_count(), 1u);
EXPECT_EQ(cookie_expiration_time(k1PSIDTSCookieName),
GetTimeInTenMinutes() - kCookieExpirationThreshold);
EXPECT_EQ(cookie_expiration_time(k3PSIDTSCookieName),
Expand All @@ -298,7 +298,7 @@ TEST_F(BoundSessionCookieControllerImplTest, CookieRefreshOnStartup) {
TEST_F(BoundSessionCookieControllerImplTest,
MaybeRefreshCookieMultipleRequests) {
CompletePendingRefreshRequestIfAny();
ResetOnBoundSessionParamsChangedCallCount();
ResetOnBoundSessionThrottlerParamsChangedCallCount();

EXPECT_FALSE(cookie_fetcher());
MaybeRefreshCookie();
Expand All @@ -314,26 +314,26 @@ TEST_F(BoundSessionCookieControllerImplTest,
TEST_F(BoundSessionCookieControllerImplTest,
NotifiesOnlyIfMinimumCookieExpirationDateChanged) {
CompletePendingRefreshRequestIfAny();
ResetOnBoundSessionParamsChangedCallCount();
ResetOnBoundSessionThrottlerParamsChangedCallCount();

// Update with the same date.
SetExpirationTimeAndNotify(
k1PSIDTSCookieName,
cookie_expiration_time(k1PSIDTSCookieName) + kCookieExpirationThreshold);
EXPECT_EQ(on_bound_session_params_changed_call_count(), 0u);
EXPECT_EQ(on_bound_session_throttler_params_changed_call_count(), 0u);

// Update with null time should change the minimum expiration date and
// trigger a notification.
SetExpirationTimeAndNotify(k1PSIDTSCookieName, base::Time());
EXPECT_EQ(on_bound_session_params_changed_call_count(), 1u);
EXPECT_EQ(on_bound_session_throttler_params_changed_call_count(), 1u);
EXPECT_EQ(cookie_expiration_time(k1PSIDTSCookieName), base::Time());
EXPECT_EQ(bound_session_cookie_controller()->min_cookie_expiration_time(),
base::Time());
}

TEST_F(BoundSessionCookieControllerImplTest, CookieChange) {
CompletePendingRefreshRequestIfAny();
ResetOnBoundSessionParamsChangedCallCount();
ResetOnBoundSessionThrottlerParamsChangedCallCount();
task_environment()->FastForwardBy(base::Minutes(2));

BoundSessionCookieController* controller = bound_session_cookie_controller();
Expand All @@ -356,7 +356,7 @@ TEST_F(BoundSessionCookieControllerImplTest, CookieChange) {
// The new `expiration_time_1PSIDTS` is larger than the other cookie
// expiration time so the minimum remains unchanged.
EXPECT_EQ(controller->min_cookie_expiration_time(), minimum_expiration_time);
EXPECT_EQ(on_bound_session_params_changed_call_count(), 0u);
EXPECT_EQ(on_bound_session_throttler_params_changed_call_count(), 0u);

task_environment()->FastForwardBy(base::Minutes(2));
// Simulate cookie change of 2nd cookie.
Expand All @@ -366,7 +366,7 @@ TEST_F(BoundSessionCookieControllerImplTest, CookieChange) {
// Expiration time of: `k3PSIDTSCookieName` > `k1PSIDTSCookieName`.
// The minimum changes to the expiration date of `k1PSIDTSCookieName`.
EXPECT_EQ(controller->min_cookie_expiration_time(), expiration_time_1PSIDTS);
EXPECT_EQ(on_bound_session_params_changed_call_count(), 1u);
EXPECT_EQ(on_bound_session_throttler_params_changed_call_count(), 1u);
}

TEST_F(BoundSessionCookieControllerImplTest,
Expand Down Expand Up @@ -508,7 +508,7 @@ TEST_F(BoundSessionCookieControllerImplTest,
RequestBlockedOnCookieMultipleRequests) {
ASSERT_FALSE(IsConnectionTypeAvailableAndOffline());
CompletePendingRefreshRequestIfAny();
ResetOnBoundSessionParamsChangedCallCount();
ResetOnBoundSessionThrottlerParamsChangedCallCount();
// Cookie stale.
task_environment()->FastForwardBy(base::Minutes(12));

Expand All @@ -526,7 +526,7 @@ TEST_F(BoundSessionCookieControllerImplTest,
for (auto& future : futures) {
EXPECT_TRUE(future.IsReady());
}
EXPECT_EQ(on_bound_session_params_changed_call_count(), 1u);
EXPECT_EQ(on_bound_session_throttler_params_changed_call_count(), 1u);
EXPECT_TRUE(AreAllCookiesFresh());
}

Expand Down Expand Up @@ -697,9 +697,9 @@ TEST_F(BoundSessionCookieControllerImplTest,

TEST_F(BoundSessionCookieControllerImplTest,
ScheduleCookieRotationOnSetCookieExpiration) {
ResetOnBoundSessionParamsChangedCallCount();
ResetOnBoundSessionThrottlerParamsChangedCallCount();
EXPECT_TRUE(CompletePendingRefreshRequestIfAny());
EXPECT_EQ(on_bound_session_params_changed_call_count(), 1u);
EXPECT_EQ(on_bound_session_throttler_params_changed_call_count(), 1u);
EXPECT_TRUE(cookie_refresh_timer()->IsRunning());
base::TimeDelta expected_refresh_delay =
bound_session_cookie_controller()->min_cookie_expiration_time() -
Expand Down Expand Up @@ -749,10 +749,10 @@ TEST_F(BoundSessionCookieControllerImplTest,
TEST_F(BoundSessionCookieControllerImplTest,
RefreshCookieImmediatelyOnSetCookieExpirationBelowRefreshInterval) {
EXPECT_TRUE(CompletePendingRefreshRequestIfAny());
ResetOnBoundSessionParamsChangedCallCount();
ResetOnBoundSessionThrottlerParamsChangedCallCount();
SetExpirationTimeAndNotify(k1PSIDTSCookieName,
base::Time::Now() + kCookieRefreshInterval / 2);
EXPECT_EQ(on_bound_session_params_changed_call_count(), 1u);
EXPECT_EQ(on_bound_session_throttler_params_changed_call_count(), 1u);
EXPECT_FALSE(cookie_refresh_timer()->IsRunning());
EXPECT_TRUE(cookie_fetcher());
CompletePendingRefreshRequestIfAny();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ class BoundSessionCookieRefreshService
: public KeyedService,
public chrome::mojom::BoundSessionRequestThrottledListener {
public:
using RendererBoundSessionParamsUpdaterDelegate = base::RepeatingClosure;
using RendererBoundSessionThrottlerParamsUpdaterDelegate =
base::RepeatingClosure;

BoundSessionCookieRefreshService() = default;

Expand All @@ -46,8 +47,8 @@ class BoundSessionCookieRefreshService
const net::HttpResponseHeaders* headers) = 0;

// Returns bound session params.
virtual chrome::mojom::BoundSessionParamsPtr GetBoundSessionParams()
const = 0;
virtual chrome::mojom::BoundSessionThrottlerParamsPtr
GetBoundSessionThrottlerParams() const = 0;

virtual void CreateRegistrationRequest(
BoundSessionRegistrationFetcherParam registration_params) = 0;
Expand All @@ -58,9 +59,10 @@ class BoundSessionCookieRefreshService
friend class RendererUpdater;

// `RendererUpdater` class that is responsible for pushing updates to all
// renderers calls this setter to subscribe for bound session params updates.
virtual void SetRendererBoundSessionParamsUpdaterDelegate(
RendererBoundSessionParamsUpdaterDelegate renderer_updater) = 0;
// renderers calls this setter to subscribe for bound session throttler params
// updates.
virtual void SetRendererBoundSessionThrottlerParamsUpdaterDelegate(
RendererBoundSessionThrottlerParamsUpdaterDelegate renderer_updater) = 0;

// Adds a Receiver to `BoundSessionCookieRefreshService` to receive
// notification when a request is throttled and requires a fresh cookie.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,18 +81,18 @@ void BoundSessionCookieRefreshServiceImpl::MaybeTerminateSession(
}
}

chrome::mojom::BoundSessionParamsPtr
BoundSessionCookieRefreshServiceImpl::GetBoundSessionParams() const {
chrome::mojom::BoundSessionThrottlerParamsPtr
BoundSessionCookieRefreshServiceImpl::GetBoundSessionThrottlerParams() const {
if (!cookie_controller_) {
return chrome::mojom::BoundSessionParamsPtr();
return chrome::mojom::BoundSessionThrottlerParamsPtr();
}

return cookie_controller_->bound_session_params();
return cookie_controller_->bound_session_throttler_params();
}

void BoundSessionCookieRefreshServiceImpl::
SetRendererBoundSessionParamsUpdaterDelegate(
RendererBoundSessionParamsUpdaterDelegate renderer_updater) {
SetRendererBoundSessionThrottlerParamsUpdaterDelegate(
RendererBoundSessionThrottlerParamsUpdaterDelegate renderer_updater) {
renderer_updater_ = renderer_updater;
}

Expand Down Expand Up @@ -190,7 +190,8 @@ BoundSessionCookieRefreshServiceImpl::GetRegistrationParams() {
return absl::nullopt;
}

void BoundSessionCookieRefreshServiceImpl::OnBoundSessionParamsChanged() {
void BoundSessionCookieRefreshServiceImpl::
OnBoundSessionThrottlerParamsChanged() {
UpdateAllRenderers();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ class BoundSessionCookieRefreshServiceImpl
void RegisterNewBoundSession(
const bound_session_credentials::RegistrationParams& params) override;
void MaybeTerminateSession(const net::HttpResponseHeaders* headers) override;
chrome::mojom::BoundSessionParamsPtr GetBoundSessionParams() const override;
chrome::mojom::BoundSessionThrottlerParamsPtr GetBoundSessionThrottlerParams()
const override;
void AddBoundSessionRequestThrottledListenerReceiver(
mojo::PendingReceiver<chrome::mojom::BoundSessionRequestThrottledListener>
receiver) override;
Expand Down Expand Up @@ -83,8 +84,9 @@ class BoundSessionCookieRefreshServiceImpl
Delegate* delegate)>;

// BoundSessionCookieRefreshService:
void SetRendererBoundSessionParamsUpdaterDelegate(
RendererBoundSessionParamsUpdaterDelegate renderer_updater) override;
void SetRendererBoundSessionThrottlerParamsUpdaterDelegate(
RendererBoundSessionThrottlerParamsUpdaterDelegate renderer_updater)
override;

void set_controller_factory_for_testing(
const BoundSessionCookieControllerFactoryForTesting&
Expand All @@ -103,7 +105,7 @@ class BoundSessionCookieRefreshServiceImpl
GetRegistrationParams();

// BoundSessionCookieController::Delegate
void OnBoundSessionParamsChanged() override;
void OnBoundSessionThrottlerParamsChanged() override;
void TerminateSession() override;

std::unique_ptr<BoundSessionCookieController>
Expand All @@ -120,7 +122,7 @@ class BoundSessionCookieRefreshServiceImpl
const raw_ptr<content::StoragePartition> storage_partition_;
const raw_ptr<network::NetworkConnectionTracker> network_connection_tracker_;
BoundSessionCookieControllerFactoryForTesting controller_factory_for_testing_;
RendererBoundSessionParamsUpdaterDelegate renderer_updater_;
RendererBoundSessionThrottlerParamsUpdaterDelegate renderer_updater_;

std::unique_ptr<BoundSessionCookieController> cookie_controller_;

Expand Down

0 comments on commit 1764a40

Please sign in to comment.