3 files changed, 153 insertions, 200 deletions
diff --git a/chrome/browser/dom_ui/dom_ui_unittest.cc b/chrome/browser/dom_ui/dom_ui_unittest.cc
index c7320b0..40a5617 100644
--- a/chrome/browser/dom_ui/dom_ui_unittest.cc
+++ b/chrome/browser/dom_ui/dom_ui_unittest.cc
@@ -1,133 +1,116 @@
-// Copyright (c) 2009 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 "chrome/browser/dom_ui/new_tab_ui.h"
-#include "chrome/browser/renderer_host/test_render_view_host.h"
-#include "chrome/common/url_constants.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-class DOMUITest : public RenderViewHostTestHarness {
- public:
-  DOMUITest() {}
-
-  // Tests navigating with a DOM UI from a fresh (nothing pending or committed)
-  // state, through pending, committed, then another navigation. The first page
-  // ID that we should use is passed as a parameter. We'll use the next two
-  // values. This must be increasing for the life of the tests.
-  static void DoNavigationTest(WebContents* contents, int page_id) {
-    NavigationController* controller = contents->controller();
-
-    // Start a pending load.
-    GURL new_tab_url(chrome::kChromeUINewTabURL);
-    controller->LoadURL(new_tab_url, GURL(), PageTransition::LINK);
-
-    // The navigation entry should be pending with no committed entry.
-    ASSERT_TRUE(controller->GetPendingEntry());
-    ASSERT_FALSE(controller->GetLastCommittedEntry());
-
-    // Check the things the pending DOM UI should have set.
-    EXPECT_FALSE(contents->ShouldDisplayURL());
-    EXPECT_FALSE(contents->ShouldDisplayFavIcon());
-    EXPECT_TRUE(contents->IsBookmarkBarAlwaysVisible());
-    EXPECT_TRUE(contents->FocusLocationBarByDefault());
-
-    // Now commit the load.
-    static_cast<TestRenderViewHost*>(
-        contents->render_view_host())->SendNavigate(page_id, new_tab_url);
-
-    // The same flags should be set as before now that the load has committed.
-    EXPECT_FALSE(contents->ShouldDisplayURL());
-    EXPECT_FALSE(contents->ShouldDisplayFavIcon());
-    EXPECT_TRUE(contents->IsBookmarkBarAlwaysVisible());
-    EXPECT_TRUE(contents->FocusLocationBarByDefault());
-
-    // Start a pending navigation to a regular page.
-    GURL next_url("http://google.com/");
-    controller->LoadURL(next_url, GURL(), PageTransition::LINK);
-
-    // Check the flags. Some should reflect the new page (URL, title), some
-    // should reflect the old one (bookmark bar) until it has committed.
-    EXPECT_TRUE(contents->ShouldDisplayURL());
-    EXPECT_TRUE(contents->ShouldDisplayFavIcon());
-    EXPECT_TRUE(contents->IsBookmarkBarAlwaysVisible());
-    EXPECT_FALSE(contents->FocusLocationBarByDefault());
-
-    // Commit the regular page load. Note that we must send it to the "pending"
-    // RenderViewHost, since this transition will also cause a process
-    // transition, and our RVH pointer will be the "committed" one.
-    static_cast<TestRenderViewHost*>(
-        contents->render_manager()->pending_render_view_host())->SendNavigate(
-            page_id + 1, next_url);
-
-    // The state should now reflect a regular page.
-    EXPECT_TRUE(contents->ShouldDisplayURL());
-    EXPECT_TRUE(contents->ShouldDisplayFavIcon());
-    EXPECT_FALSE(contents->IsBookmarkBarAlwaysVisible());
-    EXPECT_FALSE(contents->FocusLocationBarByDefault());
-  }
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(DOMUITest);
-};
-
-// Tests that the New Tab Page flags are correctly set and propogated by
-// WebContents when we first navigate to a DOM UI page, then to a standard
-// non-DOM-UI page.
-TEST_F(DOMUITest, DOMUIToStandard) {
-  DoNavigationTest(contents(), 1);
-
-  // Make 
-  WebContents* contents2 = new TestWebContents(profile_.get(), NULL,
-      &rvh_factory_);
-  NavigationController* controller2 = new NavigationController(contents2,
-                                                               profile_.get());
-  DoNavigationTest(contents2, 101);
-  contents2->CloseContents();
-}
-
-TEST_F(DOMUITest, DOMUIToDOMUI) {
-  // Do a load (this state is tested above).
-  GURL new_tab_url(chrome::kChromeUINewTabURL);
-  controller()->LoadURL(new_tab_url, GURL(), PageTransition::LINK);
-  rvh()->SendNavigate(1, new_tab_url);
-
-  // Start another pending load of the new tab page.
-  controller()->LoadURL(new_tab_url, GURL(), PageTransition::LINK);
-  rvh()->SendNavigate(2, new_tab_url);
-
-  // The flags should be the same as the non-pending state.
-  EXPECT_FALSE(contents()->ShouldDisplayURL());
-  EXPECT_FALSE(contents()->ShouldDisplayFavIcon());
-  EXPECT_TRUE(contents()->IsBookmarkBarAlwaysVisible());
-  EXPECT_TRUE(contents()->FocusLocationBarByDefault());
-}
-
-TEST_F(DOMUITest, StandardToDOMUI) {
-  // Start a pending navigation to a regular page.
-  GURL std_url("http://google.com/");
-  controller()->LoadURL(std_url, GURL(), PageTransition::LINK);
-
-  // The state should now reflect the default.
-  EXPECT_TRUE(contents()->ShouldDisplayURL());
-  EXPECT_TRUE(contents()->ShouldDisplayFavIcon());
-  EXPECT_FALSE(contents()->IsBookmarkBarAlwaysVisible());
-  EXPECT_FALSE(contents()->FocusLocationBarByDefault());
-
-  // Commit the load, the state should be the same.
-  rvh()->SendNavigate(1, std_url);
-  EXPECT_TRUE(contents()->ShouldDisplayURL());
-  EXPECT_TRUE(contents()->ShouldDisplayFavIcon());
-  EXPECT_FALSE(contents()->IsBookmarkBarAlwaysVisible());
-  EXPECT_FALSE(contents()->FocusLocationBarByDefault());
-
-  // Start a pending load for a DOMUI.
-  GURL new_tab_url(chrome::kChromeUINewTabURL);
-  controller()->LoadURL(new_tab_url, GURL(), PageTransition::LINK);
-  EXPECT_FALSE(contents()->ShouldDisplayURL());
-  EXPECT_FALSE(contents()->ShouldDisplayFavIcon());
-  EXPECT_FALSE(contents()->IsBookmarkBarAlwaysVisible());
-  EXPECT_TRUE(contents()->FocusLocationBarByDefault());
-
-  // Committing DOM UI is tested above.
-}
+// Copyright (c) 2009 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 "chrome/browser/dom_ui/new_tab_ui.h"
+#include "chrome/browser/renderer_host/test_render_view_host.h"
+#include "chrome/common/url_constants.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class DOMUITest : public RenderViewHostTestHarness {
+ public:
+  DOMUITest() {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(DOMUITest);
+};
+
+// Tests that the New Tab Page flags are correctly set and propogated by
+// WebContents when we first navigate to a DOM UI page, then to a standard
+// non-DOM-UI page.
+TEST_F(DOMUITest, DOMUIToStandard) {
+  // Start a pending load.
+  GURL new_tab_url(chrome::kChromeUINewTabURL);
+  controller()->LoadURL(new_tab_url, GURL(), PageTransition::LINK);
+
+  // The navigation entry should be pending with no committed entry.
+  ASSERT_TRUE(controller()->GetPendingEntry());
+  ASSERT_FALSE(controller()->GetLastCommittedEntry());
+
+  // Check the things the pending DOM UI should have set.
+  EXPECT_FALSE(contents()->ShouldDisplayURL());
+  EXPECT_FALSE(contents()->ShouldDisplayFavIcon());
+  EXPECT_TRUE(contents()->IsBookmarkBarAlwaysVisible());
+  EXPECT_TRUE(contents()->FocusLocationBarByDefault());
+
+  // Now commit the load.
+  rvh()->SendNavigate(1, new_tab_url);
+
+  // The same flags should be set as before now that the load has committed.
+  // Note that the location bar isn't focused now. Once the load commits, we
+  // don't care about this flag, so this value is OK.
+  EXPECT_FALSE(contents()->ShouldDisplayURL());
+  EXPECT_FALSE(contents()->ShouldDisplayFavIcon());
+  EXPECT_TRUE(contents()->IsBookmarkBarAlwaysVisible());
+  EXPECT_FALSE(contents()->FocusLocationBarByDefault());
+
+  // Start a pending navigation to a regular page.
+  GURL next_url("http://google.com/");
+  controller()->LoadURL(next_url, GURL(), PageTransition::LINK);
+
+  // Check the flags. Some should reflect the new page (URL, title), some should
+  // reflect the old one (bookmark bar) until it has committed.
+  EXPECT_TRUE(contents()->ShouldDisplayURL());
+  EXPECT_TRUE(contents()->ShouldDisplayFavIcon());
+  EXPECT_TRUE(contents()->IsBookmarkBarAlwaysVisible());
+  EXPECT_FALSE(contents()->FocusLocationBarByDefault());
+
+  // Commit the regular page load. Note that we must send it to the "pending"
+  // RenderViewHost, since this transition will also cause a process transition,
+  // and our RVH pointer will be the "committed" one.
+  static_cast<TestRenderViewHost*>(
+      contents()->render_manager()->pending_render_view_host())->SendNavigate(
+          2, next_url);
+
+  // The state should now reflect a regular page.
+  EXPECT_TRUE(contents()->ShouldDisplayURL());
+  EXPECT_TRUE(contents()->ShouldDisplayFavIcon());
+  EXPECT_FALSE(contents()->IsBookmarkBarAlwaysVisible());
+  EXPECT_FALSE(contents()->FocusLocationBarByDefault());
+}
+
+TEST_F(DOMUITest, DOMUIToDOMUI) {
+  // Do a load (this state is tested above).
+  GURL new_tab_url(chrome::kChromeUINewTabURL);
+  controller()->LoadURL(new_tab_url, GURL(), PageTransition::LINK);
+  rvh()->SendNavigate(1, new_tab_url);
+
+  // Start another pending load of the new tab page.
+  controller()->LoadURL(new_tab_url, GURL(), PageTransition::LINK);
+  rvh()->SendNavigate(2, new_tab_url);
+
+  // The flags should be the same as the non-pending state.
+  EXPECT_FALSE(contents()->ShouldDisplayURL());
+  EXPECT_FALSE(contents()->ShouldDisplayFavIcon());
+  EXPECT_TRUE(contents()->IsBookmarkBarAlwaysVisible());
+  EXPECT_FALSE(contents()->FocusLocationBarByDefault());
+}
+
+TEST_F(DOMUITest, StandardToDOMUI) {
+  // Start a pending navigation to a regular page.
+  GURL std_url("http://google.com/");
+  controller()->LoadURL(std_url, GURL(), PageTransition::LINK);
+
+  // The state should now reflect the default.
+  EXPECT_TRUE(contents()->ShouldDisplayURL());
+  EXPECT_TRUE(contents()->ShouldDisplayFavIcon());
+  EXPECT_FALSE(contents()->IsBookmarkBarAlwaysVisible());
+  EXPECT_FALSE(contents()->FocusLocationBarByDefault());
+
+  // Commit the load, the state should be the same.
+  rvh()->SendNavigate(1, std_url);
+  EXPECT_TRUE(contents()->ShouldDisplayURL());
+  EXPECT_TRUE(contents()->ShouldDisplayFavIcon());
+  EXPECT_FALSE(contents()->IsBookmarkBarAlwaysVisible());
+  EXPECT_FALSE(contents()->FocusLocationBarByDefault());
+
+  // Start a pending load for a DOMUI.
+  GURL new_tab_url(chrome::kChromeUINewTabURL);
+  controller()->LoadURL(new_tab_url, GURL(), PageTransition::LINK);
+  EXPECT_FALSE(contents()->ShouldDisplayURL());
+  EXPECT_FALSE(contents()->ShouldDisplayFavIcon());
+  EXPECT_FALSE(contents()->IsBookmarkBarAlwaysVisible());
+  EXPECT_TRUE(contents()->FocusLocationBarByDefault());
+
+  // Committing DOM UI is tested above.
+}
diff --git a/chrome/browser/tab_contents/web_contents.cc b/chrome/browser/tab_contents/web_contents.cc
index 7ba7c1a..6a8b721 100644
--- a/chrome/browser/tab_contents/web_contents.cc
+++ b/chrome/browser/tab_contents/web_contents.cc
@@ -363,16 +363,30 @@ SiteInstance* WebContents::GetSiteInstance() const {
 }
 
 bool WebContents::ShouldDisplayURL() {
-  DOMUI* dom_ui = GetDOMUIForCurrentState();
-  if (dom_ui)
-    return !dom_ui->should_hide_url();
+  if (controller()->GetPendingEntry()) {
+    // When there is a pending entry, that should determine whether the URL is
+    // displayed (including getting the default behavior if the DOMUI doesn't
+    // specify).
+    if (render_manager_.pending_dom_ui())
+      return !render_manager_.pending_dom_ui()->should_hide_url();
+    return true;
+  }
+
+  if (render_manager_.dom_ui())
+    return !render_manager_.dom_ui()->should_hide_url();
   return true;
 }
 
 bool WebContents::ShouldDisplayFavIcon() {
-  DOMUI* dom_ui = GetDOMUIForCurrentState();
-  if (dom_ui)
-    return !dom_ui->hide_favicon();
+  if (controller()->GetPendingEntry()) {
+    // See ShouldDisplayURL.
+    if (render_manager_.pending_dom_ui())
+      return !render_manager_.pending_dom_ui()->hide_favicon();
+    return true;
+  }
+  
+  if (render_manager_.dom_ui())
+    return !render_manager_.dom_ui()->hide_favicon();
   return true;
 }
 
@@ -518,24 +532,21 @@ void WebContents::HideContents() {
 }
 
 bool WebContents::IsBookmarkBarAlwaysVisible() {
-  // See GetDOMUIForCurrentState() comment for more info. This case is very
-  // similar, but for non-first loads, we want to use the committed entry. This
-  // is so the bookmarks bar disappears at the same time the page does.
-  if (controller()->GetLastCommittedEntry()) {
-    // Not the first load, always use the committed DOM UI.
-    if (render_manager_.dom_ui())
-      return render_manager_.dom_ui()->force_bookmark_bar_visible();
-    return false;  // Default.
+  // We want the bookmarks bar to go with the committed entry. This way, when
+  // you're on the new tab page and navigate, the bookmarks bar doesn't
+  // disappear until the next load commits (the same time the page changes).
+  if (!controller()->GetLastCommittedEntry()) {
+    // However, when there is no committed entry (the first load of the tab),
+    // then we fall back on the pending entry. This means that the bookmarks bar
+    // will be visible before the new tab page load commits.
+    if (render_manager_.pending_dom_ui())
+      return render_manager_.pending_dom_ui()->force_bookmark_bar_visible();
+    return false;
   }
 
-  // When it's the first load, we know either the pending one or the committed
-  // one will have the DOM UI in it (see GetDOMUIForCurrentState), and only one
-  // of them will be valid, so we can just check both.
-  if (render_manager_.pending_dom_ui())
-    return render_manager_.pending_dom_ui()->force_bookmark_bar_visible();
   if (render_manager_.dom_ui())
     return render_manager_.dom_ui()->force_bookmark_bar_visible();
-  return false;  // Default.
+  return false;
 }
 
 void WebContents::SetDownloadShelfVisible(bool visible) {
@@ -552,9 +563,12 @@ void WebContents::PopupNotificationVisibilityChanged(bool visible) {
 }
 
 bool WebContents::FocusLocationBarByDefault() {
-  DOMUI* dom_ui = GetDOMUIForCurrentState();
-  if (dom_ui)
-    return dom_ui->focus_location_bar_by_default();
+  // Allow the DOM UI to override the default. We use the pending DOM UI since
+  // that's what the user "just did" so should control what happens after they
+  // did it. Using the committed one would mean when they navigate from a DOMUI
+  // to a regular page, the location bar would be focused.
+  if (render_manager_.pending_dom_ui())
+    return render_manager_.pending_dom_ui()->focus_location_bar_by_default();
   return false;
 }
 
@@ -2004,43 +2018,3 @@ void WebContents::GenerateKeywordIfNecessary(
   new_url->set_safe_for_autoreplace(true);
   url_model->Add(new_url);
 }
-
-DOMUI* WebContents::GetDOMUIForCurrentState() {
-  // When there is a pending navigation entry, we want to use the pending DOMUI
-  // that goes along with it to control the basic flags. For example, we want to
-  // show the pending URL in the URL bar, so we want the display_url flag to
-  // be from the pending entry.
-  //
-  // The confusion comes because there are multiple possibilities for the
-  // initial load in a tab as a side effect of the way the RenderViewHostManager
-  // works.
-  //
-  //  - For the very first tab the load looks "normal". The new tab DOM UI is
-  //    the pending one, and we want it to apply here.
-  //
-  //  - For subsequent new tabs, they'll get a new SiteInstance which will then
-  //    get switched to the one previously associated with the new tab pages.
-  //    This switching will cause the manager to commit the RVH/DOMUI. So we'll
-  //    have a committed DOM UI in this case.
-  //
-  // This condition handles all of these cases:
-  //
-  //  - First load in first tab: no committed nav entry + pending nav entry +
-  //    pending dom ui:
-  //    -> Use pending DOM UI if any.
-  // 
-  //  - First load in second tab: no committed nav entry + pending nav entry +
-  //    no pending DOM UI:
-  //    -> Use the committed DOM UI if any.
-  //
-  //  - Second navigation in any tab: committed nav entry + pending nav entry:
-  //    -> Use pending DOM UI if any.
-  //
-  //  - Normal state with no load: committed nav entry + no pending nav entry:
-  //    -> Use committed DOM UI.
-  if (controller()->GetPendingEntry() &&
-      (controller()->GetLastCommittedEntry() ||
-       render_manager_.pending_dom_ui()))
-    return render_manager_.pending_dom_ui();
-  return render_manager_.dom_ui();
-}
diff --git a/chrome/browser/tab_contents/web_contents.h b/chrome/browser/tab_contents/web_contents.h
index 3407a93..58d4fb4 100644
--- a/chrome/browser/tab_contents/web_contents.h
+++ b/chrome/browser/tab_contents/web_contents.h
@@ -582,10 +582,6 @@ class WebContents : public TabContents,
   void GenerateKeywordIfNecessary(
       const ViewHostMsg_FrameNavigate_Params& params);
 
-  // Returns the DOMUI for the current state of the tab. This will either be
-  // the pending DOMUI, the committed DOMUI, or NULL.
-  DOMUI* GetDOMUIForCurrentState();
-
   // Data ----------------------------------------------------------------------
 
   // The corresponding view.