Save-Data should not be added to CORS Access-Control-Request-Headers

content/browser/service_worker/service_worker_browsertest.cc

Index: content/browser/service_worker/service_worker_browsertest.cc
diff --git a/content/browser/service_worker/service_worker_browsertest.cc b/content/browser/service_worker/service_worker_browsertest.cc
index 560272db2991920d38aa61e0f148bd0f5f2a7a4b..60a377201d0450b5aab977b80821c4eb7efaa198 100644
--- a/content/browser/service_worker/service_worker_browsertest.cc
+++ b/content/browser/service_worker/service_worker_browsertest.cc
@@ -13,6 +13,7 @@
 #include "base/macros.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
+#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/time/time.h"
@@ -217,6 +218,34 @@ scoped_ptr<net::test_server::HttpResponse> VerifySaveDataHeaderNotInRequest(
   return make_scoped_ptr(new net::test_server::BasicHttpResponse());
 }
 
+scoped_ptr<net::test_server::HttpResponse>
+VerifySaveDataNotInAccessControlRequestHeader(
+    const net::test_server::HttpRequest& request) {
+  // Save-Data should be present.
+  auto it = request.headers.find("Save-Data");
+  EXPECT_NE(request.headers.end(), it);
+  EXPECT_EQ("on", it->second);
+
+  scoped_ptr<net::test_server::BasicHttpResponse> http_response(
+      new net::test_server::BasicHttpResponse());
+  if (request.method == net::test_server::METHOD_OPTIONS) {
+    // Access-Control-Request-Headers should contain 'X-Custom-Header' and not
+    // contain 'Save-Data'.
+    auto it_acrh = request.headers.find("Access-Control-Request-Headers");

[falken, 2016/04/06 04:41:36]

nit: "acrh_iter" is a more common Chromium naming style.

[Raj, 2016/04/06 05:08:41]

Done.

+    EXPECT_NE(request.headers.end(), it_acrh);
+    EXPECT_NE(std::string::npos, it_acrh->second.find("x-custom-header"));
+    EXPECT_EQ(std::string::npos, it_acrh->second.find("save-data"));
+    http_response->AddCustomHeader("Access-Control-Allow-Headers",
+                                   it_acrh->second);
+    http_response->AddCustomHeader("Access-Control-Allow-Methods", "GET");
+    http_response->AddCustomHeader("Access-Control-Allow-Origin", "*");
+  } else {
+    http_response->AddCustomHeader("Access-Control-Allow-Origin", "*");
+    http_response->set_content("PASS");
+  }
+  return std::move(http_response);
+}
+
 // The ImportsBustMemcache test requires that the imported script
 // would naturally be cached in blink's memcache, but the embedded
 // test server doesn't produce headers that allow the blink's memcache
@@ -1116,6 +1145,49 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerBrowserTest, FetchPageWithSaveData) {
   run_loop.Run();
 }
 
+IN_PROC_BROWSER_TEST_F(ServiceWorkerBrowserTest, CrossOriginFetchWithSaveData) {
+  const char kPageUrl[] = "/service_worker/fetch_cross_origin.html";
+  const char kWorkerUrl[] = "/service_worker/fetch_event_pass_through.js";
+  net::EmbeddedTestServer cross_origin_server;
+  cross_origin_server.ServeFilesFromSourceDirectory("content/test/data");
+  cross_origin_server.RegisterRequestHandler(
+      base::Bind(&VerifySaveDataNotInAccessControlRequestHeader));
+  cross_origin_server.Start();
+
+  MockContentBrowserClient content_browser_client;
+  content_browser_client.set_data_saver_enabled(true);
+  ContentBrowserClient* old_client =
+      SetBrowserClientForTesting(&content_browser_client);
+  shell()->web_contents()->GetRenderViewHost()->OnWebkitPreferencesChanged();
+  scoped_refptr<WorkerActivatedObserver> observer =
+      new WorkerActivatedObserver(wrapper());
+  observer->Init();
+  public_context()->RegisterServiceWorker(
+      embedded_test_server()->GetURL(kPageUrl),
+      embedded_test_server()->GetURL(kWorkerUrl),
+      base::Bind(&ExpectResultAndRun, true, base::Bind(&base::DoNothing)));
+  observer->Wait();
+
+  NavigateToURL(shell(),
+                embedded_test_server()->GetURL(base::StringPrintf(
+                    "%s?%s", kPageUrl,
+                    cross_origin_server.GetURL("/cross_origin_request.html")
+                        .spec()
+                        .c_str())));
+  const base::string16 title1 = base::ASCIIToUTF16("PASS");
+  TitleWatcher title_watcher1(shell()->web_contents(), title1);
+  EXPECT_EQ(title1, title_watcher1.WaitAndGetTitle());

[falken, 2016/04/06 04:41:36]

Remove the 1 from these names?

[Raj, 2016/04/06 05:08:41]

Done.

+
+  SetBrowserClientForTesting(old_client);
+  shell()->Close();
+
+  base::RunLoop run_loop;
+  public_context()->UnregisterServiceWorker(
+      embedded_test_server()->GetURL(kPageUrl),
+      base::Bind(&ExpectResultAndRun, true, run_loop.QuitClosure()));
+  run_loop.Run();
+}
+
 IN_PROC_BROWSER_TEST_F(ServiceWorkerBrowserTest,
                        FetchPageWithSaveDataPassThroughOnFetch) {
   const char kPageUrl[] = "/service_worker/pass_through_fetch.html";