[DBSC] Add API to prototype to stop cookies rotations
It adds `BoundSessionCookieController::StopCookiesRotation` to stop
cookies rotation for the given controller and
`BoundSessionCookieRefreshService::StopCookiesRotation` to stop cookies
rotations for a given session.
This API will be used by `BoundSessionOAuthMultiLoginDelegate` to stop
cookies rotations until the cookies are set (to avoid undesired race
conditions) - implemented in next CL(s).
Bug: 312719798
Change-Id: I13caaa145f44d00e7cd11077e350551ccddb4c3b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6832201
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Commit-Queue: Ernest Nguyen Hung <ernn@google.com>
Cr-Commit-Position: refs/heads/main@{#1503833}
diff --git a/chrome/browser/signin/bound_session_credentials/bound_session_cookie_controller.h b/chrome/browser/signin/bound_session_credentials/bound_session_cookie_controller.h
index 00284ad..891c34c 100644
--- a/chrome/browser/signin/bound_session_credentials/bound_session_cookie_controller.h
+++ b/chrome/browser/signin/bound_session_credentials/bound_session_cookie_controller.h
@@ -50,6 +50,12 @@
// cookie expiration date changes. Cookie deletion is considered as a change
// in the expiration date to the null time.
virtual void OnBoundSessionThrottlerParamsChanged() = 0;
+
+ // Called when the cookie rotation has been stopped for more than a timeout
+ // period. `BoundSessionCookieController` is expected to be deleted after
+ // this call.
+ virtual void OnCookieRotationStoppedTimeout(
+ BoundSessionCookieController* controller) = 0;
};
BoundSessionCookieController(
@@ -68,6 +74,17 @@
chrome::mojom::BoundSessionRequestThrottledHandler::
HandleRequestBlockedOnCookieCallback resume_blocked_request) = 0;
+ // Stops the cookie rotation for the given session.
+ //
+ // Once the cookie rotation is stopped, all throttled requests will remain
+ // throttled until the session is terminated. This is used by OAML to ensure
+ // all requests are throttled until the returned cookies are set.
+ //
+ // The session will be terminated after a timeout if it has not been
+ // terminated explicitly. This is a safety net to ensure the session is
+ // eventually terminated even if OAML fails to terminate the session.
+ virtual void StopCookieRotation() = 0;
+
// URL determining the scope of the bound session. All requests that are
// within the scope are subject to throttling.
const GURL& scope_url() const { return scope_url_; }
diff --git a/chrome/browser/signin/bound_session_credentials/bound_session_cookie_controller_impl.cc b/chrome/browser/signin/bound_session_credentials/bound_session_cookie_controller_impl.cc
index b639e4b..458e4135 100644
--- a/chrome/browser/signin/bound_session_credentials/bound_session_cookie_controller_impl.cc
+++ b/chrome/browser/signin/bound_session_credentials/bound_session_cookie_controller_impl.cc
@@ -23,6 +23,7 @@
#include "chrome/browser/signin/bound_session_credentials/rotation_debug_info.pb.h"
#include "chrome/browser/signin/bound_session_credentials/session_binding_helper.h"
#include "chrome/common/renderer_configuration.mojom.h"
+#include "components/signin/public/base/signin_switches.h"
#include "content/public/browser/storage_partition.h"
#include "net/base/backoff_entry.h"
#include "services/network/public/cpp/network_connection_tracker.h"
@@ -62,6 +63,8 @@
false,
};
+constexpr base::TimeDelta kResumeBlockedRequestsTimeout = base::Seconds(20);
+
void RecordNumberOfSuccessiveTimeoutIfAny(size_t successive_timeout) {
if (successive_timeout == 0) {
return;
@@ -209,24 +212,36 @@
resume_blocked_requests_.push_back(std::move(resume_blocked_request));
MaybeRefreshCookie(
BoundSessionRefreshCookieFetcher::Trigger::kBlockedRequest);
+ MaybeStartResumeBlockedRequestsTimer();
+}
- if (!resume_blocked_requests_timer_.IsRunning() &&
- !resume_blocked_requests_.empty()) {
- // Ensure all blocked requests are released after a timeout.
- // `base::Unretained(this)` is safe because `this` owns
- // `resume_blocked_requests_timer_`.
- const base::TimeDelta kResumeBlockedRequestTimeout = base::Seconds(20);
- resume_blocked_requests_timer_.Start(
- FROM_HERE, kResumeBlockedRequestTimeout,
- base::BindRepeating(
- &BoundSessionCookieControllerImpl::OnResumeBlockedRequestsTimeout,
- base::Unretained(this)));
- }
+void BoundSessionCookieControllerImpl::StopCookieRotation() {
+ CHECK(base::FeatureList::IsEnabled(
+ switches::kEnableOAuthMultiloginCookiesBinding));
+ rotation_stopped_ = true;
+ // Stop the cookie rotation.
+ cookie_refresh_timer_.Stop();
+ // Cancel any pending rotation request.
+ refresh_cookie_fetcher_.reset();
+ // Make sure that all blocked requests remain blocked until the session is
+ // terminated.
+ resume_blocked_requests_timer_.Stop();
+ // Start a timer to make sure the session is eventually terminated (reusing
+ // the same timeout period as for resuming blocked requests).
+ //
+ // `base::Unretained` is safe because:
+ // - it is guaranteed that `delegate_` outlives `this`,
+ // - `this` owns `rotation_stopped_timer_`.
+ rotation_stopped_timer_.Start(
+ FROM_HERE, kResumeBlockedRequestsTimeout,
+ base::BindOnce(&BoundSessionCookieController::Delegate::
+ OnCookieRotationStoppedTimeout,
+ base::Unretained(delegate_), base::Unretained(this)));
}
bool BoundSessionCookieControllerImpl::ShouldPauseThrottlingRequests() const {
- return refresh_cookie_fetcher_backoff_.failure_count() >
- kNumberOfErrorsToIgnoreForBackoff;
+ return !rotation_stopped_ && refresh_cookie_fetcher_backoff_.failure_count() >
+ kNumberOfErrorsToIgnoreForBackoff;
}
bound_session_credentials::RotationDebugInfo
@@ -302,7 +317,7 @@
}
bool BoundSessionCookieControllerImpl::CanCreateRefreshCookieFetcher() const {
- return !refresh_cookie_fetcher_ &&
+ return !rotation_stopped_ && !refresh_cookie_fetcher_ &&
!refresh_cookie_fetcher_backoff_.ShouldRejectRequest();
}
@@ -464,6 +479,9 @@
void BoundSessionCookieControllerImpl::MaybeScheduleCookieRotation(
BoundSessionRefreshCookieFetcher::Trigger trigger) {
+ if (rotation_stopped_) {
+ return;
+ }
const base::TimeDelta kCookieRefreshInterval = base::Minutes(2);
base::TimeDelta preemptive_refresh_in =
min_cookie_expiration_time() - base::Time::Now() - kCookieRefreshInterval;
@@ -494,6 +512,21 @@
base::Unretained(this), trigger));
}
+void BoundSessionCookieControllerImpl::MaybeStartResumeBlockedRequestsTimer() {
+ if (rotation_stopped_ || resume_blocked_requests_timer_.IsRunning() ||
+ resume_blocked_requests_.empty()) {
+ return;
+ }
+ // Ensure all blocked requests are released after a timeout.
+ // `base::Unretained(this)` is safe because `this` owns
+ // `resume_blocked_requests_timer_`.
+ resume_blocked_requests_timer_.Start(
+ FROM_HERE, kResumeBlockedRequestsTimeout,
+ base::BindRepeating(
+ &BoundSessionCookieControllerImpl::OnResumeBlockedRequestsTimeout,
+ base::Unretained(this)));
+}
+
void BoundSessionCookieControllerImpl::ResumeBlockedRequests(
ResumeBlockedRequestsTrigger trigger) {
resume_blocked_requests_timer_.Stop();
diff --git a/chrome/browser/signin/bound_session_credentials/bound_session_cookie_controller_impl.h b/chrome/browser/signin/bound_session_credentials/bound_session_cookie_controller_impl.h
index c91b5ba..5576326 100644
--- a/chrome/browser/signin/bound_session_credentials/bound_session_cookie_controller_impl.h
+++ b/chrome/browser/signin/bound_session_credentials/bound_session_cookie_controller_impl.h
@@ -56,6 +56,7 @@
void HandleRequestBlockedOnCookie(
chrome::mojom::BoundSessionRequestThrottledHandler::
HandleRequestBlockedOnCookieCallback resume_blocked_request) override;
+ void StopCookieRotation() override;
bool ShouldPauseThrottlingRequests() const override;
bound_session_credentials::RotationDebugInfo TakeDebugInfo() override;
@@ -91,6 +92,7 @@
void ResetCookieFetcherBackoff();
void MaybeScheduleCookieRotation(
BoundSessionRefreshCookieFetcher::Trigger trigger);
+ void MaybeStartResumeBlockedRequestsTimer();
void ResumeBlockedRequests(
chrome::mojom::ResumeBlockedRequestsTrigger trigger);
void OnResumeBlockedRequestsTimeout();
@@ -140,6 +142,15 @@
base::OneShotTimer resume_blocked_requests_timer_;
size_t successive_timeout_ = 0;
+ // Once set to `true`, the cookies rotation is stopped and all requests are
+ // throttled until the session is terminated (i.e. this field is never flipped
+ // back to `false`).
+ bool rotation_stopped_ = false;
+
+ // Used to call `OnCookieRotationStoppedTimeout` after the session has been
+ // stopped for more than a timeout threshold.
+ base::OneShotTimer rotation_stopped_timer_;
+
RefreshCookieFetcherFactoryForTesting
refresh_cookie_fetcher_factory_for_testing_;
diff --git a/chrome/browser/signin/bound_session_credentials/bound_session_cookie_controller_impl_unittest.cc b/chrome/browser/signin/bound_session_credentials/bound_session_cookie_controller_impl_unittest.cc
index 7489dea..27d69c2 100644
--- a/chrome/browser/signin/bound_session_credentials/bound_session_cookie_controller_impl_unittest.cc
+++ b/chrome/browser/signin/bound_session_credentials/bound_session_cookie_controller_impl_unittest.cc
@@ -12,6 +12,7 @@
#include "base/functional/callback_helpers.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/protobuf_matchers.h"
+#include "base/test/scoped_feature_list.h"
#include "base/test/task_environment.h"
#include "base/test/test_future.h"
#include "base/time/time.h"
@@ -26,6 +27,7 @@
#include "chrome/browser/signin/bound_session_credentials/rotation_debug_info.pb.h"
#include "chrome/browser/signin/bound_session_credentials/session_binding_helper.h"
#include "chrome/common/renderer_configuration.mojom-shared.h"
+#include "components/signin/public/base/signin_switches.h"
#include "components/unexportable_keys/service_error.h"
#include "components/unexportable_keys/unexportable_key_id.h"
#include "components/unexportable_keys/unexportable_key_loader.h"
@@ -239,6 +241,11 @@
on_persistent_error_encountered_refresh_error_ = refresh_error;
}
+ void OnCookieRotationStoppedTimeout(
+ BoundSessionCookieController* controller) override {
+ on_cookie_rotation_stopped_timeout_called_ = true;
+ }
+
void SetExpirationTimeAndNotify(const std::string& cookie_name,
const base::Time& expiration_time) {
bound_session_cookie_controller_->SetCookieExpirationTimeAndNotify(
@@ -316,6 +323,10 @@
return on_persistent_error_encountered_refresh_error_;
}
+ bool on_cookie_rotation_stopped_timeout_called() {
+ return on_cookie_rotation_stopped_timeout_called_;
+ }
+
void ResetOnBoundSessionThrottlerParamsChangedCallCount() {
on_bound_session_throttler_params_changed_call_count_ = 0;
}
@@ -405,6 +416,7 @@
bound_session_cookie_controller_;
size_t on_bound_session_throttler_params_changed_call_count_ = 0;
std::optional<Result> on_persistent_error_encountered_refresh_error_;
+ bool on_cookie_rotation_stopped_timeout_called_ = false;
};
TEST_F(BoundSessionCookieControllerImplTest, KeyLoadedOnStartup) {
@@ -1545,3 +1557,122 @@
/*periodic=*/false,
kNumberOfErrorsToIgnoreForBackoff + 1 + kFailedAttempts);
}
+
+TEST_F(BoundSessionCookieControllerImplTest,
+ StopCookieRotationCancelsPendingRequests) {
+ base::test::ScopedFeatureList feature_list;
+ feature_list.InitAndEnableFeature(
+ switches::kEnableOAuthMultiloginCookiesBinding);
+
+ ASSERT_EQ(cookie_fetcher_trigger(),
+ BoundSessionRefreshCookieFetcher::Trigger::kStartup);
+
+ bound_session_cookie_controller()->StopCookieRotation();
+
+ EXPECT_FALSE(CompletePendingRefreshRequestIfAny());
+ base::test::TestFuture<ResumeBlockedRequestsTrigger> future;
+ bound_session_cookie_controller()->HandleRequestBlockedOnCookie(
+ future.GetCallback());
+ // The request is not resumed.
+ EXPECT_FALSE(future.IsReady());
+}
+
+TEST_F(BoundSessionCookieControllerImplTest,
+ StopCookieRotationBlocksNewRequests) {
+ base::test::ScopedFeatureList feature_list;
+ feature_list.InitAndEnableFeature(
+ switches::kEnableOAuthMultiloginCookiesBinding);
+
+ bound_session_cookie_controller()->StopCookieRotation();
+
+ base::test::TestFuture<ResumeBlockedRequestsTrigger> future;
+ bound_session_cookie_controller()->HandleRequestBlockedOnCookie(
+ future.GetCallback());
+ task_environment()->FastForwardBy(kResumeBlockedRequestTimeout);
+
+ EXPECT_FALSE(future.IsReady());
+}
+
+TEST_F(BoundSessionCookieControllerImplTest,
+ StopCookieRotationKeepsBlockingAlreadyBlockedRequests) {
+ base::test::ScopedFeatureList feature_list;
+ feature_list.InitAndEnableFeature(
+ switches::kEnableOAuthMultiloginCookiesBinding);
+
+ base::test::TestFuture<ResumeBlockedRequestsTrigger> future;
+ bound_session_cookie_controller()->HandleRequestBlockedOnCookie(
+ future.GetCallback());
+
+ // Verify that the request is blocked and the resume timer is running.
+ ASSERT_FALSE(future.IsReady());
+ base::OneShotTimer* resume_requests_timer = resume_blocked_requests_timer();
+ ASSERT_NE(resume_requests_timer, nullptr);
+ ASSERT_TRUE(resume_requests_timer->IsRunning());
+
+ bound_session_cookie_controller()->StopCookieRotation();
+
+ // The request is still blocked (even after the resume timer timeout) and the
+ // resume timer is stopped.
+ task_environment()->FastForwardBy(kResumeBlockedRequestTimeout);
+ EXPECT_FALSE(future.IsReady());
+ EXPECT_FALSE(resume_requests_timer->IsRunning());
+}
+
+TEST_F(BoundSessionCookieControllerImplTest,
+ StopCookieRotationStopsCookieRotationTimer) {
+ base::test::ScopedFeatureList feature_list;
+ feature_list.InitAndEnableFeature(
+ switches::kEnableOAuthMultiloginCookiesBinding);
+
+ CompletePendingRefreshRequestIfAny();
+ ASSERT_TRUE(cookie_refresh_timer()->IsRunning());
+
+ bound_session_cookie_controller()->StopCookieRotation();
+
+ EXPECT_FALSE(cookie_refresh_timer()->IsRunning());
+}
+
+TEST_F(BoundSessionCookieControllerImplTest,
+ StopCookieRotationsPreventsFromSchedulingCookieRotation) {
+ base::test::ScopedFeatureList feature_list;
+ feature_list.InitAndEnableFeature(
+ switches::kEnableOAuthMultiloginCookiesBinding);
+
+ bound_session_cookie_controller()->StopCookieRotation();
+
+ // Setting up an offline state and then online state to trigger on connection
+ // changed.
+ SetConnectionType(network::mojom::ConnectionType::CONNECTION_NONE);
+
+ // Cookie rotation should NOT start if the cookie rotation is stopped.
+ SetConnectionType(network::mojom::ConnectionType::CONNECTION_5G);
+ EXPECT_EQ(cookie_fetcher(), nullptr);
+}
+
+TEST_F(BoundSessionCookieControllerImplTest,
+ StopCookieRotationPreventsFromPausingThrottling) {
+ base::test::ScopedFeatureList feature_list;
+ feature_list.InitAndEnableFeature(
+ switches::kEnableOAuthMultiloginCookiesBinding);
+
+ TriggerThrottlingPausedAndVerify();
+
+ bound_session_cookie_controller()->StopCookieRotation();
+
+ EXPECT_FALSE(
+ bound_session_cookie_controller()->ShouldPauseThrottlingRequests());
+}
+
+TEST_F(BoundSessionCookieControllerImplTest, StopCookieRotationTimeout) {
+ base::test::ScopedFeatureList feature_list;
+ feature_list.InitAndEnableFeature(
+ switches::kEnableOAuthMultiloginCookiesBinding);
+
+ ASSERT_FALSE(on_cookie_rotation_stopped_timeout_called());
+
+ bound_session_cookie_controller()->StopCookieRotation();
+
+ EXPECT_FALSE(on_cookie_rotation_stopped_timeout_called());
+ task_environment()->FastForwardBy(kResumeBlockedRequestTimeout);
+ EXPECT_TRUE(on_cookie_rotation_stopped_timeout_called());
+}
diff --git a/chrome/browser/signin/bound_session_credentials/bound_session_cookie_refresh_service.h b/chrome/browser/signin/bound_session_credentials/bound_session_cookie_refresh_service.h
index 35d38073..0e18b823 100644
--- a/chrome/browser/signin/bound_session_credentials/bound_session_cookie_refresh_service.h
+++ b/chrome/browser/signin/bound_session_credentials/bound_session_cookie_refresh_service.h
@@ -19,6 +19,7 @@
#include "mojo/public/cpp/bindings/pending_receiver.h"
struct BoundSessionDebugInfo;
+struct BoundSessionKey;
// BoundSessionCookieRefreshService is responsible for maintaining cookies
// associated with bound sessions. This class does the following:
@@ -73,6 +74,19 @@
virtual void CreateRegistrationRequest(
BoundSessionRegistrationFetcherParam registration_params) = 0;
+ // Stops the cookie rotation for the given session. This is a no-op if the
+ // given session does not exist.
+ //
+ // Once the cookie rotation is stopped, all throttled requests will remain
+ // throttled until the session is terminated.
+ // This is used by OAML to ensure all requests are throttled until the
+ // returned cookies are set.
+ //
+ // The session will be terminated after a timeout if it has not been
+ // terminated explicitly. This is a safety net to ensure the session is
+ // eventually terminated even if OAML fails to terminate the session.
+ virtual void StopCookieRotation(const BoundSessionKey& key) = 0;
+
virtual base::WeakPtr<BoundSessionCookieRefreshService> GetWeakPtr() = 0;
virtual void AddObserver(Observer* observer) = 0;
diff --git a/chrome/browser/signin/bound_session_credentials/bound_session_cookie_refresh_service_impl.cc b/chrome/browser/signin/bound_session_credentials/bound_session_cookie_refresh_service_impl.cc
index b084d13e..7a680e3 100644
--- a/chrome/browser/signin/bound_session_credentials/bound_session_cookie_refresh_service_impl.cc
+++ b/chrome/browser/signin/bound_session_credentials/bound_session_cookie_refresh_service_impl.cc
@@ -121,6 +121,8 @@
: RotationDebugInfo::TERMINATION_REASON_OTHER;
case kSessionOverride:
return RotationDebugInfo::SESSION_OVERRIDE;
+ case kRotationStoppedTimeout:
+ return RotationDebugInfo::ROTATION_STOPPED_TIMEOUT;
case kCookiesCleared:
// `kCookiesCleared` should not be reported in the debug header.
NOTREACHED();
@@ -407,6 +409,15 @@
}
}
+void BoundSessionCookieRefreshServiceImpl::StopCookieRotation(
+ const BoundSessionKey& key) {
+ auto controller_it = cookie_controllers_.find(key);
+ if (controller_it == cookie_controllers_.end()) {
+ return;
+ }
+ controller_it->second->StopCookieRotation();
+}
+
base::WeakPtr<BoundSessionCookieRefreshService>
BoundSessionCookieRefreshServiceImpl::GetWeakPtr() {
return weak_ptr_factory_.GetWeakPtr();
@@ -466,6 +477,12 @@
UpdateAllRenderers();
}
+void BoundSessionCookieRefreshServiceImpl::OnCookieRotationStoppedTimeout(
+ BoundSessionCookieController* controller) {
+ TerminateSession(controller,
+ SessionTerminationTrigger::kRotationStoppedTimeout);
+}
+
void BoundSessionCookieRefreshServiceImpl::OnPersistentErrorEncountered(
BoundSessionCookieController* controller,
BoundSessionRefreshCookieFetcher::Result refresh_error) {
@@ -599,8 +616,9 @@
SessionTerminationTrigger trigger,
std::optional<BoundSessionRefreshCookieFetcher::Result> refresh_error) {
if (trigger == SessionTerminationTrigger::kCookiesCleared) {
- // Do not send the debug report if cookies were cleared as the request won't
- // be attributed to a user in any case.
+ // Do not send the debug report if cookies were cleared or the rotation was
+ // terminated due to a stopping timeout as the request won't be attributed
+ // to a user in any case.
return;
}
diff --git a/chrome/browser/signin/bound_session_credentials/bound_session_cookie_refresh_service_impl.h b/chrome/browser/signin/bound_session_credentials/bound_session_cookie_refresh_service_impl.h
index f7aaae7..ba8ea15 100644
--- a/chrome/browser/signin/bound_session_credentials/bound_session_cookie_refresh_service_impl.h
+++ b/chrome/browser/signin/bound_session_credentials/bound_session_cookie_refresh_service_impl.h
@@ -61,7 +61,8 @@
kCookiesCleared = 1,
kSessionTerminationHeader = 2,
kSessionOverride = 3,
- kMaxValue = kSessionOverride,
+ kRotationStoppedTimeout = 4,
+ kMaxValue = kRotationStoppedTimeout,
};
BoundSessionCookieRefreshServiceImpl(
@@ -87,6 +88,7 @@
receiver) override;
void CreateRegistrationRequest(
BoundSessionRegistrationFetcherParam registration_params) override;
+ void StopCookieRotation(const BoundSessionKey& key) override;
base::WeakPtr<BoundSessionCookieRefreshService> GetWeakPtr() override;
void AddObserver(
BoundSessionCookieRefreshService::Observer* observer) override;
@@ -160,6 +162,8 @@
void OnPersistentErrorEncountered(
BoundSessionCookieController* controller,
BoundSessionRefreshCookieFetcher::Result refresh_error) override;
+ void OnCookieRotationStoppedTimeout(
+ BoundSessionCookieController* controller) override;
// StoragePartition::DataRemovalObserver:
void OnStorageKeyDataCleared(
diff --git a/chrome/browser/signin/bound_session_credentials/bound_session_cookie_refresh_service_impl_unittest.cc b/chrome/browser/signin/bound_session_credentials/bound_session_cookie_refresh_service_impl_unittest.cc
index 63a6777..52f50ae 100644
--- a/chrome/browser/signin/bound_session_credentials/bound_session_cookie_refresh_service_impl_unittest.cc
+++ b/chrome/browser/signin/bound_session_credentials/bound_session_cookie_refresh_service_impl_unittest.cc
@@ -142,6 +142,10 @@
resume_blocked_requests_.push_back(std::move(resume_blocked_request));
}
+ bool rotation_stopped() const { return rotation_stopped_; }
+
+ void StopCookieRotation() override { rotation_stopped_ = true; }
+
bound_session_credentials::RotationDebugInfo TakeDebugInfo() override {
return {};
}
@@ -169,6 +173,10 @@
this, BoundSessionRefreshCookieFetcher::Result::kServerPersistentError);
}
+ void SimulateOnCookieRotationStoppedTimeout() {
+ delegate_->OnCookieRotationStoppedTimeout(this);
+ }
+
void SimulateRefreshBoundSessionCompleted() {
EXPECT_FALSE(resume_blocked_requests_.empty());
std::vector<chrome::mojom::BoundSessionRequestThrottledHandler::
@@ -191,6 +199,7 @@
std::vector<uint8_t> wrapped_key_;
signin::Tribool was_new_session_at_init_ = signin::Tribool::kUnknown;
bool throttling_requests_paused_ = false;
+ bool rotation_stopped_ = false;
base::WeakPtrFactory<FakeBoundSessionCookieController> weak_ptr_factory_{
this};
};
@@ -405,7 +414,9 @@
bound_session_credentials::RotationDebugInfo debug_info),
());
- BoundSessionCookieRefreshServiceImpl* GetCookieRefreshServiceImpl() {
+ // Returns the cookie refresh service if it already exists, or creates a new
+ // one.
+ BoundSessionCookieRefreshServiceImpl* GetOrCreateCookieRefreshServiceImpl() {
if (!cookie_refresh_service_) {
cookie_refresh_service_ = CreateBoundSessionCookieRefreshServiceImpl();
}
@@ -464,6 +475,10 @@
MockObserver* mock_observer() { return &mock_observer_; }
+ void FastForwardBy(base::TimeDelta delta) {
+ task_environment_.FastForwardBy(delta);
+ }
+
void RunUntilIdle() { task_environment_.RunUntilIdle(); }
BoundSessionRegistrationFetcherParam CreateTestRegistrationFetcherParams(
@@ -603,7 +618,7 @@
TEST_F(BoundSessionCookieRefreshServiceImplTest, VerifyControllerParams) {
SetupPreConditionForBoundSession();
- GetCookieRefreshServiceImpl();
+ GetOrCreateCookieRefreshServiceImpl();
VerifyBoundSession(CreateTestBoundSessionParams());
EXPECT_EQ(cookie_controller()->was_new_session_at_init(),
signin::Tribool::kFalse);
@@ -611,14 +626,15 @@
TEST_F(BoundSessionCookieRefreshServiceImplTest,
VerifyBoundSessionThrottlerParamsUnboundSession) {
- GetCookieRefreshServiceImpl();
+ GetOrCreateCookieRefreshServiceImpl();
VerifyNoBoundSession();
}
TEST_F(BoundSessionCookieRefreshServiceImplTest,
VerifyBoundSessionThrottlerParamsBoundSession) {
SetupPreConditionForBoundSession();
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
ASSERT_TRUE(cookie_controller());
std::vector<chrome::mojom::BoundSessionThrottlerParamsPtr>
@@ -640,14 +656,15 @@
CreateCookieCredential("cookieB", GURL("https://accounts.google.com"));
ASSERT_TRUE(storage()->SaveParams(params));
- GetCookieRefreshServiceImpl();
+ GetOrCreateCookieRefreshServiceImpl();
VerifyBoundSession(params);
}
TEST_F(BoundSessionCookieRefreshServiceImplTest,
RefreshBoundSessionCookieBoundSession) {
SetupPreConditionForBoundSession();
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
EXPECT_TRUE(cookie_controller());
base::test::TestFuture<ResumeBlockedRequestsTrigger> future;
service->HandleRequestBlockedOnCookie(kTestGoogleURL, future.GetCallback());
@@ -661,7 +678,8 @@
TEST_F(BoundSessionCookieRefreshServiceImplTest,
RequestBlockedOnCookieThrottlingPaused) {
SetupPreConditionForBoundSession();
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
EXPECT_TRUE(cookie_controller());
cookie_controller()->SetThrottlingRequestsPaused(true);
base::test::TestFuture<ResumeBlockedRequestsTrigger> future;
@@ -674,7 +692,8 @@
TEST_F(BoundSessionCookieRefreshServiceImplTest,
RefreshBoundSessionCookieUnboundSession) {
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
EXPECT_FALSE(cookie_controller());
// Unbound session, the callback should be called immediately.
@@ -687,7 +706,8 @@
TEST_F(BoundSessionCookieRefreshServiceImplTest,
UpdateAllRenderersOnBoundSessionStarted) {
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
EXPECT_FALSE(cookie_controller());
EXPECT_THAT(service->GetBoundSessionThrottlerParams(), IsEmpty());
base::MockRepeatingCallback<void()> renderer_updater;
@@ -709,7 +729,8 @@
base::MockRepeatingCallback<void()> renderer_updater;
EXPECT_CALL(renderer_updater, Run()).Times(0);
SetupPreConditionForBoundSession();
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
EXPECT_TRUE(cookie_controller());
SetRendererUpdater(renderer_updater.Get());
testing::Mock::VerifyAndClearExpectations(&renderer_updater);
@@ -733,7 +754,7 @@
base::MockRepeatingCallback<void()> renderer_updater;
EXPECT_CALL(renderer_updater, Run()).Times(0);
SetupPreConditionForBoundSession();
- GetCookieRefreshServiceImpl();
+ GetOrCreateCookieRefreshServiceImpl();
EXPECT_TRUE(cookie_controller());
SetRendererUpdater(renderer_updater.Get());
testing::Mock::VerifyAndClearExpectations(&renderer_updater);
@@ -755,7 +776,7 @@
TEST_F(BoundSessionCookieRefreshServiceImplTest,
SendDebugReportOnBoundSessionTerminated) {
SetupPreConditionForBoundSession();
- GetCookieRefreshServiceImpl();
+ GetOrCreateCookieRefreshServiceImpl();
EXPECT_TRUE(cookie_controller());
EXPECT_CALL(
@@ -783,7 +804,8 @@
TEST_F(BoundSessionCookieRefreshServiceImplTest, TerminateSession) {
SetupPreConditionForBoundSession();
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
EXPECT_THAT(service->GetBoundSessionThrottlerParams(), Not(IsEmpty()));
EXPECT_CALL(
@@ -801,7 +823,7 @@
// Verify prefs were cleared.
// Ensure on next startup, there won't be a bound session.
ResetCookieRefreshService();
- service = GetCookieRefreshServiceImpl();
+ service = GetOrCreateCookieRefreshServiceImpl();
SCOPED_TRACE("No bound session on Startup.");
VerifyNoBoundSession();
@@ -810,7 +832,8 @@
TEST_F(BoundSessionCookieRefreshServiceImplTest,
TerminateSessionOnPersistentErrorEncountered) {
SetupPreConditionForBoundSession();
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
EXPECT_THAT(service->GetBoundSessionThrottlerParams(), Not(IsEmpty()));
ASSERT_TRUE(cookie_controller());
@@ -829,7 +852,7 @@
// Verify prefs were cleared.
// Ensure on next startup, there won't be a bound session.
ResetCookieRefreshService();
- service = GetCookieRefreshServiceImpl();
+ service = GetOrCreateCookieRefreshServiceImpl();
SCOPED_TRACE("No bound session on Startup.");
VerifyNoBoundSession();
@@ -842,7 +865,8 @@
base::MakeRefCounted<net::HttpResponseHeaders>("");
headers->AddHeader(kSessionTerminationHeaderName,
GetSessionTerminationHeaderValue(kTestSessionId));
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
EXPECT_CALL(
*mock_observer(),
OnBoundSessionTerminated(kTestGoogleURL,
@@ -866,7 +890,8 @@
base::StringPrintf(
"first_attribute=abc;session_id=%s;third_attribute=edf",
kTestSessionId));
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
EXPECT_CALL(
*mock_observer(),
OnBoundSessionTerminated(kTestGoogleURL,
@@ -887,7 +912,8 @@
base::MakeRefCounted<net::HttpResponseHeaders>("");
headers->AddHeader(kSessionTerminationHeaderName,
GetSessionTerminationHeaderValue(kTestSessionId));
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
EXPECT_CALL(
*mock_observer(),
OnBoundSessionTerminated(kTestGoogleURL,
@@ -909,7 +935,8 @@
headers->AddHeader(kSessionTerminationHeaderName,
GetSessionTerminationHeaderValue("different_session_id"));
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
service->MaybeTerminateSession(kTestGoogleURL, headers.get());
VerifyBoundSession(CreateTestBoundSessionParams());
histogram_tester().ExpectTotalCount(
@@ -924,7 +951,8 @@
headers->AddHeader(kSessionTerminationHeaderName,
GetSessionTerminationHeaderValue(kTestSessionId));
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
// `kTestOtherURL` and the bound session URL are from different sites.
service->MaybeTerminateSession(kTestOtherURL, headers.get());
VerifyBoundSession(CreateTestBoundSessionParams());
@@ -938,7 +966,8 @@
scoped_refptr<net::HttpResponseHeaders> headers =
base::MakeRefCounted<net::HttpResponseHeaders>("");
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
service->MaybeTerminateSession(kTestGoogleURL, headers.get());
VerifyBoundSession(CreateTestBoundSessionParams());
histogram_tester().ExpectTotalCount(
@@ -952,7 +981,8 @@
base::MakeRefCounted<net::HttpResponseHeaders>("");
headers->AddHeader(kSessionTerminationHeaderName,
kTestSessionId); // no "session_id=" key
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
service->MaybeTerminateSession(kTestGoogleURL, headers.get());
@@ -969,7 +999,8 @@
headers->AddHeader(kSessionTerminationHeaderName,
base::StringPrintf("other_attribute=%s", kTestSessionId));
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
service->MaybeTerminateSession(kTestGoogleURL, headers.get());
VerifyBoundSession(CreateTestBoundSessionParams());
histogram_tester().ExpectTotalCount(
@@ -979,7 +1010,8 @@
TEST_F(BoundSessionCookieRefreshServiceImplTest,
AddBoundSessionRequestThrottledHandlerReceivers) {
SetupPreConditionForBoundSession();
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
ASSERT_TRUE(cookie_controller());
mojo::Remote<chrome::mojom::BoundSessionRequestThrottledHandler> handler_1;
mojo::Remote<chrome::mojom::BoundSessionRequestThrottledHandler> handler_2;
@@ -1007,7 +1039,8 @@
}
TEST_F(BoundSessionCookieRefreshServiceImplTest, RegisterNewBoundSession) {
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
VerifyNoBoundSession();
auto params = CreateTestBoundSessionParams();
@@ -1019,7 +1052,8 @@
TEST_F(BoundSessionCookieRefreshServiceImplTest,
OverrideExistingBoundSessionSameSessionId) {
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
service->RegisterNewBoundSession(CreateTestBoundSessionParams());
auto new_params = CreateTestBoundSessionParams();
@@ -1036,7 +1070,8 @@
TEST_F(BoundSessionCookieRefreshServiceImplTest,
OverrideExistingBoundSessionWithInvalidParams) {
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
auto original_params = CreateTestBoundSessionParams();
service->RegisterNewBoundSession(original_params);
@@ -1051,7 +1086,8 @@
}
TEST_F(BoundSessionCookieRefreshServiceImplTest, ClearMatchingData) {
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
service->RegisterNewBoundSession(CreateTestBoundSessionParams());
EXPECT_CALL(
@@ -1069,7 +1105,8 @@
TEST_F(BoundSessionCookieRefreshServiceImplTest,
ClearMatchingDataTypeMismatch) {
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
auto params = CreateTestBoundSessionParams();
service->RegisterNewBoundSession(params);
@@ -1082,7 +1119,8 @@
TEST_F(BoundSessionCookieRefreshServiceImplTest,
ClearMatchingDataOriginMismatch) {
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
auto params = CreateTestBoundSessionParams();
service->RegisterNewBoundSession(params);
@@ -1095,7 +1133,8 @@
TEST_F(BoundSessionCookieRefreshServiceImplTest,
ClearMatchingDataOriginMismatchSuborigin) {
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
auto params = CreateTestBoundSessionParams();
service->RegisterNewBoundSession(params);
@@ -1108,7 +1147,8 @@
TEST_F(BoundSessionCookieRefreshServiceImplTest,
ClearMatchingDataCreationTimeMismatch) {
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
auto params = CreateTestBoundSessionParams();
service->RegisterNewBoundSession(params);
@@ -1122,7 +1162,8 @@
}
TEST_F(BoundSessionCookieRefreshServiceImplTest, CreateRegistrationRequest) {
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
service->CreateRegistrationRequest(
CreateTestRegistrationFetcherParams(kDefaultRegistrationPath));
ASSERT_TRUE(registration_fetcher());
@@ -1140,7 +1181,8 @@
TEST_F(BoundSessionCookieRefreshServiceImplTest,
CreateRegistrationRequestNonDefaultPath) {
const std::string kNonDefaultRegistrationPath = "/NonDefaultPath";
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
service->CreateRegistrationRequest(
CreateTestRegistrationFetcherParams(kNonDefaultRegistrationPath));
ASSERT_TRUE(registration_fetcher());
@@ -1155,7 +1197,8 @@
scoped_feature_list.InitAndEnableFeatureWithParameters(
switches::kEnableBoundSessionCredentials,
{{"exclusive-registration-path", ""}});
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
service->CreateRegistrationRequest(
CreateTestRegistrationFetcherParams(kNonDefaultRegistrationPath));
ASSERT_TRUE(registration_fetcher());
@@ -1170,7 +1213,8 @@
scoped_feature_list.InitAndEnableFeatureWithParameters(
switches::kEnableBoundSessionCredentials,
{{"exclusive-registration-path", kCustomPath}});
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
service->CreateRegistrationRequest(
CreateTestRegistrationFetcherParams(kCustomPath));
ASSERT_TRUE(registration_fetcher());
@@ -1184,12 +1228,61 @@
scoped_feature_list.InitAndEnableFeatureWithParameters(
switches::kEnableBoundSessionCredentials,
{{"exclusive-registration-path", "/CustomPath"}});
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
service->CreateRegistrationRequest(
CreateTestRegistrationFetcherParams("/AnotherPath"));
EXPECT_FALSE(registration_fetcher());
}
+TEST_F(BoundSessionCookieRefreshServiceImplTest, StopCookieRotation) {
+ SetupPreConditionForBoundSession();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
+ ASSERT_NE(service, nullptr);
+ ASSERT_NE(cookie_controller(), nullptr);
+ EXPECT_FALSE(cookie_controller()->rotation_stopped());
+
+ service->StopCookieRotation(cookie_controller()->GetBoundSessionKey());
+
+ EXPECT_TRUE(cookie_controller()->rotation_stopped());
+}
+
+TEST_F(BoundSessionCookieRefreshServiceImplTest,
+ StopCookieRotationUnknownSession) {
+ SetupPreConditionForBoundSession();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
+ ASSERT_NE(service, nullptr);
+ ASSERT_NE(cookie_controller(), nullptr);
+ EXPECT_FALSE(cookie_controller()->rotation_stopped());
+
+ service->StopCookieRotation(
+ {.site = kTestGoogleURL, .session_id = "unknown_session"});
+
+ EXPECT_FALSE(cookie_controller()->rotation_stopped());
+}
+
+TEST_F(BoundSessionCookieRefreshServiceImplTest,
+ TerminatesSessionOnCookieRotationStoppedTimeout) {
+ SetupPreConditionForBoundSession();
+ GetOrCreateCookieRefreshServiceImpl();
+
+ EXPECT_CALL(
+ *mock_observer(),
+ OnBoundSessionTerminated(kTestGoogleURL,
+ base::flat_set<std::string>(
+ {k1PSIDTSCookieName, k3PSIDTSCookieName})))
+ .Times(1);
+
+ ASSERT_NE(cookie_controller(), nullptr);
+ cookie_controller()->SimulateOnCookieRotationStoppedTimeout();
+
+ VerifyNoBoundSession();
+ VerifySessionTerminationTriggerRecorded(
+ SessionTerminationTrigger::kRotationStoppedTimeout);
+}
+
// Test suite for tests involving multiple sessions.
class BoundSessionCookieRefreshServiceImplMultiSessionTest
: public BoundSessionCookieRefreshServiceImplTestBase {
@@ -1286,7 +1379,7 @@
for (const auto& params : all_params) {
ASSERT_TRUE(storage()->SaveParams(params));
}
- GetCookieRefreshServiceImpl();
+ GetOrCreateCookieRefreshServiceImpl();
VerifyBoundSessions(all_params);
}
@@ -1298,7 +1391,8 @@
switches::kEnableBoundSessionCredentials,
{{"exclusive-registration-path", ""}});
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
const std::string kFirstPath = "/First";
const std::string kSecondPath = "/Second";
@@ -1341,7 +1435,8 @@
for (const auto& params : all_params) {
ASSERT_TRUE(storage()->SaveParams(params));
}
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
base::test::TestFuture<ResumeBlockedRequestsTrigger> future;
service->HandleRequestBlockedOnCookie(kTestGoogleURL, future.GetCallback());
@@ -1365,7 +1460,8 @@
for (const auto& params : all_params) {
ASSERT_TRUE(storage()->SaveParams(params));
}
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
base::test::TestFuture<ResumeBlockedRequestsTrigger> future;
service->HandleRequestBlockedOnCookie(kTestGoogleURL, future.GetCallback());
@@ -1395,7 +1491,8 @@
ASSERT_TRUE(storage()->SaveParams(params));
}
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
GetCookieController(kGoogleSessionKeyOne)->SetThrottlingRequestsPaused(true);
base::test::TestFuture<ResumeBlockedRequestsTrigger> future;
@@ -1418,7 +1515,8 @@
ASSERT_TRUE(storage()->SaveParams(params));
}
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
GetCookieController(kGoogleSessionKeyOne)->SetThrottlingRequestsPaused(true);
GetCookieController(kGoogleSessionKeyTwo)->SetThrottlingRequestsPaused(true);
@@ -1437,7 +1535,8 @@
for (const auto& params : all_params) {
ASSERT_TRUE(storage()->SaveParams(params));
}
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
// Mark the second session as fresh.
GetCookieController(kGoogleSessionKeyTwo)
->SimulateOnCookieExpirationDateChanged(
@@ -1461,7 +1560,8 @@
for (const auto& params : all_params) {
ASSERT_TRUE(storage()->SaveParams(params));
}
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
// Mark both sessions as fresh.
GetCookieController(kGoogleSessionKeyOne)
->SimulateOnCookieExpirationDateChanged(
@@ -1488,7 +1588,8 @@
for (const auto& params : all_params) {
ASSERT_TRUE(storage()->SaveParams(params));
}
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
base::test::TestFuture<ResumeBlockedRequestsTrigger> future;
// This request is not covered by any session.
@@ -1500,7 +1601,8 @@
TEST_F(BoundSessionCookieRefreshServiceImplMultiSessionTest,
HandleRequestBlockedOnCookieNoSessions) {
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
base::test::TestFuture<ResumeBlockedRequestsTrigger> future;
service->HandleRequestBlockedOnCookie(kTestGoogleURL, future.GetCallback());
@@ -1518,7 +1620,8 @@
for (const auto& params : all_params) {
ASSERT_TRUE(storage()->SaveParams(params));
}
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
base::test::TestFuture<ResumeBlockedRequestsTrigger> future;
service->HandleRequestBlockedOnCookie(kTestGoogleURL, future.GetCallback());
@@ -1532,7 +1635,8 @@
TEST_F(BoundSessionCookieRefreshServiceImplMultiSessionTest,
RegisterNewBoundSession) {
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
VerifyBoundSessions({});
auto params =
@@ -1543,7 +1647,8 @@
TEST_F(BoundSessionCookieRefreshServiceImplMultiSessionTest,
RegisterSecondBoundSessionSameDomainDifferentSessionIds) {
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
auto first_params =
CreateBoundSessionParams(kGoogleSessionKeyOne, {"cookieA", "cookieB"});
service->RegisterNewBoundSession(first_params);
@@ -1556,7 +1661,8 @@
TEST_F(BoundSessionCookieRefreshServiceImplMultiSessionTest,
RegisterSecondBoundSessionSameSessionIdDifferentDomains) {
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
auto first_params =
CreateBoundSessionParams(kGoogleSessionKeyOne, {"cookieA", "cookieB"});
service->RegisterNewBoundSession(first_params);
@@ -1569,7 +1675,8 @@
TEST_F(BoundSessionCookieRefreshServiceImplMultiSessionTest,
RegisterBoundSessionSameSessionKey) {
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
auto other_params =
CreateBoundSessionParams(kGoogleSessionKeyTwo, {"cookieX"});
service->RegisterNewBoundSession(other_params);
@@ -1594,7 +1701,7 @@
for (const auto& params : all_params) {
ASSERT_TRUE(storage()->SaveParams(params));
}
- GetCookieRefreshServiceImpl();
+ GetOrCreateCookieRefreshServiceImpl();
EXPECT_CALL(
*mock_observer(),
@@ -1629,7 +1736,8 @@
OnBoundSessionTerminated(
kTestGoogleURL, base::flat_set<std::string>({"cookieA", "cookieB"})))
.WillOnce([this] { PruneDestroyedControllers(); });
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
service->MaybeTerminateSession(GURL("https://google.com/SignOut"),
headers.get());
// all_params[0] should have been terminated.
@@ -1647,7 +1755,7 @@
for (const auto& params : all_params) {
ASSERT_TRUE(storage()->SaveParams(params));
}
- GetCookieRefreshServiceImpl();
+ GetOrCreateCookieRefreshServiceImpl();
EXPECT_CALL(
*mock_observer(),
@@ -1676,7 +1784,7 @@
ASSERT_TRUE(storage()->SaveParams(params));
}
base::HistogramTester histogram_tester;
- GetCookieRefreshServiceImpl();
+ GetOrCreateCookieRefreshServiceImpl();
histogram_tester.ExpectUniqueSample(
"Signin.BoundSessionCredentials.SessionCountOnInit", all_params.size(),
/*expected_bucket_count=*/1);
@@ -1724,7 +1832,7 @@
for (const auto& params : all_params) {
ASSERT_TRUE(storage()->SaveParams(params));
}
- GetCookieRefreshServiceImpl();
+ GetOrCreateCookieRefreshServiceImpl();
std::vector<bound_session_credentials::BoundSessionParams> expected_sessions;
if (IsContinuityEnabled()) {
// All sessions are expected to run.
@@ -1738,7 +1846,8 @@
TEST_P(BoundSessionCookieRefreshServiceImplFeatureDisabledTest,
CreateRegistrationRequest) {
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
service->CreateRegistrationRequest(
CreateTestRegistrationFetcherParams("/RegisterSession"));
// New sessions shouldn't be registered no matter the extra feature state.
@@ -1747,7 +1856,8 @@
TEST_P(BoundSessionCookieRefreshServiceImplFeatureDisabledTest,
CreateRegistrationRequestWithWsbeta) {
- BoundSessionCookieRefreshServiceImpl* service = GetCookieRefreshServiceImpl();
+ BoundSessionCookieRefreshServiceImpl* service =
+ GetOrCreateCookieRefreshServiceImpl();
service->CreateRegistrationRequest(CreateTestRegistrationFetcherParams(
"/RegisterSession", /*is_wsbeta=*/true));
if (IsWsbetaEnabled()) {
diff --git a/chrome/browser/signin/bound_session_credentials/fake_bound_session_cookie_refresh_service.h b/chrome/browser/signin/bound_session_credentials/fake_bound_session_cookie_refresh_service.h
index db09710..6435391 100644
--- a/chrome/browser/signin/bound_session_credentials/fake_bound_session_cookie_refresh_service.h
+++ b/chrome/browser/signin/bound_session_credentials/fake_bound_session_cookie_refresh_service.h
@@ -47,6 +47,7 @@
base::RepeatingClosure updated_callback) override {}
void CreateRegistrationRequest(
BoundSessionRegistrationFetcherParam registration_params) override {}
+ void StopCookieRotation(const BoundSessionKey& key) override {}
base::WeakPtr<BoundSessionCookieRefreshService> GetWeakPtr() override;
void AddObserver(Observer* observer) override;
void RemoveObserver(Observer* observer) override;
diff --git a/chrome/browser/signin/bound_session_credentials/rotation_debug_info.proto b/chrome/browser/signin/bound_session_credentials/rotation_debug_info.proto
index d7a0d38..c23b696 100644
--- a/chrome/browser/signin/bound_session_credentials/rotation_debug_info.proto
+++ b/chrome/browser/signin/bound_session_credentials/rotation_debug_info.proto
@@ -39,6 +39,7 @@
TERMINATION_HEADER_RECEIVED = 7;
SESSION_OVERRIDE = 8;
ROTATION_CHALLENGE_SESSION_ID_MISMATCH = 9;
+ ROTATION_STOPPED_TIMEOUT = 10;
}
// Contains how many times a specific type of error occurred.
diff --git a/chrome/browser/signin/header_modification_delegate_impl_unittest.cc b/chrome/browser/signin/header_modification_delegate_impl_unittest.cc
index b4b36d942..ee43695 100644
--- a/chrome/browser/signin/header_modification_delegate_impl_unittest.cc
+++ b/chrome/browser/signin/header_modification_delegate_impl_unittest.cc
@@ -48,6 +48,11 @@
(BoundSessionRegistrationFetcherParam registration_params),
(override));
+ MOCK_METHOD(void,
+ StopCookieRotation,
+ (const BoundSessionKey& key),
+ (override));
+
MOCK_METHOD(void, Initialize, (), (override));
MOCK_METHOD(void,
RegisterNewBoundSession,
diff --git a/tools/metrics/histograms/metadata/signin/enums.xml b/tools/metrics/histograms/metadata/signin/enums.xml
index 9333606..6d46c67 100644
--- a/tools/metrics/histograms/metadata/signin/enums.xml
+++ b/tools/metrics/histograms/metadata/signin/enums.xml
@@ -808,6 +808,7 @@
<int value="1" label="Cookies cleared"/>
<int value="2" label="Session termination header"/>
<int value="3" label="Session override"/>
+ <int value="4" label="Rotation stopped timeout"/>
</enum>
<enum name="ShouldShowChromeSigninBubbleWithReason">
