Better test coverage for prerender sessionStorage merging.

chrome/browser/prerender/prerender_browsertest.cc

 // Copyright (c) 2012 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 <deque>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
@@ skipping 874 matching lines @@
     PrerenderTestURLImpl(prerender_url, destination_url,
                          expected_final_status_queue,
                          expected_number_of_loads,
                          false);
   }
 
   void NavigateToDestURL() const {
     NavigateToDestURLWithDisposition(CURRENT_TAB, true);
   }
 
-  void NavigateToDestURLInNewTab() const {
-    // First, open a new tab.
-    ui_test_utils::NavigateToURLWithDisposition(
-        current_browser(), GURL(content::kAboutBlankURL),
-        NEW_FOREGROUND_TAB,
-        ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
-    // Next, navigate to the destination URL. The swap-in will not succeed,
-    // due to session storage namespace mismatch. The merge is only kicked off
-    // asynchronously.
-    NavigateToDestURLWithDisposition(CURRENT_TAB, false);
-  }
-
   // Opens the url in a new tab, with no opener.
   void NavigateToDestURLWithDisposition(
       WindowOpenDisposition disposition,
       bool expect_swap_to_succeed) const {
     NavigateToURLWithParams(
         content::OpenURLParams(dest_url_, Referrer(), disposition,
                                content::PAGE_TRANSITION_TYPED, false),
         expect_swap_to_succeed);
   }
 
@@ skipping 2528 matching lines @@
 // prerendered page tries to make a second navigation entry.
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderNewNavigationEntry) {
   PrerenderTestURL("files/prerender/prerender_new_entry.html",
                    FINAL_STATUS_NEW_NAVIGATION_ENTRY,
                    1);
 }
 
 // Attempt a swap-in in a new tab, verifying that session storage namespace
 // merging works.
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderPageNewTab) {
-  PrerenderTestURL("files/prerender/prerender_page.html", FINAL_STATUS_USED, 1);
-  NavigateToDestURLInNewTab();
+  PrerenderTestURL("files/prerender/prerender_session_storage.html",
+                   FINAL_STATUS_USED, 1);
+
+  // Open a new tab to navigate in.
+  ui_test_utils::NavigateToURLWithDisposition(
+      current_browser(),
+      test_server()->GetURL("files/prerender/init_session_storage.html"),
+      NEW_FOREGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+
+  // Now navigate in the new tab. Set expect_swap_to_succeed to false because
+  // the swap does not occur synchronously.

[mmenke, 2014/01/09 16:17:01]

Now that we have the swap_observer, can't we just expect the swap to succeed? 
In fact, without the swap observer, these tests could fail due to the
asynchronous swap, no?

[mmenke, 2014/01/09 16:20:27]

Oh, is the issue that OpenURL() returns the wrong WebContents?  If that's the
case, I think that's very confusing, and wonder if we could do better, since
passing false for a swap that does in fact succeed before the function returns
is really weird.

Maybe have the swap_observer get the right web_contents?

[davidben, 2014/01/09 17:39:15]

Yeah, it's really confusing. This isn't new from this CL though;
NavigateToDestURLInNewTab also did the same thing. (It never even called
DidDisplayPass.) My plan was to, when making all prerenders asynchronous for
beforeunload purposes, remove that WebContents check and then this can be a
normal NavigateToDestURL.

I can add a TODO to that effect or even just punt on the WebContents assert
right now if you think that's preferable. Doing it in the observer would be a
little screwy since it's not whether the swap happens or not. It's whether the
swap happens before or after OpenURL returns.

[mmenke, 2014/01/09 21:48:35]

The watcher would always be able to get the right WebContents, though, so you
could always just get it from that.

Regardless, I think it's fine to add a TODO, since this was pre-existing
behavior.

[davidben, 2014/01/09 21:51:49]

Added TODO.

+  NavigateToDestURLWithDisposition(CURRENT_TAB, false);
+
+  // Verify DidDisplayPass manually since the previous call skipped it.
+  EXPECT_TRUE(DidDisplayPass(
+      current_browser()->tab_strip_model()->GetActiveWebContents()));
+}
+
+// Verify that session storage conflicts don't merge.
+IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderSessionStorageConflict) {
+  PrerenderTestURL("files/prerender/prerender_session_storage_conflict.html",
+                   FINAL_STATUS_APP_TERMINATING, 1);
+
+  // Open a new tab to navigate in.
+  ui_test_utils::NavigateToURLWithDisposition(
+      current_browser(),
+      test_server()->GetURL("files/prerender/init_session_storage.html"),
+      NEW_FOREGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+
+  // Now navigate in the new tab.
+  NavigateToDestURLWithDisposition(CURRENT_TAB, false);
+
+  // Verify DidDisplayPass in the new tab.
+  EXPECT_TRUE(DidDisplayPass(
+      current_browser()->tab_strip_model()->GetActiveWebContents()));
 }
 
 // Checks that prerenders honor |should_replace_current_entry|.
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderReplaceCurrentEntry) {
   PrerenderTestURL("files/prerender/prerender_page.html", FINAL_STATUS_USED, 1);
 
   content::OpenURLParams params(dest_url(), Referrer(), CURRENT_TAB,
                                 content::PAGE_TRANSITION_TYPED, false);
   params.should_replace_current_entry = true;
   NavigateToURLWithParams(params, false);
 
   WebContents* web_contents =
       current_browser()->tab_strip_model()->GetActiveWebContents();
   const NavigationController& controller = web_contents->GetController();
   // First entry is about:blank, second is prerender_page.html.
   EXPECT_TRUE(controller.GetPendingEntry() == NULL);
   EXPECT_EQ(2, controller.GetEntryCount());
   EXPECT_EQ(GURL(content::kAboutBlankURL),
             controller.GetEntryAtIndex(0)->GetURL());
   EXPECT_EQ(dest_url(), controller.GetEntryAtIndex(1)->GetURL());
 }
 
 }  // namespace prerender