save-data header value gets appended on page refreshes

chrome/browser/data_saver/data_saver_browsertest.cc

Index: chrome/browser/data_saver/data_saver_browsertest.cc
diff --git a/chrome/browser/data_saver/data_saver_browsertest.cc b/chrome/browser/data_saver/data_saver_browsertest.cc
index 3c8e38264cd64ee6f8349ca392fbdceb1cb52d4f..f816ed236f48e279dc71824edd69e47f47b02318 100644
--- a/chrome/browser/data_saver/data_saver_browsertest.cc
+++ b/chrome/browser/data_saver/data_saver_browsertest.cc
@@ -6,6 +6,7 @@
 
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/in_process_browser_test.h"
@@ -14,6 +15,8 @@
 #include "content/public/test/browser_test_base.h"
 #include "content/public/test/browser_test_utils.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"
 
 class DataSaverBrowserTest : public InProcessBrowserTest {
  protected:
@@ -45,3 +48,50 @@ IN_PROC_BROWSER_TEST_F(DataSaverBrowserTest, DataSaverDisabled) {
   EnableDataSaver(false);
   VerifySaveDataHeader("None");
 }
+
+class DataSaverWithServerBrowserTest : public InProcessBrowserTest {
+ protected:
+  void Init() {
+    test_server_.reset(new net::EmbeddedTestServer());
+    test_server_->RegisterRequestHandler(
+        base::Bind(&DataSaverWithServerBrowserTest::VerifySaveDataHeader,
+                   base::Unretained(this)));
+    test_server_->ServeFilesFromSourceDirectory("chrome/test/data");
+  }
+  void EnableDataSaver(bool enabled) {
+    PrefService* prefs = browser()->profile()->GetPrefs();
+    prefs->SetBoolean(prefs::kDataSaverEnabled, enabled);
+  }
+
+  scoped_ptr<net::test_server::HttpResponse> VerifySaveDataHeader(
+      const net::test_server::HttpRequest& request) {
+    auto save_data_header = request.headers.find("save-data");

[sclittle, 2016/03/10 21:57:27]

nit: For clarity, could you rename |save_data_header| so that it's clear that
it's an iterator? e.g. save_data_header_it or something maybe. Right now it
looks like it would be the actual header string.

[Raj, 2016/03/10 23:30:39]

Done.

+
+    if (!expected_save_data_header_.empty()) {
+      EXPECT_TRUE(save_data_header != request.headers.end());
+      EXPECT_EQ(expected_save_data_header_, save_data_header->second);
+    }

[sclittle, 2016/03/10 21:57:27]

Should you also check that if |expected_save_data_header_| is empty, there's no
save-data header?

[Raj, 2016/03/10 23:30:39]

Done.

+    return scoped_ptr<net::test_server::HttpResponse>();
+  }
+
+  scoped_ptr<net::EmbeddedTestServer> test_server_;
+  std::string expected_save_data_header_;
+};
+
+IN_PROC_BROWSER_TEST_F(DataSaverWithServerBrowserTest,

[sclittle, 2016/03/10 21:57:27]

Should there also be a test that the save-data header is not present if Data
Saver is turned off before reloading the page?

[Raj, 2016/03/10 23:30:39]

Done.

+                       DataSaverEnabledReloadPage) {
+  Init();
+  ASSERT_TRUE(test_server_->Start());
+  EnableDataSaver(true);
+
+  ui_test_utils::NavigateToURL(browser(),
+                               test_server_->GetURL("/google/google.html"));
+
+  // Reload the webpage and expect all the subresources will get the corect

[sclittle, 2016/03/10 21:57:27]

nit: spelling of "correct"

[Raj, 2016/03/10 23:30:39]

Done.

+  // save-data header.
+  expected_save_data_header_ = "on";
+  chrome::Reload(browser(), CURRENT_TAB);
+  content::WebContents* tab =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  content::WaitForLoadStop(tab);
+}