Changing URL match method to support fragments.

chrome/browser/prerender/prerender_browsertest.cc

 // Copyright (c) 2011 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 "base/command_line.h"
 #include "base/path_service.h"
 #include "base/string_util.h"
 #include "chrome/browser/prerender/prerender_contents.h"
@@ skipping 150 matching lines @@
   void PrerenderTestURL(
       const std::string& html_file,
       const std::deque<FinalStatus>& expected_final_status_queue,
       int total_navigations) {
     PrerenderTestURLImpl(html_file,
                          expected_final_status_queue,
                          total_navigations);
   }
 
   void NavigateToDestURL() const {
-    ui_test_utils::NavigateToURL(browser(), dest_url_);
+    NavigateToURLImpl(dest_url_);
+  }
 
-    // Make sure the PrerenderContents found earlier was used or removed
-    EXPECT_TRUE(prerender_manager()->FindEntry(dest_url_) == NULL);
-
-    // Check if page behaved as expected when actually displayed.
-    bool display_test_result = false;
-    ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
-        browser()->GetSelectedTabContents()->render_view_host(), L"",
-        L"window.domAutomationController.send(DidDisplayPass())",
-        &display_test_result));
-    EXPECT_TRUE(display_test_result);
+  // Should be const but test_server()->GetURL(...) is not const.
+  void NavigateToURL(const std::string& dest_html_file) {

[cbentzel, 2011/04/22 19:49:34]

Note: I use set_dest_url in other tests for this. Would that be sufficient? 

[dominich, 2011/04/22 20:48:11]

That's subtly different. Here I'm navigating to a new URL but the CHECK that
calls FindEntry is checking against the old URL.

Is it possible that where you've used set_dest_url() you didn't want to change
that CHECK?

[cbentzel, 2011/04/22 21:01:16]

OK, I see your point.

To capture this, I'd recommend doing a CHECK before the navigation to make sure
that FindEntry is non-NULL on dest_url_, do the navigation, and then test again
that it FindEntry is NULL. In the fragment mismatch case this will work due to
your predicate function.

Either way, I want either the NavigateToURL with an overrideable URL, or
set_dest_url, not both. If you keep this one, I'd suggest just passing in a
const-ref-GURL rather than constructing one.

+    const GURL dest_url = test_server()->GetURL(dest_html_file);
+    NavigateToURLImpl(dest_url);
   }
 
   bool UrlIsInPrerenderManager(const std::string& html_file) {
     GURL dest_url = test_server()->GetURL(html_file);
     return (prerender_manager()->FindEntry(dest_url) != NULL);
   }
 
   bool UrlIsInPrerenderManager(const GURL& url) {
     return (prerender_manager()->FindEntry(url) != NULL);
   }
@@ skipping 79 matching lines @@
         break;
       }
       default:
         // In the failure case, we should have removed dest_url_ from the
         // prerender_manager.
         EXPECT_TRUE(prerender_contents == NULL);
         break;
     }
   }
 
+  void NavigateToURLImpl(const GURL& dest_url) const {
+    ui_test_utils::NavigateToURL(browser(), dest_url);
+
+    // Make sure the PrerenderContents found earlier was used or removed
+    EXPECT_TRUE(prerender_manager()->FindEntry(dest_url_) == NULL);
+
+    // Check if page behaved as expected when actually displayed.
+    bool display_test_result = false;
+    ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
+        browser()->GetSelectedTabContents()->render_view_host(), L"",
+        L"window.domAutomationController.send(DidDisplayPass())",
+        &display_test_result));
+    EXPECT_TRUE(display_test_result);
+  }
+
   PrerenderManager* prerender_manager() const {
     Profile* profile = browser()->GetSelectedTabContents()->profile();
     PrerenderManager* prerender_manager = profile->GetPrerenderManager();
     return prerender_manager;
   }
 
   WaitForLoadPrerenderContentsFactory* prc_factory_;
   GURL dest_url_;
   bool use_https_src_server_;
 };
@@ skipping 371 matching lines @@
 }
 
 // Checks that scripts can retrieve the correct window size while prerendering.
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderWindowSize) {
   PrerenderTestURL("files/prerender/prerender_size.html",
                    FINAL_STATUS_USED,
                    1);
   NavigateToDestURL();
 }
 
+// Checks that we correctly use a prerendered page when navigating to a
+// fragment.
+IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderPageNavigateFragment) {
+  PrerenderTestURL("files/prerender/prerender_fragment.html",
+                   FINAL_STATUS_USED,
+                   1);
+  NavigateToURL("files/prerender/prerender_fragment.html#fragment");
+}
+
+// Checks that we correctly use a prerendered page when we prerender a fragment
+// but navigate to the main page.

[cbentzel, 2011/04/22 19:49:34]

Can you add a test for prerendering with one fragment and navigating to another
fragment?

[dominich, 2011/04/22 20:48:11]

Done.

+IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderFragmentNavigatePage) {
+  PrerenderTestURL("files/prerender/prerender_page.html#fragment",
+                   FINAL_STATUS_USED,
+                   1);
+  NavigateToURL("files/prerender/prerender_page.html");
+}
+
 }  // namespace prerender