Reload cache control test: add more tests

content/browser/loader/reload_cache_control_browsertest.cc

Index: content/browser/loader/reload_cache_control_browsertest.cc
diff --git a/content/browser/loader/reload_cache_control_browsertest.cc b/content/browser/loader/reload_cache_control_browsertest.cc
index 5affe6942b66386a7170ea7ad9dda39711ed3f09..bace4aed5ad17eee8744a7a169fb05ab78b53dcf 100644
--- a/content/browser/loader/reload_cache_control_browsertest.cc
+++ b/content/browser/loader/reload_cache_control_browsertest.cc
@@ -88,6 +88,24 @@ IN_PROC_BROWSER_TEST_F(ReloadCacheControlBrowserTest, NormalReload) {
   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) {
@@ -106,9 +124,66 @@ IN_PROC_BROWSER_TEST_F(ReloadCacheControlBrowserTest, BypassingReload) {
   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