chromium/chromium-115-add_BoundSessionRefreshCookieFetcher::Result.patch

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
 }