PlzWorker: Add the Save-Data header to browser-initiated requests for shared worker scripts

This CL adds the Save-Data header to browser-initiated requests for shared
worker's main script. This has been tested by LayoutTests, but it's no longer
available after NetworkService is enabled because LayoutTest's internal API to
enable the data saver feature doesn't take effect on the preferences in the
browser process. Therefore, this CL adds browser tests instead.

Design doc of the PlzWorker:
https://docs.google.com/document/d/1Jtn33bvqkqWxq6K7HIA4uU6HLWPTmOD7vFviacfTmhM/edit?usp=sharing

Bug: 715632
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I1390205530742dba73057f6a866af8db158afe54
Reviewed-on: https://chromium-review.googlesource.com/1209004Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589839}

chrome/browser/data_saver/data_saver_browsertest.cc

@@ -4,6 +4,7 @@
 
 #include <string>
 
+#include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.h"
 #include "chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_factory.h"
@@ -314,3 +315,112 @@ IN_PROC_BROWSER_TEST_F(DataSaverWithServerBrowserTest, HttpRttEstimate) {
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(base::TimeDelta::FromMilliseconds(500), GetHttpRttEstimate());
 }
+
+class DataSaverForWorkerBrowserTest : public InProcessBrowserTest {
+ protected:
+  void Init() {
+    embedded_test_server()->ServeFilesFromSourceDirectory("content/test/data");
+  }
+
+  void EnableDataSaver(bool enabled) {
+    PrefService* prefs = browser()->profile()->GetPrefs();
+    prefs->SetBoolean(prefs::kDataSaverEnabled, enabled);
+    // Give the setting notification a chance to propagate.
+    content::RunAllPendingInMessageLoop();
+  }
+
+  std::unique_ptr<net::test_server::HttpResponse> CaptureHeaderHandler(
+      const std::string& path,
+      net::test_server::HttpRequest::HeaderMap* header_map,
+      base::OnceClosure done_callback,
+      const net::test_server::HttpRequest& request) {
+    GURL request_url = request.GetURL();
+    if (request_url.path() != path)
+      return nullptr;
+
+    *header_map = request.headers;
+    std::move(done_callback).Run();
+    return std::make_unique<net::test_server::BasicHttpResponse>();
+  }
+
+  // Sends a request to |url| and returns its headers via |header_map|.
+  void RequestAndGetHeaders(
+      const std::string& url,
+      net::test_server::HttpRequest::HeaderMap* header_map) {
+    base::RunLoop loop;
+    embedded_test_server()->RegisterRequestHandler(base::BindRepeating(
+        &DataSaverForWorkerBrowserTest::CaptureHeaderHandler,
+        base::Unretained(this), "/capture", header_map, loop.QuitClosure()));
+    ASSERT_TRUE(embedded_test_server()->Start());
+
+    ui_test_utils::NavigateToURL(browser(),
+                                 embedded_test_server()->GetURL(url));
+    loop.Run();
+  }
+};
+
+// Checks that the Save-Data header isn't sent in a request for dedicated worker
+// script when the data saver is disabled.
+IN_PROC_BROWSER_TEST_F(DataSaverForWorkerBrowserTest, DedicatedWorker_Off) {
+  Init();
+  EnableDataSaver(false);
+
+  net::test_server::HttpRequest::HeaderMap header_map;
+  RequestAndGetHeaders("/workers/create_worker.html?worker_url=/capture",
+                       &header_map);
+
+  EXPECT_TRUE(header_map.find("Save-Data") == header_map.end());
+}
+
+// Checks that the Save-Data header is sent in a request for dedicated worker
+// script when the data saver is enabled.
+IN_PROC_BROWSER_TEST_F(DataSaverForWorkerBrowserTest, DedicatedWorker_On) {
+  Init();
+  EnableDataSaver(true);
+
+  net::test_server::HttpRequest::HeaderMap header_map;
+  RequestAndGetHeaders("/workers/create_worker.html?worker_url=/capture",
+                       &header_map);
+
+  EXPECT_TRUE(header_map.find("Save-Data") != header_map.end());
+  EXPECT_EQ("on", header_map["Save-Data"]);
+}
+
+// Checks that the Save-Data header isn't sent in a request for shared worker
+// script when the data saver is disabled. Disabled on Android since a shared
+// worker is not available on Android.
+#if defined(OS_ANDROID)
+#define MAYBE_SharedWorker_Off DISABLED_SharedWorker_Off
+#else
+#define MAYBE_SharedWorker_Off SharedWorker_Off
+#endif
+IN_PROC_BROWSER_TEST_F(DataSaverForWorkerBrowserTest, MAYBE_SharedWorker_Off) {
+  Init();
+  EnableDataSaver(false);
+
+  net::test_server::HttpRequest::HeaderMap header_map;
+  RequestAndGetHeaders("/workers/create_shared_worker.html?worker_url=/capture",
+                       &header_map);
+
+  EXPECT_TRUE(header_map.find("Save-Data") == header_map.end());
+}
+
+// Checks that the Save-Data header is sent in a request for shared worker
+// script when the data saver is enabled. Disabled on Android since a shared
+// worker is not available on Android.
+#if defined(OS_ANDROID)
+#define MAYBE_SharedWorker_On DISABLED_SharedWorker_On
+#else
+#define MAYBE_SharedWorker_On SharedWorker_On
+#endif
+IN_PROC_BROWSER_TEST_F(DataSaverForWorkerBrowserTest, MAYBE_SharedWorker_On) {
+  Init();
+  EnableDataSaver(true);
+
+  net::test_server::HttpRequest::HeaderMap header_map;
+  RequestAndGetHeaders("/workers/create_shared_worker.html?worker_url=/capture",
+                       &header_map);
+
+  EXPECT_TRUE(header_map.find("Save-Data") != header_map.end());
+  EXPECT_EQ("on", header_map["Save-Data"]);
+}

content/browser/shared_worker/shared_worker_service_impl.cc

@@ -14,6 +14,7 @@
 #include "base/feature_list.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/metrics/field_trial_params.h"
 #include "base/task/post_task.h"
 #include "content/browser/appcache/appcache_navigation_handle.h"
 #include "content/browser/appcache/appcache_navigation_handle_core.h"
@@ -37,6 +38,7 @@
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/common/bind_interface_helpers.h"
+#include "content/public/common/content_features.h"
 #include "content/public/common/renderer_preferences.h"
 #include "mojo/public/cpp/bindings/strong_associated_binding.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
@@ -340,6 +342,34 @@ void SharedWorkerServiceImpl::DestroyHost(SharedWorkerHost* host) {
     std::move(terminate_all_workers_callback_).Run();
 }
 
+// TODO(nhiroki): Align this function with AddAdditionalRequestHeaders() in
+// navigation_request.cc, FrameFetchContext, and WorkerFetchContext.
+void SharedWorkerServiceImpl::AddAdditionalRequestHeaders(
+    net::HttpRequestHeaders* headers,
+    BrowserContext* browser_context) {
+  DCHECK(base::FeatureList::IsEnabled(network::features::kNetworkService));
+
+  // Set the "Accept" header.
+  headers->SetHeaderIfMissing(network::kAcceptHeader,
+                              network::kDefaultAcceptHeader);
+
+  // Set the "DNT" header if necessary.
+  RendererPreferences renderer_preferences;
+  GetContentClient()->browser()->UpdateRendererPreferencesForWorker(
+      browser_context, &renderer_preferences);
+  if (renderer_preferences.enable_do_not_track)
+    headers->SetHeaderIfMissing(kDoNotTrackHeader, "1");
+
+  // Set the "Save-Data" header if necessary.
+  if (GetContentClient()->browser()->IsDataSaverEnabled(browser_context) &&
+      !base::GetFieldTrialParamByFeatureAsBool(features::kDataSaverHoldback,
+                                               "holdback_web", false)) {
+    headers->SetHeaderIfMissing("Save-Data", "on");
+  }
+
+  // TODO(nhiroki): Set the "Sec-Metadata" header (https://crbug.com/715632).
+}
+
 void SharedWorkerServiceImpl::CreateWorker(
     std::unique_ptr<SharedWorkerInstance> instance,
     mojom::SharedWorkerClientPtr client,
@@ -389,21 +419,10 @@ void SharedWorkerServiceImpl::CreateWorker(
           weak_host->instance()->constructor_origin();
       resource_request->resource_type = RESOURCE_TYPE_SHARED_WORKER;
 
-      // Set the "Accept" header.
-      resource_request->headers.SetHeader(network::kAcceptHeader,
-                                          network::kDefaultAcceptHeader);
-
-      // Set the "DNT" header if necessary.
-      RendererPreferences renderer_preferences;
-      GetContentClient()->browser()->UpdateRendererPreferencesForWorker(
-          RenderProcessHost::FromID(process_id)->GetBrowserContext(),
-          &renderer_preferences);
-      if (renderer_preferences.enable_do_not_track)
-        resource_request->headers.SetHeader(kDoNotTrackHeader, "1");
-
-      // TODO(nhiroki): Set the "Sec-Metadata" header and the "Save-Data"
-      // header. See AddAdditionalRequestHeaders() in navigation_request.cc for
-      // reference (https://crbug.com/715632).
+      auto* render_process_host = RenderProcessHost::FromID(process_id);
+      DCHECK(!IsShuttingDown(render_process_host));
+      AddAdditionalRequestHeaders(&resource_request->headers,
+                                  render_process_host->GetBrowserContext());
     }
 
     // NetworkService (PlzWorker):

content/browser/shared_worker/shared_worker_service_impl.h

@@ -73,6 +73,9 @@ class CONTENT_EXPORT SharedWorkerServiceImpl : public SharedWorkerService {
   friend class SharedWorkerServiceImplTest;
   friend class SharedWorkerHostTest;
 
+  static void AddAdditionalRequestHeaders(net::HttpRequestHeaders* headers,
+                                          BrowserContext* browser_context);
+
   void CreateWorker(
       std::unique_ptr<SharedWorkerInstance> instance,
       mojom::SharedWorkerClientPtr client,

third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

@@ -30,4 +30,3 @@ Bug(none) http/tests/misc/redirect-to-about-blank.html [ Timeout ]
 # PlzWorker: These are tentatively failing because of unimplemented code.
 crbug.com/715632 external/wpt/fetch/sec-metadata/sharedworker.tentative.https.sub.html [ Failure ]
 crbug.com/715632 fast/workers/shared-worker-load-error.html [ Timeout ]
-crbug.com/715632 http/tests/fetch/chromium/data-saver.html [ Failure ]

third_party/WebKit/LayoutTests/http/tests/fetch/chromium/data-saver.html

@@ -54,28 +54,9 @@ window.top.promise_test(t => {
        });
 }, 'fetch() from dedicated worker with Data-Saver disabled.');
 
-window.top.promise_test(t => {
-  internals.setSaveDataEnabled(false);
-  var worker =
-      new SharedWorker('./resources/data-saver-worker.php?shared-disabled');
-  return new Promise(resolve => {
-         worker.port.addEventListener('message', resolve);
-         worker.port.start();
-       })
-     .then(msg => {
-         var result = msg.data;
-         assert_equals(
-             result['worker_script_header'], 'No Save-Data',
-             'Save-Data header should not be sent for worker script when ' +
-             'disabled.');
-         for (var i = 0; i < METHODS.length; ++i) {
-           assert_equals(
-               result[METHODS[i]], 'No Save-Data',
-               'Save-Data header should not be sent when disabled. method: ' +
-               METHODS[i]);
-         }
-       });
-}, 'fetch() from shared worker with Data-Saver disabled.');
+// A test for shared workers is implemented as a browser test because shared
+// worker script is requested from the browser process when NetworkService is
+// enabled, and internals.setSaveDataEnable() doesn't work in that case.
 
 </script>
 "></iframe>
@@ -131,29 +112,11 @@ window.top.promise_test(t => {
        });
 }, 'fetch() from dedicated worker with Data-Saver enabled.');
 
-window.top.promise_test(t => {
-  internals.setSaveDataEnabled(true);
-  var worker =
-      new SharedWorker('./resources/data-saver-worker.php?shared-enabled');
-  return new Promise(resolve => {
-         worker.port.addEventListener('message', resolve);
-         worker.port.start();
-       })
-     .then(msg => {
-         var result = msg.data;
-         assert_equals(
-             result['worker_script_header'], 'Save-Data: on',
-             'Save-Data header should be sent for worker script when enabled.');
-         for (var i = 0; i < METHODS.length; ++i) {
-           assert_equals(
-               result[METHODS[i]], 'Save-Data: on',
-               'Save-Data header should be sent when enabled. method: ' +
-               METHODS[i]);
-         }
-       });
-}, 'fetch() from shared worker with Data-Saver enabled.');
+// A test for shared workers is implemented as a browser test because shared
+// worker script is requested from the browser process when NetworkService is
+// enabled, and internals.setSaveDataEnable() doesn't work in that case.
 
 </script>
 "></iframe>
 
-</html>
+</html>
\ No newline at end of file

third_party/WebKit/LayoutTests/virtual/outofblink-cors-ns/http/tests/fetch/chromium/data-saver-expected.txt

-This is a testharness.js-based test.
-PASS fetch() from document with Data-Saver disabled.
-PASS fetch() from dedicated worker with Data-Saver disabled.
-PASS fetch() from shared worker with Data-Saver disabled.
-PASS fetch() from document with Data-Saver enabled.
-PASS fetch() from dedicated worker with Data-Saver enabled.
-FAIL fetch() from shared worker with Data-Saver enabled. promise_test: Unhandled rejection with value: object "Error: assert_equals: Save-Data header should be sent for worker script when enabled. expected "Save-Data: on" but got "No Save-Data""
-Harness: the test ran to completion.
-