Browser Test (Prerendering) for Favicon

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 "base/test/test_timeouts.h"
 #include "chrome/browser/content_settings/host_content_settings_map.h"
+#include "chrome/browser/favicon/favicon_tab_helper.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/prerender/prerender_contents.h"
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/task_manager/task_manager.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
 #include "chrome/common/chrome_paths.h"
@@ skipping 353 matching lines @@
                                                         expected_final_status);
     PrerenderTestURLImpl(url, expected_final_status_queue, total_navigations);
   }
 
   void NavigateToDestURL() const {
     NavigateToURLImpl(dest_url_);
   }
 
   void OpenDestUrlInNewWindowViaJs() const {
     // Make sure in navigating we have a URL to use in the PrerenderManager.
-    TestPrerenderContents* prerender_contents =
-        static_cast<TestPrerenderContents*>(
-            prerender_manager()->FindEntry(dest_url_));
+    TestPrerenderContents* prerender_contents = GetPrerenderContents();
     ASSERT_TRUE(prerender_contents != NULL);
     prerender_contents->set_quit_message_loop_on_destruction(false);
 
     bool open_window_result = false;
     ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
         browser()->GetSelectedTabContents()->render_view_host(), L"",
         L"window.domAutomationController.send(JsOpenLinkInNewWindow())",
         &open_window_result));
     EXPECT_TRUE(open_window_result);
 
     // If the prerender contents has not been destroyed, run message loop.
-    if (prerender_manager()->FindEntry(dest_url_) != NULL) {
+    if (GetPrerenderContents() != NULL) {
       prerender_contents->set_quit_message_loop_on_destruction(true);
       ui_test_utils::RunMessageLoop();
     }
   }
 
   void OpenDestUrlInNewWindowViaClick() const {
     // Make sure in navigating we have a URL to use in the PrerenderManager.
-    TestPrerenderContents* prerender_contents =
-        static_cast<TestPrerenderContents*>(
-            prerender_manager()->FindEntry(dest_url_));
+    TestPrerenderContents* prerender_contents = GetPrerenderContents();
     ASSERT_TRUE(prerender_contents != NULL);
     prerender_contents->set_quit_message_loop_on_destruction(false);
 
     bool click_prerendered_link_result = false;
     ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
         browser()->GetSelectedTabContents()->render_view_host(), L"",
         L"window.domAutomationController.send(ClickOpenLinkInNewWindow())",
         &click_prerendered_link_result));
     EXPECT_TRUE(click_prerendered_link_result);
 
     // If the prerender contents has not been destroyed, run message loop.
-    if (prerender_manager()->FindEntry(dest_url_) != NULL) {
+    if (GetPrerenderContents() != NULL) {
       prerender_contents->set_quit_message_loop_on_destruction(true);
       ui_test_utils::RunMessageLoop();
     }
   }
 
   // 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);
   }
@@ skipping 27 matching lines @@
   PrerenderManager* prerender_manager() const {
     Profile* profile = browser()->GetSelectedTabContents()->profile();
     PrerenderManager* prerender_manager = profile->GetPrerenderManager();
     return prerender_manager;
   }
 
   FakeSafeBrowsingService* GetSafeBrowsingService() {
     return safe_browsing_factory_->most_recent_service();
   }
 
+  TestPrerenderContents* GetPrerenderContents() const {
+    return static_cast<TestPrerenderContents*>(
+        prerender_manager()->FindEntry(dest_url_));
+  }
+
  private:
   void PrerenderTestURLImpl(
       const GURL& url,
       const std::deque<FinalStatus>& expected_final_status_queue,
       int total_navigations) {
     dest_url_ = url;
 
     std::vector<net::TestServer::StringPair> replacement_text;
     replacement_text.push_back(
         make_pair("REPLACE_WITH_PRERENDER_URL", dest_url_.spec()));
@@ skipping 25 matching lines @@
     prerender_manager()->SetPrerenderContentsFactory(
         prerender_contents_factory_);
     FinalStatus expected_final_status = expected_final_status_queue.front();
 
     // ui_test_utils::NavigateToURL uses its own observer and message loop.
     // Since the test needs to wait until the prerendered page has stopped
     // loading, rather than the page directly navigated to, need to
     // 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_));
+    TestPrerenderContents* prerender_contents = GetPrerenderContents();
 
     if (ShouldRenderPrerenderedPageCorrectly(expected_final_status)) {
       ASSERT_TRUE(prerender_contents != NULL);
       EXPECT_EQ(FINAL_STATUS_MAX, prerender_contents->final_status());
 
       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);
       }
     } else {
       // In the failure case, we should have removed dest_url_ from the
       // prerender_manager.
       EXPECT_TRUE(prerender_contents == NULL);
     }
   }
 
   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);
+    EXPECT_TRUE(GetPrerenderContents() != 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
     // loaded, there is no race between loading |dest_url| and swapping the
     // prerendered TabContents into the tab.
     browser()->OpenURL(dest_url, GURL(), CURRENT_TAB, PageTransition::TYPED);
     ui_test_utils::RunMessageLoop();
 
     // Make sure the PrerenderContents found earlier was used or removed.
-    EXPECT_TRUE(prerender_manager()->FindEntry(dest_url_) == NULL);
+    EXPECT_TRUE(GetPrerenderContents() == NULL);
 
     if (call_javascript_) {
       // 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);
     }
@@ skipping 766 matching lines @@
 }
 
 // Checks that a local storage write will not cause prerender to fail.
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderLocalStorageWrite) {
   PrerenderTestURL("files/prerender/prerender_localstorage_write.html",
                    FINAL_STATUS_USED,
                    1);
   NavigateToDestURL();
 }
 
+// Checks that the favicon is properly loaded on prerender.
+IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderFavicon) {
+  PrerenderTestURL("files/prerender/prerender_favicon.html",
+                   FINAL_STATUS_USED,
+                   1);
+  TestPrerenderContents* prerender_contents = GetPrerenderContents();
+  ASSERT_TRUE(prerender_contents != NULL);
+  prerender_contents->set_quit_message_loop_on_destruction(false);
+  // The Favicon should show within two seconds of navigating to the page,
+  // otherwise something is wrong.
+  MessageLoopForUI::current()->PostDelayedTask(

[cbentzel, 2011/06/09 15:41:04]

I agree with Pawel that this is pretty hairy. 

Could you do something like the TitleWatcher in
http://codereview.chromium.org/7046053/ which listens for the FAVICON_CHANGED
event instead? 

[Paweł Hajdan Jr., 2011/06/09 19:29:20]

Yes, that's what I'd like to see. Please do that. Also, I'm slightly annoyed
that the change got committed with unresolved comments.

Just a note: my comments about this code are not about flakiness, at least not
directly. Waiting for a fixed amount of time and waiting for an event with a
possibly similar timeout are essentially equally solid.

However, waiting for a fixed amount of time is just wrong. It makes the test
unnecessarily slow, and might encourage a bad test writing style. It does
happen, and it does matter. When most Sleeps disappeared from the codebase,
people stopped adding them to new tests (yes, I'm just watching _every_ test
change, so this is all based on facts and data). This PostDelayedTask is
essentially a Sleep, and that's what makes me unhappy.

Finally, thank you for dealing with flaky tests - I appreciate that effort very
much, and the goal of my comments is just to further increase the quality of the
tests.

+      FROM_HERE,
+      new MessageLoop::QuitTask(),
+      TestTimeouts::action_timeout_ms());
+  NavigateToDestURL();
+  ASSERT_TRUE(TabContentsWrapper::GetCurrentWrapperForContents(
+      browser()->GetSelectedTabContents())->favicon_tab_helper()
+              ->FaviconIsValid());
+}
+
 }  // namespace prerender