Add browser test for ChromeResourceDispatcherHostDelegate.

Also fixed bug in ChromeResourceDispatcherHostDelegate where it was using the wrong URL when detecting whether to add a policy header to a redirect (yay for tests).

BUG=378729

Review URL: https://codereview.chromium.org/303993005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@275498 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc

@@ -387,7 +387,7 @@ void ChromeResourceDispatcherHostDelegate::RequestBeginning(
 
 #if defined(ENABLE_CONFIGURATION_POLICY)
   if (io_data->policy_header_helper())
-    io_data->policy_header_helper()->AddPolicyHeaders(request);
+    io_data->policy_header_helper()->AddPolicyHeaders(request->url(), request);
 #endif
 
   signin::AppendMirrorRequestHeaderIfPossible(
@@ -690,7 +690,7 @@ void ChromeResourceDispatcherHostDelegate::OnRequestRedirected(
 
 #if defined(ENABLE_CONFIGURATION_POLICY)
   if (io_data->policy_header_helper())
-    io_data->policy_header_helper()->AddPolicyHeaders(request);
+    io_data->policy_header_helper()->AddPolicyHeaders(redirect_url, request);
 #endif
 }
 

chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate_browsertest.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/command_line.h"
+#include "base/strings/string_util.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/policy/cloud/policy_header_service_factory.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "components/policy/core/common/cloud/cloud_policy_constants.h"
+#include "components/policy/core/common/cloud/policy_header_io_helper.h"
+#include "components/policy/core/common/cloud/policy_header_service.h"
+#include "components/policy/core/common/policy_switches.h"
+#include "content/public/browser/resource_dispatcher_host.h"
+#include "net/http/http_request_headers.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "net/test/embedded_test_server/http_request.h"
+#include "net/test/embedded_test_server/http_response.h"
+#include "net/url_request/url_request.h"
+
+namespace {
+static const char kTestPolicyHeader[] = "test_header";
+static const char kServerRedirectUrl[] = "/server-redirect";
+
+scoped_ptr<net::test_server::HttpResponse> HandleTestRequest(
+    const net::test_server::HttpRequest& request) {
+  if (StartsWithASCII(request.relative_url, kServerRedirectUrl, true)) {
+    // Extract the target URL and redirect there.
+    size_t query_string_pos = request.relative_url.find('?');
+    std::string redirect_target =
+        request.relative_url.substr(query_string_pos + 1);
+
+    scoped_ptr<net::test_server::BasicHttpResponse> http_response(
+        new net::test_server::BasicHttpResponse);
+    http_response->set_code(net::HTTP_MOVED_PERMANENTLY);
+    http_response->AddCustomHeader("Location", redirect_target);
+    return http_response.PassAs<net::test_server::HttpResponse>();
+  } else {
+    scoped_ptr<net::test_server::BasicHttpResponse> http_response(
+        new net::test_server::BasicHttpResponse);
+    http_response->set_code(net::HTTP_OK);
+    http_response->set_content("Success");
+    return http_response.PassAs<net::test_server::HttpResponse>();
+  }
+}
+
+class TestDispatcherHostDelegate : public ChromeResourceDispatcherHostDelegate {
+ public:
+  explicit TestDispatcherHostDelegate(
+      prerender::PrerenderTracker* prerender_tracker)
+      : ChromeResourceDispatcherHostDelegate(prerender_tracker) {
+  }
+
+  virtual ~TestDispatcherHostDelegate() {}
+
+  virtual void RequestBeginning(
+      net::URLRequest* request,
+      content::ResourceContext* resource_context,
+      appcache::AppCacheService* appcache_service,
+      ResourceType::Type resource_type,
+      int child_id,
+      int route_id,
+      ScopedVector<content::ResourceThrottle>* throttles) OVERRIDE {
+    ChromeResourceDispatcherHostDelegate::RequestBeginning(
+        request,
+        resource_context,
+        appcache_service,
+        resource_type,
+        child_id,
+        route_id,
+        throttles);
+    request_headers_.MergeFrom(request->extra_request_headers());
+  }
+
+  virtual void OnRequestRedirected(
+      const GURL& redirect_url,
+      net::URLRequest* request,
+      content::ResourceContext* resource_context,
+      content::ResourceResponse* response) OVERRIDE {
+    ChromeResourceDispatcherHostDelegate::OnRequestRedirected(
+        redirect_url,
+        request,
+        resource_context,
+        response);
+    request_headers_.MergeFrom(request->extra_request_headers());
+  }
+
+  net::HttpRequestHeaders request_headers_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TestDispatcherHostDelegate);
+};
+
+}  // namespace
+
+class ChromeResourceDispatcherHostDelegateBrowserTest :
+    public InProcessBrowserTest {
+ public:
+  ChromeResourceDispatcherHostDelegateBrowserTest() {}
+
+  virtual void SetUpOnMainThread() OVERRIDE {
+    InProcessBrowserTest::SetUpOnMainThread();
+    // Hook navigations with our delegate.
+    dispatcher_host_delegate_.reset(new TestDispatcherHostDelegate(
+        g_browser_process->prerender_tracker()));
+    content::ResourceDispatcherHost::Get()->SetDelegate(
+        dispatcher_host_delegate_.get());
+
+    embedded_test_server()->RegisterRequestHandler(
+        base::Bind(&HandleTestRequest));
+    ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
+    // Tell chrome that this is our DM server.
+    dm_url_ = embedded_test_server()->GetURL("/DeviceManagement");
+
+    // At this point, the Profile is already initialized and it's too
+    // late to set the DMServer URL via command line flags, so directly
+    // inject it to the PolicyHeaderIOHelper.
+    policy::PolicyHeaderService* policy_header_service =
+        policy::PolicyHeaderServiceFactory::GetForBrowserContext(
+            browser()->profile());
+    std::vector<policy::PolicyHeaderIOHelper*> helpers =
+        policy_header_service->GetHelpersForTest();
+    for (std::vector<policy::PolicyHeaderIOHelper*>::const_iterator it =
+             helpers.begin();
+         it != helpers.end(); ++it) {
+      (*it)->SetServerURLForTest(dm_url_.spec());
+      (*it)->UpdateHeader(kTestPolicyHeader);
+    }
+  }
+
+  virtual void CleanUpOnMainThread() OVERRIDE {
+    content::ResourceDispatcherHost::Get()->SetDelegate(NULL);
+    dispatcher_host_delegate_.reset();
+    InProcessBrowserTest::CleanUpOnMainThread();
+  }
+
+ protected:
+  // The fake URL for DMServer we are using.
+  GURL dm_url_;
+  scoped_ptr<TestDispatcherHostDelegate> dispatcher_host_delegate_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ChromeResourceDispatcherHostDelegateBrowserTest);
+};
+
+
+IN_PROC_BROWSER_TEST_F(ChromeResourceDispatcherHostDelegateBrowserTest,
+                       NoPolicyHeader) {
+  // When fetching non-DMServer URLs, we should not add a policy header to the
+  // request.
+  DCHECK(!embedded_test_server()->base_url().spec().empty());
+  ui_test_utils::NavigateToURL(browser(), embedded_test_server()->base_url());
+  ASSERT_FALSE(dispatcher_host_delegate_->request_headers_.HasHeader(
+      policy::kChromePolicyHeader));
+}
+
+IN_PROC_BROWSER_TEST_F(ChromeResourceDispatcherHostDelegateBrowserTest,
+                       PolicyHeader) {
+  // When fetching a DMServer URL, we should add a policy header to the
+  // request.
+  ui_test_utils::NavigateToURL(browser(), dm_url_);
+  std::string value;
+  ASSERT_TRUE(dispatcher_host_delegate_->request_headers_.GetHeader(
+      policy::kChromePolicyHeader, &value));
+  ASSERT_EQ(kTestPolicyHeader, value);
+}
+
+IN_PROC_BROWSER_TEST_F(ChromeResourceDispatcherHostDelegateBrowserTest,
+                       PolicyHeaderForRedirect) {
+
+  // Build up a URL that results in a redirect to the DMServer URL to make
+  // sure the policy header is still added.
+  std::string redirect_url;
+  redirect_url += kServerRedirectUrl;
+  redirect_url += "?";
+  redirect_url += dm_url_.spec();
+  ui_test_utils::NavigateToURL(browser(), embedded_test_server()->GetURL(
+      redirect_url));
+  std::string value;
+  ASSERT_TRUE(dispatcher_host_delegate_->request_headers_.GetHeader(
+      policy::kChromePolicyHeader, &value));
+  ASSERT_EQ(kTestPolicyHeader, value);
+}

chrome/chrome_tests.gypi

@@ -1326,6 +1326,7 @@
         'browser/renderer_context_menu/render_view_context_menu_test_util.h',
         'browser/renderer_context_menu/spellchecker_submenu_observer_browsertest.cc',
         'browser/renderer_context_menu/spelling_menu_observer_browsertest.cc',
+        'browser/renderer_host/chrome_resource_dispatcher_host_delegate_browsertest.cc',
         'browser/renderer_host/render_process_host_chrome_browsertest.cc',
         'browser/renderer_host/web_cache_manager_browsertest.cc',
         'browser/repost_form_warning_browsertest.cc',

components/policy/core/common/cloud/cloud_policy_constants.cc

@@ -51,6 +51,8 @@ const char kChromeExtensionPolicyType[] = "google/chrome/extension";
 
 }  // namespace dm_protocol
 
+const char kChromePolicyHeader[] = "Chrome-Policy-Posture";
+
 #if !defined(OS_CHROMEOS)
 const uint8 kPolicyVerificationKey[] = {
   0x30, 0x82, 0x01, 0x22, 0x30, 0x0D, 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7,

components/policy/core/common/cloud/cloud_policy_constants.h

@@ -52,6 +52,9 @@ enum PolicyFetchStatus {
 
 }  // namespace dm_protocol
 
+// The header used to transmit the policy ID for this client.
+POLICY_EXPORT extern const char kChromePolicyHeader[];
+
 // Information about the verification key used to verify that policy signing
 // keys are valid.
 POLICY_EXPORT std::string GetPolicyVerificationKey();

components/policy/core/common/cloud/policy_header_io_helper.cc

@@ -7,15 +7,9 @@
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/sequenced_task_runner.h"
+#include "components/policy/core/common/cloud/cloud_policy_constants.h"
 #include "net/url_request/url_request.h"
 
-namespace {
-
-// The name of the header containing the policy information.
-const char kChromePolicyHeader[] = "Chrome-Policy-Posture";
-
-}  // namespace
-
 namespace policy {
 
 PolicyHeaderIOHelper::PolicyHeaderIOHelper(
@@ -31,9 +25,9 @@ PolicyHeaderIOHelper::~PolicyHeaderIOHelper() {
 }
 
 // Sets any necessary policy headers on the passed request.
-void PolicyHeaderIOHelper::AddPolicyHeaders(net::URLRequest* request) const {
+void PolicyHeaderIOHelper::AddPolicyHeaders(const GURL& url,
+                                            net::URLRequest* request) const {
   DCHECK(io_task_runner_->RunsTasksOnCurrentThread());
-  const GURL& url = request->url();
   if (!policy_header_.empty() &&
       url.spec().compare(0, server_url_.size(), server_url_) == 0) {
     request->SetExtraRequestHeaderByName(kChromePolicyHeader,
@@ -56,4 +50,18 @@ void PolicyHeaderIOHelper::UpdateHeaderOnIOThread(
   policy_header_ = new_header;
 }
 
+void PolicyHeaderIOHelper::SetServerURLForTest(const std::string& server_url) {
+  io_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&PolicyHeaderIOHelper::SetServerURLOnIOThread,
+                 base::Unretained(this), server_url));
+
+}
+
+void PolicyHeaderIOHelper::SetServerURLOnIOThread(
+    const std::string& server_url) {
+  DCHECK(io_task_runner_->RunsTasksOnCurrentThread());
+  server_url_ = server_url;
+}
+
 }  // namespace policy

components/policy/core/common/cloud/policy_header_io_helper.h

@@ -10,6 +10,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/sequenced_task_runner.h"
 #include "components/policy/policy_export.h"
+#include "url/gurl.h"
 
 namespace net {
 class URLRequest;
@@ -31,9 +32,10 @@ class POLICY_EXPORT PolicyHeaderIOHelper {
       const scoped_refptr<base::SequencedTaskRunner>& task_runner);
   ~PolicyHeaderIOHelper();
 
-  // Sets any necessary policy headers on the passed request. Should be invoked
-  // only from the I/O thread.
-  void AddPolicyHeaders(net::URLRequest* request) const;
+  // Sets any necessary policy headers for the specified URL on the passed
+  // request. Should be invoked only from the I/O thread.
+  void AddPolicyHeaders(const GURL& url,
+                        net::URLRequest* request) const;
 
   // API invoked when the header changes. Can be called from any thread - calls
   // are marshalled via the TaskRunner to run on the appropriate thread.
@@ -41,10 +43,18 @@ class POLICY_EXPORT PolicyHeaderIOHelper {
   // outgoing requests.
   void UpdateHeader(const std::string& new_header);
 
+  // Test-only routine used to inject the server URL at runtime - this is
+  // required because PolicyHeaderIOHelper is created very early in BrowserTest
+  // initialization, before we startup the EmbeddedTestServer.
+  void SetServerURLForTest(const std::string& server_url);
+
  private:
   // API invoked via the TaskRunner to update the header.
   void UpdateHeaderOnIOThread(const std::string& new_header);
 
+  // Helper routine invoked via the TaskRunner to update the cached server URL.
+  void SetServerURLOnIOThread(const std::string& new_header);
+
   // The URL we should add policy headers to.
   std::string server_url_;
 

components/policy/core/common/cloud/policy_header_io_helper_unittest.cc

@@ -51,14 +51,14 @@ class PolicyHeaderIOHelperTest : public testing::Test {
 TEST_F(PolicyHeaderIOHelperTest, InitialHeader) {
   net::TestURLRequest request(
       GURL(kDMServerURL), net::DEFAULT_PRIORITY, NULL, &context_);
-  helper_->AddPolicyHeaders(&request);
+  helper_->AddPolicyHeaders(request.url(), &request);
   ValidateHeader(request.extra_request_headers(), kInitialPolicyHeader);
 }
 
 TEST_F(PolicyHeaderIOHelperTest, NoHeaderOnNonMatchingURL) {
   net::TestURLRequest request(
       GURL("http://non-matching.com"), net::DEFAULT_PRIORITY, NULL, &context_);
-  helper_->AddPolicyHeaders(&request);
+  helper_->AddPolicyHeaders(request.url(), &request);
   EXPECT_TRUE(request.extra_request_headers().IsEmpty());
 }
 
@@ -68,7 +68,7 @@ TEST_F(PolicyHeaderIOHelperTest, HeaderChange) {
   task_runner_->RunUntilIdle();
   net::TestURLRequest request(
       GURL(kDMServerURL), net::DEFAULT_PRIORITY, NULL, &context_);
-  helper_->AddPolicyHeaders(&request);
+  helper_->AddPolicyHeaders(request.url(), &request);
   ValidateHeader(request.extra_request_headers(), new_header);
 }
 
@@ -77,7 +77,7 @@ TEST_F(PolicyHeaderIOHelperTest, ChangeToNoHeader) {
   task_runner_->RunUntilIdle();
   net::TestURLRequest request(
       GURL(kDMServerURL), net::DEFAULT_PRIORITY, NULL, &context_);
-  helper_->AddPolicyHeaders(&request);
+  helper_->AddPolicyHeaders(request.url(), &request);
   EXPECT_TRUE(request.extra_request_headers().IsEmpty());
 }
 

components/policy/core/common/cloud/policy_header_service.cc

@@ -98,4 +98,8 @@ void PolicyHeaderService::OnStoreError(CloudPolicyStore* store) {
   // Do nothing on errors.
 }
 
+std::vector<PolicyHeaderIOHelper*> PolicyHeaderService::GetHelpersForTest() {
+  return helpers_;
+}
+
 }  // namespace policy

components/policy/core/common/cloud/policy_header_service.h

@@ -47,6 +47,9 @@ class POLICY_EXPORT PolicyHeaderService : public CloudPolicyStore::Observer {
   virtual void OnStoreLoaded(CloudPolicyStore* store) OVERRIDE;
   virtual void OnStoreError(CloudPolicyStore* store) OVERRIDE;
 
+  // Returns a list of all PolicyHeaderIOHelpers created by this object.
+  std::vector<PolicyHeaderIOHelper*> GetHelpersForTest();
+
  private:
   // Generate a policy header based on the currently loaded policy.
   std::string CreateHeaderValue();

components/policy/core/common/cloud/policy_header_service_unittest.cc

@@ -108,7 +108,7 @@ TEST_F(PolicyHeaderServiceTest, TestWithAndWithoutPolicyHeader) {
   net::TestURLRequestContext context;
   net::TestURLRequest request(
       GURL(kDMServerURL), net::DEFAULT_PRIORITY, NULL, &context);
-  helper_->AddPolicyHeaders(&request);
+  helper_->AddPolicyHeaders(request.url(), &request);
   ValidateHeader(request.extra_request_headers(), expected_dmtoken,
                  expected_policy_token);
 
@@ -118,7 +118,7 @@ TEST_F(PolicyHeaderServiceTest, TestWithAndWithoutPolicyHeader) {
 
   net::TestURLRequest request2(
       GURL(kDMServerURL), net::DEFAULT_PRIORITY, NULL, &context);
-  helper_->AddPolicyHeaders(&request2);
+  helper_->AddPolicyHeaders(request2.url(), &request2);
   ValidateHeader(request2.extra_request_headers(), "", "");
 }