content_browsertests for stale-while-revalidate

content/browser/loader/async_revalidation_manager_browsertest.cc

Index: content/browser/loader/async_revalidation_manager_browsertest.cc
diff --git a/content/browser/loader/async_revalidation_manager_browsertest.cc b/content/browser/loader/async_revalidation_manager_browsertest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..46e1efb6994605a37cfa40ec9596129cbadccdb9
--- /dev/null
+++ b/content/browser/loader/async_revalidation_manager_browsertest.cc
@@ -0,0 +1,218 @@
+// Copyright 2015 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 <string>
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/command_line.h"
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/run_loop.h"
+#include "base/strings/string16.h"
+#include "base/strings/string_piece.h"
+#include "base/strings/stringprintf.h"
+#include "base/strings/utf_string_conversions.h"
+#include "content/public/test/browser_test_utils.h"
+#include "content/public/test/content_browser_test.h"
+#include "content/public/test/content_browser_test_utils.h"
+#include "content/shell/browser/shell.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 "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+namespace content {
+
+namespace {
+
+using net::test_server::HttpResponse;
+using net::test_server::HttpRequest;
+using net::test_server::BasicHttpResponse;
+
+class AsyncRevalidationManagerBrowserTest : public ContentBrowserTest {
+ protected:
+  AsyncRevalidationManagerBrowserTest() {}
+  ~AsyncRevalidationManagerBrowserTest() override {}
+
+  void SetUp() override {
+    ASSERT_TRUE(embedded_test_server()->InitializeAndListen());
+    ContentBrowserTest::SetUp();
+  }
+
+  void SetUpOnMainThread() override {
+    embedded_test_server()->StartAcceptingConnections();
+  }
+
+  base::RunLoop* run_loop() { return &run_loop_; }
+  int requests_counted() { return requests_counted_; }

[kinuko, 2015/12/15 15:28:54]

nit: can be a const method

[Adam Rice, 2016/01/05 09:21:44]

Done.

+
+  void CheckTitleTest(const GURL& url, const std::string& expected_title) {
+    base::string16 expected_title16(base::ASCIIToUTF16(expected_title));
+    TitleWatcher title_watcher(shell()->web_contents(), expected_title16);
+    NavigateToURL(shell(), url);
+    EXPECT_EQ(expected_title16, title_watcher.WaitAndGetTitle());
+  }
+
+  void RegisterCountingRequestHandler() {
+    embedded_test_server()->RegisterRequestHandler(base::Bind(
+        &AsyncRevalidationManagerBrowserTest::CountingRequestHandler, this));
+  }
+
+  void RegisterCookieRequestHandler() {
+    embedded_test_server()->RegisterRequestHandler(base::Bind(
+        &AsyncRevalidationManagerBrowserTest::CookieRequestHandler, this));
+  }
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {

[kinuko, 2015/12/15 15:28:54]

nit: please place override methods from the same class next to each other

[Adam Rice, 2016/01/05 09:21:44]

Done.

+    // TODO(ricea): Remove this once it becomes the default.
+    command_line->AppendSwitch("enable-stale-while-revalidate");
+  }
+
+ private:
+  // A request handler which increases the number in the title tag on every
+  // request.
+  scoped_ptr<HttpResponse> CountingRequestHandler(const HttpRequest& request) {
+    if (request.relative_url != "/counted.html")
+      return nullptr;
+
+    int version = ++requests_counted_;
+
+    scoped_ptr<BasicHttpResponse> http_response(StaleWhileRevalidateHeaders());
+    http_response->set_content(
+        base::StringPrintf("<title>Version %d</title>", version));
+
+    // This condition is appropriate for the StaleWhileRevalidateIsApplied
+    // test.
+    // TODO(ricea): Support a general condition if other tests need it.

[kinuko, 2015/12/15 15:28:54]

nit: I don't think these TODO comments add some value in testing code, please
just clarify why this condition is appropriate in the current test code

[Adam Rice, 2016/01/05 09:21:43]

Done.

+    if (version == 2)
+      run_loop_.Quit();
+    return http_response.Pass();

[kinuko, 2015/12/15 15:28:54]

nit: move?

[Adam Rice, 2016/01/05 09:21:43]

Done.

+  }
+
+  // A request handler which increases a cookie value on every request.
+  scoped_ptr<HttpResponse> CookieRequestHandler(const HttpRequest& request) {
+    static const char kHtml[] =
+        "<script>\n"
+        "var intervalId;\n"
+        "function checkCookie() {\n"
+        "  if (document.cookie.search(/version=2/) != -1) {\n"
+        "    clearInterval(intervalId);\n"
+        "    document.title = \"PASS\";\n"
+        "  }\n"
+        "}\n"
+        "intervalId = setInterval(checkCookie, 10);\n"
+        "</script>\n"
+        "<title>Loaded</title>\n";
+
+    if (request.relative_url != "/cookie.html")
+      return nullptr;
+
+    int version = ++requests_counted_;
+
+    scoped_ptr<BasicHttpResponse> http_response(StaleWhileRevalidateHeaders());
+    http_response->AddCustomHeader("Set-Cookie",
+                                   base::StringPrintf("version=%d", version));
+    http_response->set_content(kHtml);
+    return http_response.Pass();
+  }
+
+  // Generate the standard response headers common to all request handlers.
+  scoped_ptr<BasicHttpResponse> StaleWhileRevalidateHeaders() {
+    scoped_ptr<BasicHttpResponse> http_response(new BasicHttpResponse);
+    http_response->set_code(net::HTTP_OK);
+    http_response->set_content_type("text/html; charset=utf-8");
+    http_response->AddCustomHeader("Cache-Control",
+                                   "max-age=0, stale-while-revalidate=86400");
+    // A validator is needed for revalidations, and hence
+    // stale-while-revalidate, to work.
+    std::string etag = base::StringPrintf(
+        "\"AsyncRevalidationManagerBrowserTest%d\"", requests_counted_);
+    http_response->AddCustomHeader("ETag", etag);
+    return http_response.Pass();

[kinuko, 2015/12/15 15:28:54]

std::move

[Adam Rice, 2016/01/05 09:21:44]

Done.

[Adam Rice, 2016/01/05 11:40:32]

Actually, this caused a compiler error about std::move preventing copy
constructor elision. It appears to work fine without the std::move, however.

+  }
+
+  // This is only used by the StaleWhileRevalidateIsApplied test.
+  base::RunLoop run_loop_;
+  int requests_counted_ = 0;
+
+  DISALLOW_COPY_AND_ASSIGN(AsyncRevalidationManagerBrowserTest);
+};
+
+// Verify that the "Cache-Control: stale-while-revalidate" directive correctly
+// triggers an async revalidation.
+IN_PROC_BROWSER_TEST_F(AsyncRevalidationManagerBrowserTest,
+                       StaleWhileRevalidateIsApplied) {
+  RegisterCountingRequestHandler();
+  GURL url(embedded_test_server()->GetURL("/counted.html"));

[kinuko, 2015/12/15 15:28:54]

nit: should these URL be a static const variable if we refer this URL at
multiple places

[Adam Rice, 2016/01/05 09:21:44]

I used namespace-scoped constants to avoid needing separate definition and
declarations. PTAL.

+
+  CheckTitleTest(url, "Version 1");
+
+  // Force the renderer to be destroyed so that the Blink cache doesn't
+  // interfere with the result.
+  NavigateToURL(shell(), GURL("about:blank"));
+
+  // Load the page again. We should get the stale version from the cache.
+  CheckTitleTest(url, "Version 1");
+
+  // Wait for the async revalidation to complete.
+  run_loop()->Run();
+  EXPECT_EQ(2, requests_counted());

[kinuko, 2015/12/15 15:28:54]

Also check EXPECT_EQ(1, requests_counted()) before we make async revalidation?

[Adam Rice, 2016/01/05 09:21:44]

Done.

+}
+
+// The fresh cache entry must become visible eventually.
+IN_PROC_BROWSER_TEST_F(AsyncRevalidationManagerBrowserTest, CacheIsUpdated) {
+  using base::ASCIIToUTF16;
+  RegisterCountingRequestHandler();
+  GURL url(embedded_test_server()->GetURL("/counted.html"));
+
+  CheckTitleTest(url, "Version 1");
+
+  // Reset the renderer cache.
+  NavigateToURL(shell(), GURL("about:blank"));
+
+  // Load the page again. We should get the stale version from the cache.
+  CheckTitleTest(url, "Version 1");
+
+  // Load the page repeatedly until we see the new version.
+  const int kMaxLoads = 100;
+  int actual_loads = 0;
+  bool matched = false;
+
+  for (; actual_loads < kMaxLoads && !matched; ++actual_loads) {
+    TitleWatcher title_watcher(shell()->web_contents(),
+                               ASCIIToUTF16("Version 2"));
+    title_watcher.AlsoWaitForTitle(ASCIIToUTF16("Version 1"));
+    NavigateToURL(shell(), GURL("about:blank"));
+    NavigateToURL(shell(), url);
+    matched = (title_watcher.WaitAndGetTitle() == ASCIIToUTF16("Version 2"));
+  }
+
+  EXPECT_TRUE(matched);
+  // actual_loads is usually 1 but larger numbers are not an error.
+  DVLOG(1) << "Number of loads: " << actual_loads;

[kinuko, 2015/12/15 15:28:54]

Having DVLOG(1) in tests... what usage are you imagining?  This doesn't seem a
part of test if we're just printing.

[Adam Rice, 2016/01/05 09:21:44]

If the test became flaky, the log could be used to adjust the value of
kMaxLoads. However, that doesn't seem likely to happen in practice.

As you say, it isn't actually part of the test, so I have removed it.

+}
+
+// When the asynchronous revalidation arrives, any cookies it contains must be
+// applied immediately.
+IN_PROC_BROWSER_TEST_F(AsyncRevalidationManagerBrowserTest,
+                       CookieSetAsynchronously) {
+  RegisterCookieRequestHandler();
+  GURL url(embedded_test_server()->GetURL("/cookie.html"));
+
+  // Set cookie to version=1
+  NavigateToURL(shell(), url);
+
+  // Reset render cache.
+  NavigateToURL(shell(), GURL("about:blank"));
+
+  // The page will load from the cache, then when the async revalidation
+  // completes the cookie will update.
+  CheckTitleTest(url, "PASS");
+}
+
+}  // namespace
+
+}  // namespace content