Revert "Fix issues with DOM UI that cause focus to not be set to the URL bar and the"

This reverts commit r12668 because it failed to compile on Mac.

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@12673 0039d316-1c4b-4281-b951-d872f2087c98

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.