Changes to not use the prerendered contents when window.opener needs to be set.

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 31 matching lines @@
 std::string CreateClientRedirect(const std::string& dest_url) {
   const char* const kClientRedirectBase = "client-redirect?";
   return kClientRedirectBase + dest_url;
 }
 
 std::string CreateServerRedirect(const std::string& dest_url) {
   const char* const kServerRedirectBase = "server-redirect?";
   return kServerRedirectBase + dest_url;
 }
 
+// Returns true iff the final status is one in which the prerendered
+// page should prerender correctly. The page still may not be used.
+bool ShouldRenderPrerenderedPageCorrectly(FinalStatus status) {

[cbentzel, 2011/05/12 11:17:08]

Is this needed anymore? The prerendered page is destroyed with
FINAL_STATUS_WINDOW_OPENER now, so I think this could revert back to the old
behavior. 

[Shishir, 2011/05/12 22:05:18]

This is still needed as in the opener case, the prereder still has to happen
correctly. The prerender is only discarded on click.

On 2011/05/12 11:17:08, cbentzel wrote:

+  switch (status) {
+    case FINAL_STATUS_USED:
+    case FINAL_STATUS_WINDOW_OPENER:
+      return true;
+    default:
+      return false;
+  }
+}
+
 // PrerenderContents that stops the UI message loop on DidStopLoading().
 class TestPrerenderContents : public PrerenderContents {
  public:
   TestPrerenderContents(
       PrerenderManager* prerender_manager,
       Profile* profile,
       const GURL& url,
       const GURL& referrer,
       int number_of_loads,
       FinalStatus expected_final_status)
@@ skipping 20 matching lines @@
     // on quit.
     if (expected_final_status_ == FINAL_STATUS_APP_TERMINATING)
       expected_final_status_ = FINAL_STATUS_RENDERER_CRASHED;
 
     PrerenderContents::OnRenderViewGone(status, exit_code);
   }
 
   virtual void DidStopLoading() OVERRIDE {
     PrerenderContents::DidStopLoading();
     ++number_of_loads_;
-    if (expected_final_status_ == FINAL_STATUS_USED &&
+    if (ShouldRenderPrerenderedPageCorrectly(expected_final_status_) &&
         number_of_loads_ >= expected_number_of_loads_) {
       MessageLoopForUI::current()->Quit();
     } else if (expected_final_status_ == FINAL_STATUS_RENDERER_CRASHED) {
       // Crash the render process.  This has to be done here because
       // a prerender navigating to about:crash is cancelled with
       // "FINAL_STATUS_HTTPS".  Even if this were worked around,
       // about:crash can't be navigated to by a normal webpage.
       render_view_host_mutable()->NavigateToURL(GURL("about:crash"));
     }
   }
@@ skipping 101 matching lines @@
     ASSERT_TRUE(test_server()->Start());
     std::deque<FinalStatus> expected_final_status_queue(1,
                                                         expected_final_status);
     PrerenderTestURLImpl(url, expected_final_status_queue, total_navigations);
   }
 
   void NavigateToDestURL() const {
     NavigateToURLImpl(dest_url_);
   }
 
+  void OpenDestUrlInNewWindowViaJs() const {

[cbentzel, 2011/05/12 11:17:08]

In these two cases, you don't need to do the interface/Impl split like we did
for PrerenderTestURL or NavigateToURLImpl.

Thanks for trying to be consistent, though.

[Shishir, 2011/05/12 22:05:18]

Done.

+    OpenDestUrlInNewWindowViaJsImpl();
+  }
+
+  void OpenDestUrlInNewWindowViaClick() const {
+    OpenDestUrlInNewWindowViaClickImpl();
+  }
+
   // Should be const but test_server()->GetURL(...) is not const.
   void NavigateToURL(const std::string& dest_html_file) {
     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);
   }
@@ skipping 70 matching lines @@
     // handle browser navigation directly.
     browser()->OpenURL(src_url, GURL(), CURRENT_TAB, PageTransition::TYPED);
 
     TestPrerenderContents* prerender_contents = NULL;
     ui_test_utils::RunMessageLoop();
 
     prerender_contents =
         static_cast<TestPrerenderContents*>(
             prerender_manager()->FindEntry(dest_url_));
 
-    switch (expected_final_status) {
-      case FINAL_STATUS_USED: {
-        ASSERT_TRUE(prerender_contents != NULL);
+    if (ShouldRenderPrerenderedPageCorrectly(expected_final_status)) {
+      ASSERT_TRUE(prerender_contents != NULL);
+      EXPECT_EQ(prerender_contents->final_status(), FINAL_STATUS_MAX);

[sreeram, 2011/05/12 02:36:41]

Swap the arguments. In EXPECT_EQ, the first argument should be the expected
value.

[Shishir, 2011/05/12 22:05:18]

Done.

 
-        if (call_javascript_) {
-          // Check if page behaves as expected while in prerendered state.
-          bool prerender_test_result = false;
-          ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
-              prerender_contents->render_view_host_mutable(), L"",
-              L"window.domAutomationController.send(DidPrerenderPass())",
-              &prerender_test_result));
-          EXPECT_TRUE(prerender_test_result);
-        }
-        break;
+      if (call_javascript_) {
+        // Check if page behaves as expected while in prerendered state.
+        bool prerender_test_result = false;
+        ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
+            prerender_contents->render_view_host_mutable(), L"",
+            L"window.domAutomationController.send(DidPrerenderPass())",
+            &prerender_test_result));
+        EXPECT_TRUE(prerender_test_result);
       }
-      default:
-        // In the failure case, we should have removed dest_url_ from the
-        // prerender_manager.
-        EXPECT_TRUE(prerender_contents == NULL);
-        break;
+    } else {
+      // In the failure case, we should have removed dest_url_ from the
+      // prerender_manager.
+      EXPECT_TRUE(prerender_contents == NULL);
     }
   }
 
+  void OpenDestUrlInNewWindowViaClickImpl() const {
+    // Make sure in navigating we have a URL to use in the PrerenderManager.
+    EXPECT_TRUE(prerender_manager()->FindEntry(dest_url_) != NULL);
+
+    bool click_prerendered_link_result = false;
+    ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
+        browser()->GetSelectedTabContents()->render_view_host(), L"",
+        L"window.domAutomationController.send(ClickOpenPrerenderedLinkInNewWindow())",

[cbentzel, 2011/05/12 11:17:08]

Nit: 80 char-line. However, this may be more readable than the
split-the-literal-across-lines-approach.

I think you could just shorten the name of the JS function which is called,
especially since it's only in the context of some specific tests. Maybe
"ClickOpenLink" and "JsOpenLink"

[Shishir, 2011/05/12 22:05:18]

Done.

+        &click_prerendered_link_result));
+    EXPECT_TRUE(click_prerendered_link_result);
+  }
+
+  void OpenDestUrlInNewWindowViaJsImpl() const {
+    // Make sure in navigating we have a URL to use in the PrerenderManager.
+    EXPECT_TRUE(prerender_manager()->FindEntry(dest_url_) != NULL);
+
+    bool open_window_result = false;
+    ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
+        browser()->GetSelectedTabContents()->render_view_host(), L"",
+        L"window.domAutomationController.send(JsOpenPrerenderedLinkInNewWindow())",
+        &open_window_result));
+    EXPECT_TRUE(open_window_result);
+  }
+
   void NavigateToURLImpl(const GURL& dest_url) const {
     // Make sure in navigating we have a URL to use in the PrerenderManager.
     EXPECT_TRUE(prerender_manager()->FindEntry(dest_url_) != NULL);
 
     // ui_test_utils::NavigateToURL waits until DidStopLoading is called on
     // the current tab.  As that tab is going to end up deleted, and may never
     // finish loading before that happens, exit the message loop on the deletion
     // of the used prerender contents instead.
     //
     // As PrerenderTestURL waits until the prerendered page has completely
@@ skipping 609 matching lines @@
       std::make_pair("REPLACE_WITH_IMAGE_URL", https_url.spec()));
   std::string replacement_path;
   ASSERT_TRUE(net::TestServer::GetFilePathWithReplacements(
       "files/prerender/prerender_with_image.html",
       replacement_text,
       &replacement_path));
   PrerenderTestURL(replacement_path, FINAL_STATUS_USED, 1);
   NavigateToDestURL();
 }
 
+// Checks that if a page is opened in a new window by javascript the
+// prerendered page is not used.
+IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
+                       PrerenderWindowOpenerJsOpenInNewPageTest) {
+  PrerenderTestURL("files/prerender/prerender_page.html",
+                   FINAL_STATUS_WINDOW_OPENER,
+                   1);
+  OpenDestUrlInNewWindowViaJs();
+}
+
+// Checks that if a page is opened due to click on a href with target="_blank"
+// the prerendered page is not used.
+IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
+                       PrerenderWindowOpenerClickOpenInNewPageTest) {
+  PrerenderTestURL("files/prerender/prerender_page.html",
+                   FINAL_STATUS_WINDOW_OPENER,
+                   1);
+  OpenDestUrlInNewWindowViaClick();
+}
+
+// TODO(shishir): Add a test for the case when the page having the
+// prerendering link already has an opener set.
+
 }  // namespace prerender