Reload cache control test: add more tests

content/browser/loader/reload_cache_control_browsertest.cc

 // Copyright 2016 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 <memory>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "content/public/test/content_browser_test.h"
 #include "content/public/test/content_browser_test_utils.h"
@@ skipping 38 matching lines @@
     // Because the handler is registered as the first handler, MonitorHandler
     // is needed to capture all requests.
     embedded_test_server()->RegisterRequestMonitor(base::Bind(
         &ReloadCacheControlBrowserTest::MonitorRequestHandler,
         base::Unretained(this)));
 
     ASSERT_TRUE(embedded_test_server()->Start());
   }
 
  protected:
   std::vector<RequestLog> request_log_;

[mmenke, 2016/12/02 20:16:49]

While you're here, mind including string and vector?

[Takashi Toyoshima, 2016/12/05 06:39:13]

Done.

 
  private:
   void MonitorRequestHandler(const HttpRequest& request) {
     RequestLog log;
     log.relative_url = request.relative_url;
     auto cache_control = request.headers.find("Cache-Control");
     log.cache_control = cache_control == request.headers.end()
                             ? kNoCacheControl
                             : cache_control->second;
     request_log_.push_back(log);
   }
 
   DISALLOW_COPY_AND_ASSIGN(ReloadCacheControlBrowserTest);

[mmenke, 2016/12/02 20:16:49]

And macros.h

[Takashi Toyoshima, 2016/12/05 06:39:13]

Done.

 };
 
 IN_PROC_BROWSER_TEST_F(ReloadCacheControlBrowserTest, NormalReload) {
   GURL url(embedded_test_server()->GetURL(kReloadTestPath));
 
   EXPECT_TRUE(NavigateToURL(shell(), url));
   ReloadBlockUntilNavigationsComplete(shell(), 1);
 
   ASSERT_EQ(4UL, request_log_.size());
   EXPECT_EQ(kReloadTestPath, request_log_[0].relative_url);
   EXPECT_EQ(kNoCacheControl, request_log_[0].cache_control);
   EXPECT_EQ(kReloadImagePath, request_log_[1].relative_url);
   EXPECT_EQ(kNoCacheControl, request_log_[1].cache_control);
 
   EXPECT_EQ(kReloadTestPath, request_log_[2].relative_url);
   EXPECT_EQ(kMaxAgeCacheControl, request_log_[2].cache_control);
   EXPECT_EQ(kReloadImagePath, request_log_[3].relative_url);
   EXPECT_EQ(kNoCacheControl, request_log_[3].cache_control);

[mmenke, 2016/12/02 20:16:49]

These tests don't seem to be threadsafe - there's no lock around request_log_,
which is accessed on multiple threads.  I guess there's an implicit memory
barrier when the embedded test server writes to the socket, so as long as there
are no unexpected requests (Like for favicons), this should be safe, but should
we have one out of paranoia?

[Takashi Toyoshima, 2016/12/05 06:39:13]

Thanks. I introduced request_log_lock_.

+
+  shell()->ShowDevTools();
+  ReloadBlockUntilNavigationsComplete(shell(), 1);
+
+  ASSERT_EQ(6UL, request_log_.size());
+  EXPECT_EQ(kReloadTestPath, request_log_[4].relative_url);
+  EXPECT_EQ(kMaxAgeCacheControl, request_log_[4].cache_control);
+  EXPECT_EQ(kReloadImagePath, request_log_[5].relative_url);
+  EXPECT_EQ(kNoCacheControl, request_log_[5].cache_control);
+
+  shell()->CloseDevTools();
+  ReloadBlockUntilNavigationsComplete(shell(), 1);
+
+  ASSERT_EQ(8UL, request_log_.size());
+  EXPECT_EQ(kReloadTestPath, request_log_[6].relative_url);
+  EXPECT_EQ(kMaxAgeCacheControl, request_log_[6].cache_control);
+  EXPECT_EQ(kReloadImagePath, request_log_[7].relative_url);
+  EXPECT_EQ(kNoCacheControl, request_log_[7].cache_control);
 }
 
 IN_PROC_BROWSER_TEST_F(ReloadCacheControlBrowserTest, BypassingReload) {
   GURL url(embedded_test_server()->GetURL(kReloadTestPath));
 
   EXPECT_TRUE(NavigateToURL(shell(), url));
   ReloadBypassingCacheBlockUntilNavigationsComplete(shell(), 1);
 
   ASSERT_EQ(4UL, request_log_.size());
   EXPECT_EQ(kReloadTestPath, request_log_[0].relative_url);
   EXPECT_EQ(kNoCacheControl, request_log_[0].cache_control);
   EXPECT_EQ(kReloadImagePath, request_log_[1].relative_url);
   EXPECT_EQ(kNoCacheControl, request_log_[1].cache_control);
 
   EXPECT_EQ(kReloadTestPath, request_log_[2].relative_url);
   EXPECT_EQ(kNoCacheCacheControl, request_log_[2].cache_control);
   EXPECT_EQ(kReloadImagePath, request_log_[3].relative_url);
   EXPECT_EQ(kNoCacheCacheControl, request_log_[3].cache_control);
+
+  shell()->ShowDevTools();
+  ReloadBypassingCacheBlockUntilNavigationsComplete(shell(), 1);
+
+  ASSERT_EQ(6UL, request_log_.size());
+  EXPECT_EQ(kReloadTestPath, request_log_[4].relative_url);
+  EXPECT_EQ(kNoCacheCacheControl, request_log_[4].cache_control);
+  EXPECT_EQ(kReloadImagePath, request_log_[5].relative_url);
+  EXPECT_EQ(kNoCacheCacheControl, request_log_[5].cache_control);
+
+  shell()->CloseDevTools();
+  ReloadBypassingCacheBlockUntilNavigationsComplete(shell(), 1);
+
+  ASSERT_EQ(8UL, request_log_.size());
+  EXPECT_EQ(kReloadTestPath, request_log_[6].relative_url);
+  EXPECT_EQ(kNoCacheCacheControl, request_log_[6].cache_control);
+  EXPECT_EQ(kReloadImagePath, request_log_[7].relative_url);
+  EXPECT_EQ(kNoCacheCacheControl, request_log_[7].cache_control);
 }
 
-// TODO(toyoshim): Add another set of reload tests with DevTools open.
+IN_PROC_BROWSER_TEST_F(ReloadCacheControlBrowserTest, NavigateToSame) {
+  GURL url(embedded_test_server()->GetURL(kReloadTestPath));
+
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+
+  ASSERT_EQ(4UL, request_log_.size());
+
+  // The first navigation is just a normal load.
+  EXPECT_EQ(kReloadTestPath, request_log_[0].relative_url);
+  EXPECT_EQ(kNoCacheControl, request_log_[0].cache_control);
+  EXPECT_EQ(kReloadImagePath, request_log_[1].relative_url);
+  EXPECT_EQ(kNoCacheControl, request_log_[1].cache_control);

[mmenke, 2016/12/02 20:16:49]

I assume we have plenty of tests for has navigations (i.e. that they don't
trigger a reload).

[Takashi Toyoshima, 2016/12/05 06:39:13]

Yes, PlzNavigate team have many navigation related tests. So, here I just focus
on testing the same page navigation as a kind of reload variants.

+
+  // The second navigation is the same page navigation. This should be handled
+  // as a reload, revalidating the main resource, but following cache protocols
+  // for others.
+  EXPECT_EQ(kReloadTestPath, request_log_[2].relative_url);
+  EXPECT_EQ(kMaxAgeCacheControl, request_log_[2].cache_control);
+  EXPECT_EQ(kReloadImagePath, request_log_[3].relative_url);
+  EXPECT_EQ(kNoCacheControl, request_log_[3].cache_control);
+
+  shell()->ShowDevTools();
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+
+  ASSERT_EQ(6UL, request_log_.size());
+  EXPECT_EQ(kReloadTestPath, request_log_[4].relative_url);
+  EXPECT_EQ(kMaxAgeCacheControl, request_log_[4].cache_control);
+  EXPECT_EQ(kReloadImagePath, request_log_[5].relative_url);
+  EXPECT_EQ(kNoCacheControl, request_log_[5].cache_control);
+
+  shell()->CloseDevTools();
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+
+  ASSERT_EQ(8UL, request_log_.size());
+  EXPECT_EQ(kReloadTestPath, request_log_[6].relative_url);
+  EXPECT_EQ(kMaxAgeCacheControl, request_log_[6].cache_control);
+  EXPECT_EQ(kReloadImagePath, request_log_[7].relative_url);
+  EXPECT_EQ(kNoCacheControl, request_log_[7].cache_control);
+}
 
 }  // namespace
 
 }  // namespace content