File chromium-115-add_BoundSessionRefreshCookieFetcher::Result.patch of Package chromium.17998
commit 73e9d865abd6b636280c4bb45720af2ff2c1e374
Author: Monica Basta <msalama@chromium.org>
Date: Fri Jun 2 13:25:42 2023 +0000
[BSC]: Add BoundSessionRefreshCookieFetcher::Result
Bug: b/273920907
Change-Id: I6508dcb79592420bfa3ebe3aac872c097a303a02
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4574672
Commit-Queue: Monica Basta <msalama@chromium.org>
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1152484}
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 4e7e0b092a568..1c8c0110e3516 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
@@ -93,7 +93,7 @@ void BoundSessionCookieControllerImpl::MaybeRefreshCookie() {
void BoundSessionCookieControllerImpl::OnCookieRefreshFetched(
BoundSessionRefreshCookieFetcher::Result result) {
refresh_cookie_fetcher_.reset();
- if (result.net_error == net::OK && result.response_code == net::HTTP_OK) {
+ if (result == BoundSessionRefreshCookieFetcher::Result::kSuccess) {
// Requests are resumed once the cookie is set in the cookie jar. The
// cookie is expected to be fresh and `this` is notified with its
// expiration date before `OnCookieRefreshFetched` is called.
diff --git a/chrome/browser/signin/bound_session_credentials/bound_session_refresh_cookie_fetcher.h b/chrome/browser/signin/bound_session_credentials/bound_session_refresh_cookie_fetcher.h
index db62988635a26..f7a8b3693346f 100644
--- a/chrome/browser/signin/bound_session_credentials/bound_session_refresh_cookie_fetcher.h
+++ b/chrome/browser/signin/bound_session_credentials/bound_session_refresh_cookie_fetcher.h
@@ -14,10 +14,13 @@
// created per request.
class BoundSessionRefreshCookieFetcher {
public:
- struct Result {
- net::Error net_error;
- absl::optional<int> response_code;
+ enum class Result {
+ kSuccess = 0,
+ kConnectionError = 1,
+ kServerTransientError = 2,
+ kServerPersistentError = 3,
};
+
// Reports the result of the fetch request.
using RefreshCookieCompleteCallback = base::OnceCallback<void(Result)>;
diff --git a/chrome/browser/signin/bound_session_credentials/bound_session_refresh_cookie_fetcher_impl.cc b/chrome/browser/signin/bound_session_credentials/bound_session_refresh_cookie_fetcher_impl.cc
index 46be6f06b147a..a6f038b158311 100644
--- a/chrome/browser/signin/bound_session_credentials/bound_session_refresh_cookie_fetcher_impl.cc
+++ b/chrome/browser/signin/bound_session_credentials/bound_session_refresh_cookie_fetcher_impl.cc
@@ -8,6 +8,7 @@
#include "components/signin/public/base/signin_client.h"
#include "google_apis/gaia/gaia_urls.h"
+#include "net/http/http_status_code.h"
#include "net/traffic_annotation/network_traffic_annotation.h"
#include "services/network/public/cpp/resource_request.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
@@ -102,7 +103,36 @@ void BoundSessionRefreshCookieFetcherImpl::OnURLLoaderComplete(
scoped_refptr<net::HttpResponseHeaders> headers) {
net::Error net_error = static_cast<net::Error>(url_loader_->NetError());
- std::move(callback_).Run(
- Result(net_error, headers ? absl::optional<int>(headers->response_code())
- : absl::nullopt));
+ Result result = GetResultFromNetErrorAndHttpStatusCode(
+ net_error,
+ headers ? absl::optional<int>(headers->response_code()) : absl::nullopt);
+ std::move(callback_).Run(result);
+}
+
+BoundSessionRefreshCookieFetcher::Result
+BoundSessionRefreshCookieFetcherImpl::GetResultFromNetErrorAndHttpStatusCode(
+ net::Error net_error,
+ absl::optional<int> response_code) {
+ if ((net_error != net::OK &&
+ net_error != net::ERR_HTTP_RESPONSE_CODE_FAILURE) ||
+ !response_code) {
+ return BoundSessionRefreshCookieFetcher::Result::kConnectionError;
+ }
+
+ if (response_code == net::HTTP_OK) {
+ return BoundSessionRefreshCookieFetcher::Result::kSuccess;
+ }
+
+ if (response_code >= net::HTTP_INTERNAL_SERVER_ERROR) {
+ // Server error 5xx.
+ return BoundSessionRefreshCookieFetcher::Result::kServerTransientError;
+ }
+
+ if (response_code >= net::HTTP_BAD_REQUEST) {
+ // Server error 4xx.
+ return BoundSessionRefreshCookieFetcher::Result::kServerPersistentError;
+ }
+
+ // Unexpected response code.
+ return BoundSessionRefreshCookieFetcher::Result::kServerPersistentError;
}
diff --git a/chrome/browser/signin/bound_session_credentials/bound_session_refresh_cookie_fetcher_impl.h b/chrome/browser/signin/bound_session_credentials/bound_session_refresh_cookie_fetcher_impl.h
index 733ffbaae088c..52943f0194c32 100644
--- a/chrome/browser/signin/bound_session_credentials/bound_session_refresh_cookie_fetcher_impl.h
+++ b/chrome/browser/signin/bound_session_credentials/bound_session_refresh_cookie_fetcher_impl.h
@@ -31,8 +31,14 @@ class BoundSessionRefreshCookieFetcherImpl
void Start(RefreshCookieCompleteCallback callback) override;
private:
+ FRIEND_TEST_ALL_PREFIXES(BoundSessionRefreshCookieFetcherImplTest,
+ GetResultFromNetErrorAndHttpStatusCode);
+
void StartRefreshRequest();
void OnURLLoaderComplete(scoped_refptr<net::HttpResponseHeaders> headers);
+ Result GetResultFromNetErrorAndHttpStatusCode(
+ net::Error net_error,
+ absl::optional<int> response_code);
const raw_ptr<SigninClient> client_;
const scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
diff --git a/chrome/browser/signin/bound_session_credentials/bound_session_refresh_cookie_fetcher_impl_unittest.cc b/chrome/browser/signin/bound_session_credentials/bound_session_refresh_cookie_fetcher_impl_unittest.cc
index d018592022d55..36ae64f83e4ee 100644
--- a/chrome/browser/signin/bound_session_credentials/bound_session_refresh_cookie_fetcher_impl_unittest.cc
+++ b/chrome/browser/signin/bound_session_credentials/bound_session_refresh_cookie_fetcher_impl_unittest.cc
@@ -19,8 +19,6 @@
#include "services/network/test/test_utils.h"
#include "testing/gtest/include/gtest/gtest.h"
-namespace {
-
class BoundSessionRefreshCookieFetcherImplTest : public ::testing::Test {
public:
BoundSessionRefreshCookieFetcherImplTest() {
@@ -55,8 +53,7 @@ TEST_F(BoundSessionRefreshCookieFetcherImplTest, Success) {
pending_request->request.url.spec(), "");
EXPECT_TRUE(future.Wait());
BoundSessionRefreshCookieFetcher::Result result = future.Get();
- EXPECT_EQ(result.net_error, net::OK);
- EXPECT_EQ(result.response_code, net::HTTP_OK);
+ EXPECT_EQ(result, BoundSessionRefreshCookieFetcher::Result::kSuccess);
}
TEST_F(BoundSessionRefreshCookieFetcherImplTest, FailureNetError) {
@@ -75,8 +72,7 @@ TEST_F(BoundSessionRefreshCookieFetcherImplTest, FailureNetError) {
EXPECT_TRUE(future.Wait());
BoundSessionRefreshCookieFetcher::Result result = future.Get();
- EXPECT_EQ(result.net_error, status.error_code);
- EXPECT_FALSE(result.response_code.has_value());
+ EXPECT_EQ(result, BoundSessionRefreshCookieFetcher::Result::kConnectionError);
}
TEST_F(BoundSessionRefreshCookieFetcherImplTest, FailureHttpError) {
@@ -93,8 +89,38 @@ TEST_F(BoundSessionRefreshCookieFetcherImplTest, FailureHttpError) {
EXPECT_TRUE(future.Wait());
BoundSessionRefreshCookieFetcher::Result result = future.Get();
- EXPECT_EQ(result.net_error, net::ERR_HTTP_RESPONSE_CODE_FAILURE);
- EXPECT_EQ(result.response_code, net::HTTP_UNAUTHORIZED);
+ EXPECT_EQ(result,
+ BoundSessionRefreshCookieFetcher::Result::kServerPersistentError);
+}
+
+TEST_F(BoundSessionRefreshCookieFetcherImplTest,
+ GetResultFromNetErrorAndHttpStatusCode) {
+ // Connection error.
+ EXPECT_EQ(fetcher_->GetResultFromNetErrorAndHttpStatusCode(
+ net::ERR_CONNECTION_TIMED_OUT, absl::nullopt),
+ BoundSessionRefreshCookieFetcher::Result::kConnectionError);
+ // net::OK.
+ EXPECT_EQ(
+ fetcher_->GetResultFromNetErrorAndHttpStatusCode(net::OK, net::HTTP_OK),
+ BoundSessionRefreshCookieFetcher::Result::kSuccess);
+ // net::ERR_HTTP_RESPONSE_CODE_FAILURE
+ EXPECT_EQ(fetcher_->GetResultFromNetErrorAndHttpStatusCode(
+ net::ERR_HTTP_RESPONSE_CODE_FAILURE, net::HTTP_BAD_REQUEST),
+ BoundSessionRefreshCookieFetcher::Result::kServerPersistentError);
+ // Persistent error.
+ EXPECT_EQ(fetcher_->GetResultFromNetErrorAndHttpStatusCode(
+ net::OK, net::HTTP_BAD_REQUEST),
+ BoundSessionRefreshCookieFetcher::Result::kServerPersistentError);
+ EXPECT_EQ(fetcher_->GetResultFromNetErrorAndHttpStatusCode(
+ net::OK, net::HTTP_NOT_FOUND),
+ BoundSessionRefreshCookieFetcher::Result::kServerPersistentError);
+ // Transient error.
+ EXPECT_EQ(fetcher_->GetResultFromNetErrorAndHttpStatusCode(
+ net::OK, net::HTTP_INTERNAL_SERVER_ERROR),
+ BoundSessionRefreshCookieFetcher::Result::kServerTransientError);
+ EXPECT_EQ(fetcher_->GetResultFromNetErrorAndHttpStatusCode(
+ net::OK, net::HTTP_GATEWAY_TIMEOUT),
+ BoundSessionRefreshCookieFetcher::Result::kServerTransientError);
}
TEST_F(BoundSessionRefreshCookieFetcherImplTest, NetworkDelayed) {
@@ -114,5 +140,3 @@ TEST_F(BoundSessionRefreshCookieFetcherImplTest, NetworkDelayed) {
EXPECT_TRUE(future.Wait());
}
-
-} // namespace
diff --git a/chrome/browser/signin/bound_session_credentials/fake_bound_session_refresh_cookie_fetcher.cc b/chrome/browser/signin/bound_session_credentials/fake_bound_session_refresh_cookie_fetcher.cc
index b4b1a07e687cb..fcfa9305d04e9 100644
--- a/chrome/browser/signin/bound_session_credentials/fake_bound_session_refresh_cookie_fetcher.cc
+++ b/chrome/browser/signin/bound_session_credentials/fake_bound_session_refresh_cookie_fetcher.cc
@@ -51,7 +51,8 @@ void FakeBoundSessionRefreshCookieFetcher::SimulateCompleteRefreshRequest(
// Synchronous since tests use `BoundSessionTestCookieManager`.
OnRefreshCookieCompleted(CreateFakeCookie(cookie_expiration.value()));
} else {
- std::move(callback_).Run(Result(net::Error::OK, net::HTTP_FORBIDDEN));
+ std::move(callback_).Run(
+ BoundSessionRefreshCookieFetcher::Result::kServerPersistentError);
}
}
@@ -83,9 +84,11 @@ void FakeBoundSessionRefreshCookieFetcher::OnCookieSet(
net::CookieAccessResult access_result) {
bool success = access_result.status.IsInclude();
if (!success) {
- std::move(callback_).Run(Result(net::Error::OK, net::HTTP_FORBIDDEN));
+ std::move(callback_).Run(
+ BoundSessionRefreshCookieFetcher::Result::kServerPersistentError);
} else {
- std::move(callback_).Run(Result(net::Error::OK, net::HTTP_OK));
+ std::move(callback_).Run(
+ BoundSessionRefreshCookieFetcher::Result::kSuccess);
}
// |This| may be destroyed
}
