Add wrappers around signin access to net::URLRequest

This patch is preparation for allowing signin to work with the Network
Service as in that path there will be no direct access to the
net::URLRequest object. These wrapper classes expose the minimum
interface needed by the signin module to modify headers.

Bug: 789670
Change-Id: I06bc2d99dba2de8359ab2f93d9a220f6c6e3b502
Reviewed-on: https://chromium-review.googlesource.com/1098377
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569339}

chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc

@@ -430,8 +430,9 @@ void ChromeResourceDispatcherHostDelegate::RequestBeginning(
   if (io_data->policy_header_helper())
     io_data->policy_header_helper()->AddPolicyHeaders(request->url(), request);
 
-  signin::FixAccountConsistencyRequestHeader(request, GURL() /* redirect_url */,
-                                             io_data);
+  signin::ChromeRequestAdapter signin_request_adapter(request);
+  signin::FixAccountConsistencyRequestHeader(
+      &signin_request_adapter, GURL() /* redirect_url */, io_data);
 
   AppendStandardResourceThrottles(request,
                                   resource_context,
@@ -600,8 +601,9 @@ void ChromeResourceDispatcherHostDelegate::OnResponseStarted(
   const ResourceRequestInfo* info = ResourceRequestInfo::ForRequest(request);
   ProfileIOData* io_data = ProfileIOData::FromResourceContext(resource_context);
 
-  signin::ProcessAccountConsistencyResponseHeaders(request, GURL(),
-                                                   io_data->IsOffTheRecord());
+  signin::ResponseAdapter signin_response_adapter(request);
+  signin::ProcessAccountConsistencyResponseHeaders(
+      &signin_response_adapter, GURL(), io_data->IsOffTheRecord());
 
   // Built-in additional protection for the chrome web store origin.
 #if BUILDFLAG(ENABLE_EXTENSIONS)
@@ -676,9 +678,12 @@ void ChromeResourceDispatcherHostDelegate::OnRequestRedirected(
   // X-Chrome-Connected header to all Gaia requests from a connected profile so
   // Gaia could return a 204 response and let Chrome handle the action with
   // native UI.
-  signin::FixAccountConsistencyRequestHeader(request, redirect_url, io_data);
-  signin::ProcessAccountConsistencyResponseHeaders(request, redirect_url,
-                                                   io_data->IsOffTheRecord());
+  signin::ChromeRequestAdapter signin_request_adapter(request);
+  signin::FixAccountConsistencyRequestHeader(&signin_request_adapter,
+                                             redirect_url, io_data);
+  signin::ResponseAdapter signin_response_adapter(request);
+  signin::ProcessAccountConsistencyResponseHeaders(
+      &signin_response_adapter, redirect_url, io_data->IsOffTheRecord());
 
   if (io_data->policy_header_helper())
     io_data->policy_header_helper()->AddPolicyHeaders(redirect_url, request);

chrome/browser/signin/chrome_signin_helper.h

@@ -7,11 +7,15 @@
 
 #include <string>
 
+#include "base/macros.h"
+#include "components/signin/core/browser/signin_header_helper.h"
 #include "content/public/browser/resource_request_info.h"
 
 namespace net {
+class HttpResponseHeaders;
 class URLRequest;
 }
+
 class GURL;
 class ProfileIOData;
 
@@ -22,6 +26,44 @@ class ProfileIOData;
 // handle signin accordingly.
 namespace signin {
 
+class ChromeRequestAdapter : public RequestAdapter {
+ public:
+  explicit ChromeRequestAdapter(net::URLRequest* request);
+  ~ChromeRequestAdapter() override;
+
+  virtual bool IsMainRequestContext(ProfileIOData* io_data);
+  virtual content::ResourceRequestInfo::WebContentsGetter GetWebContentsGetter()
+      const;
+  virtual content::ResourceType GetResourceType() const;
+  virtual GURL GetReferrerOrigin() const;
+
+  // Associate a callback with this request which will be executed when the
+  // request is complete (including any redirects). If a callback was already
+  // registered this function does nothing.
+  virtual void SetDestructionCallback(base::OnceClosure closure);
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ChromeRequestAdapter);
+};
+
+class ResponseAdapter {
+ public:
+  explicit ResponseAdapter(const net::URLRequest* request);
+  virtual ~ResponseAdapter();
+
+  virtual content::ResourceRequestInfo::WebContentsGetter GetWebContentsGetter()
+      const;
+  virtual bool IsMainFrame() const;
+  virtual GURL GetOrigin() const;
+  virtual const net::HttpResponseHeaders* GetHeaders() const;
+  virtual void RemoveHeader(const std::string& name);
+
+ private:
+  const net::URLRequest* const request_;
+
+  DISALLOW_COPY_AND_ASSIGN(ResponseAdapter);
+};
+
 // When Dice is enabled, the AccountReconcilor is blocked for a short delay
 // after sending requests to Gaia. Exposed for testing.
 void SetDiceAccountReconcilorBlockDelayForTesting(int delay_ms);
@@ -31,13 +73,13 @@ void SetDiceAccountReconcilorBlockDelayForTesting(int delay_ms);
 // thread.
 // Returns true if the account consistency header was added to the request.
 // Removes the header if it is already in the headers but should not be there.
-void FixAccountConsistencyRequestHeader(net::URLRequest* request,
+void FixAccountConsistencyRequestHeader(ChromeRequestAdapter* request,
                                         const GURL& redirect_url,
                                         ProfileIOData* io_data);
 
 // Processes account consistency response headers (X-Chrome-Manage-Accounts and
 // Dice). |redirect_url| is empty if the request is not a redirect.
-void ProcessAccountConsistencyResponseHeaders(net::URLRequest* request,
+void ProcessAccountConsistencyResponseHeaders(ResponseAdapter* response,
                                               const GURL& redirect_url,
                                               bool is_off_the_record);
 

chrome/browser/signin/chrome_signin_helper_unittest.cc

@@ -90,7 +90,8 @@ TEST_F(ChromeSigninHelperTest, RemoveDiceSigninHeader) {
   ASSERT_TRUE(request_->response_headers()->HasHeader(kDiceResponseHeader));
 
   // Process the header.
-  signin::ProcessAccountConsistencyResponseHeaders(request_.get(), GURL(),
+  signin::ResponseAdapter response_adapter(request_.get());
+  signin::ProcessAccountConsistencyResponseHeaders(&response_adapter, GURL(),
                                                    false /* is_incognito */);
 
   // Check that the header has been removed.

chrome/browser/signin/chromeos_mirror_account_consistency_browsertest.cc

@@ -63,7 +63,9 @@ void TestMirrorRequestForProfileOnIOThread(
       profile_io->GetMainRequestContext()->CreateRequest(
           GURL(kGaiaUrl), net::DEFAULT_PRIORITY, nullptr,
           TRAFFIC_ANNOTATION_FOR_TESTS);
-  signin::FixAccountConsistencyRequestHeader(request.get(), GURL(), profile_io);
+  signin::ChromeRequestAdapter signin_request_adapter(request.get());
+  signin::FixAccountConsistencyRequestHeader(&signin_request_adapter, GURL(),
+                                             profile_io);
 
   CheckRequestHeader(request.get(), kChromeConnectedHeader,
                      expected_header_value);

components/signin/core/browser/signin_header_helper.cc

@@ -65,6 +65,27 @@ DiceResponseParams::EnableSyncInfo::~EnableSyncInfo() {}
 DiceResponseParams::EnableSyncInfo::EnableSyncInfo(const EnableSyncInfo&) =
     default;
 
+RequestAdapter::RequestAdapter(net::URLRequest* request) : request_(request) {}
+
+RequestAdapter::~RequestAdapter() = default;
+
+const GURL& RequestAdapter::GetUrl() {
+  return request_->url();
+}
+
+bool RequestAdapter::HasHeader(const std::string& name) {
+  return request_->extra_request_headers().HasHeader(name);
+}
+
+void RequestAdapter::RemoveRequestHeaderByName(const std::string& name) {
+  return request_->RemoveRequestHeaderByName(name);
+}
+
+void RequestAdapter::SetExtraHeaderByName(const std::string& name,
+                                          const std::string& value) {
+  return request_->SetExtraRequestHeaderByName(name, value, false);
+}
+
 std::string BuildMirrorRequestCookieIfPossible(
     const GURL& url,
     const std::string& account_id,
@@ -76,7 +97,7 @@ std::string BuildMirrorRequestCookieIfPossible(
 }
 
 bool SigninHeaderHelper::AppendOrRemoveRequestHeader(
-    net::URLRequest* request,
+    RequestAdapter* request,
     const GURL& redirect_url,
     const char* header_name,
     const std::string& header_value) {
@@ -84,15 +105,14 @@ bool SigninHeaderHelper::AppendOrRemoveRequestHeader(
     // If the request is being redirected, and it has the account consistency
     // header, and current url is a Google URL, and the redirected one is not,
     // remove the header.
-    if (!redirect_url.is_empty() &&
-        request->extra_request_headers().HasHeader(header_name) &&
-        IsUrlEligibleForRequestHeader(request->url()) &&
+    if (!redirect_url.is_empty() && request->HasHeader(header_name) &&
+        IsUrlEligibleForRequestHeader(request->GetUrl()) &&
         !IsUrlEligibleForRequestHeader(redirect_url)) {
       request->RemoveRequestHeaderByName(header_name);
     }
     return false;
   }
-  request->SetExtraRequestHeaderByName(header_name, header_value, false);
+  request->SetExtraHeaderByName(header_name, header_value);
   return true;
 }
 
@@ -134,13 +154,13 @@ bool SigninHeaderHelper::ShouldBuildRequestHeader(
 }
 
 void AppendOrRemoveMirrorRequestHeader(
-    net::URLRequest* request,
+    RequestAdapter* request,
     const GURL& redirect_url,
     const std::string& account_id,
     AccountConsistencyMethod account_consistency,
     const content_settings::CookieSettings* cookie_settings,
     int profile_mode_mask) {
-  const GURL& url = redirect_url.is_empty() ? request->url() : redirect_url;
+  const GURL& url = redirect_url.is_empty() ? request->GetUrl() : redirect_url;
   ChromeConnectedHeaderHelper chrome_connected_helper(account_consistency);
   std::string chrome_connected_header_value;
   if (chrome_connected_helper.ShouldBuildRequestHeader(url, cookie_settings)) {
@@ -153,7 +173,7 @@ void AppendOrRemoveMirrorRequestHeader(
 }
 
 bool AppendOrRemoveDiceRequestHeader(
-    net::URLRequest* request,
+    RequestAdapter* request,
     const GURL& redirect_url,
     const std::string& account_id,
     bool sync_enabled,
@@ -161,7 +181,7 @@ bool AppendOrRemoveDiceRequestHeader(
     AccountConsistencyMethod account_consistency,
     const content_settings::CookieSettings* cookie_settings) {
 #if BUILDFLAG(ENABLE_DICE_SUPPORT)
-  const GURL& url = redirect_url.is_empty() ? request->url() : redirect_url;
+  const GURL& url = redirect_url.is_empty() ? request->GetUrl() : redirect_url;
   DiceHeaderHelper dice_helper(
       !account_id.empty() && sync_has_auth_error && sync_enabled,
       account_consistency);

components/signin/core/browser/signin_header_helper.h

@@ -145,12 +145,30 @@ struct DiceResponseParams {
   DISALLOW_COPY_AND_ASSIGN(DiceResponseParams);
 };
 
+class RequestAdapter {
+ public:
+  explicit RequestAdapter(net::URLRequest* request);
+  virtual ~RequestAdapter();
+
+  virtual const GURL& GetUrl();
+  virtual bool HasHeader(const std::string& name);
+  virtual void RemoveRequestHeaderByName(const std::string& name);
+  virtual void SetExtraHeaderByName(const std::string& name,
+                                    const std::string& value);
+
+ protected:
+  net::URLRequest* const request_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(RequestAdapter);
+};
+
 // Base class for managing the signin headers (Dice and Chrome-Connected).
 class SigninHeaderHelper {
  public:
   // Appends or remove the header to a network request if necessary.
   // Returns whether the request has the request header.
-  bool AppendOrRemoveRequestHeader(net::URLRequest* request,
+  bool AppendOrRemoveRequestHeader(RequestAdapter* request,
                                    const GURL& redirect_url,
                                    const char* header_name,
                                    const std::string& header_value);
@@ -192,7 +210,7 @@ std::string BuildMirrorRequestCookieIfPossible(
 // the exception of requests from gaia webview.
 // Removes the header in case it should not be transfered to a redirected url.
 void AppendOrRemoveMirrorRequestHeader(
-    net::URLRequest* request,
+    RequestAdapter* request,
     const GURL& redirect_url,
     const std::string& account_id,
     AccountConsistencyMethod account_consistency,
@@ -204,7 +222,7 @@ void AppendOrRemoveMirrorRequestHeader(
 // Removes the header in case it should not be transfered to a redirected url.
 // Returns whether the request has the Dice request header.
 bool AppendOrRemoveDiceRequestHeader(
-    net::URLRequest* request,
+    RequestAdapter* request,
     const GURL& redirect_url,
     const std::string& account_id,
     bool sync_enabled,

components/signin/core/browser/signin_header_helper_unittest.cc

@@ -58,11 +58,12 @@ class SigninHeaderHelperTest : public testing::Test {
     std::unique_ptr<net::URLRequest> url_request =
         url_request_context_.CreateRequest(url, net::DEFAULT_PRIORITY, nullptr,
                                            TRAFFIC_ANNOTATION_FOR_TESTS);
+    RequestAdapter request_adapter(url_request.get());
     AppendOrRemoveMirrorRequestHeader(
-        url_request.get(), GURL(), account_id, account_consistency_,
+        &request_adapter, GURL(), account_id, account_consistency_,
         cookie_settings_.get(), PROFILE_MODE_DEFAULT);
     AppendOrRemoveDiceRequestHeader(
-        url_request.get(), GURL(), account_id, sync_enabled_,
+        &request_adapter, GURL(), account_id, sync_enabled_,
         sync_has_auth_error_, account_consistency_, cookie_settings_.get());
     return url_request;
   }
@@ -182,8 +183,9 @@ TEST_F(SigninHeaderHelperTest, TestMirrorRequestGoogleComNoProfileConsistency) {
       url_request_context_.CreateRequest(GURL("https://www.google.com"),
                                          net::DEFAULT_PRIORITY, nullptr,
                                          TRAFFIC_ANNOTATION_FOR_TESTS);
+  RequestAdapter request_adapter(url_request.get());
   AppendOrRemoveMirrorRequestHeader(
-      url_request.get(), GURL(), "0123456789", account_consistency_,
+      &request_adapter, GURL(), "0123456789", account_consistency_,
       cookie_settings_.get(), PROFILE_MODE_DEFAULT);
   CheckAccountConsistencyHeaderRequest(url_request.get(),
                                        kChromeConnectedHeader, "");
@@ -196,8 +198,9 @@ TEST_F(SigninHeaderHelperTest, TestMirrorRequestGoogleComProfileConsistency) {
       url_request_context_.CreateRequest(GURL("https://www.google.com"),
                                          net::DEFAULT_PRIORITY, nullptr,
                                          TRAFFIC_ANNOTATION_FOR_TESTS);
+  RequestAdapter request_adapter(url_request.get());
   AppendOrRemoveMirrorRequestHeader(
-      url_request.get(), GURL(), "0123456789", account_consistency_,
+      &request_adapter, GURL(), "0123456789", account_consistency_,
       cookie_settings_.get(), PROFILE_MODE_DEFAULT);
   CheckAccountConsistencyHeaderRequest(
       url_request.get(), kChromeConnectedHeader,
@@ -453,8 +456,9 @@ TEST_F(SigninHeaderHelperTest, TestMirrorHeaderEligibleRedirectURL) {
   std::unique_ptr<net::URLRequest> url_request =
       url_request_context_.CreateRequest(url, net::DEFAULT_PRIORITY, nullptr,
                                          TRAFFIC_ANNOTATION_FOR_TESTS);
+  RequestAdapter request_adapter(url_request.get());
   AppendOrRemoveMirrorRequestHeader(
-      url_request.get(), redirect_url, account_id, account_consistency_,
+      &request_adapter, redirect_url, account_id, account_consistency_,
       cookie_settings_.get(), PROFILE_MODE_DEFAULT);
   EXPECT_TRUE(
       url_request->extra_request_headers().HasHeader(kChromeConnectedHeader));
@@ -470,8 +474,9 @@ TEST_F(SigninHeaderHelperTest, TestMirrorHeaderNonEligibleRedirectURL) {
   std::unique_ptr<net::URLRequest> url_request =
       url_request_context_.CreateRequest(url, net::DEFAULT_PRIORITY, nullptr,
                                          TRAFFIC_ANNOTATION_FOR_TESTS);
+  RequestAdapter request_adapter(url_request.get());
   AppendOrRemoveMirrorRequestHeader(
-      url_request.get(), redirect_url, account_id, account_consistency_,
+      &request_adapter, redirect_url, account_id, account_consistency_,
       cookie_settings_.get(), PROFILE_MODE_DEFAULT);
   EXPECT_FALSE(
       url_request->extra_request_headers().HasHeader(kChromeConnectedHeader));
@@ -490,8 +495,9 @@ TEST_F(SigninHeaderHelperTest, TestIgnoreMirrorHeaderNonEligibleURLs) {
                                          TRAFFIC_ANNOTATION_FOR_TESTS);
   url_request->SetExtraRequestHeaderByName(kChromeConnectedHeader, fake_header,
                                            false);
+  RequestAdapter request_adapter(url_request.get());
   AppendOrRemoveMirrorRequestHeader(
-      url_request.get(), redirect_url, account_id, account_consistency_,
+      &request_adapter, redirect_url, account_id, account_consistency_,
       cookie_settings_.get(), PROFILE_MODE_DEFAULT);
   std::string header;
   EXPECT_TRUE(url_request->extra_request_headers().GetHeader(