diff options
| -rw-r--r-- | chrome/browser/dom_ui/dom_ui_unittest.cc | 111 | ||||
| -rw-r--r-- | chrome/browser/tab_contents/web_contents.cc | 100 | ||||
| -rw-r--r-- | chrome/browser/tab_contents/web_contents.h | 4 |
3 files changed, 130 insertions, 85 deletions
diff --git a/chrome/browser/dom_ui/dom_ui_unittest.cc b/chrome/browser/dom_ui/dom_ui_unittest.cc index e335293..2c69c1a 100644 --- a/chrome/browser/dom_ui/dom_ui_unittest.cc +++ b/chrome/browser/dom_ui/dom_ui_unittest.cc @@ -11,6 +11,62 @@ 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); }; @@ -19,54 +75,13 @@ class DOMUITest : public RenderViewHostTestHarness { // 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()); + DoNavigationTest(contents(), 1); - // 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()); + // Check for a non-first + WebContents* contents2 = new TestWebContents(profile_.get(), NULL, + &rvh_factory_); + DoNavigationTest(contents2, 101); + contents2->CloseContents(); } TEST_F(DOMUITest, DOMUIToDOMUI) { @@ -83,7 +98,7 @@ TEST_F(DOMUITest, DOMUIToDOMUI) { EXPECT_FALSE(contents()->ShouldDisplayURL()); EXPECT_FALSE(contents()->ShouldDisplayFavIcon()); EXPECT_TRUE(contents()->IsBookmarkBarAlwaysVisible()); - EXPECT_FALSE(contents()->FocusLocationBarByDefault()); + EXPECT_TRUE(contents()->FocusLocationBarByDefault()); } TEST_F(DOMUITest, StandardToDOMUI) { diff --git a/chrome/browser/tab_contents/web_contents.cc b/chrome/browser/tab_contents/web_contents.cc index 6a8b721..7ba7c1a 100644 --- a/chrome/browser/tab_contents/web_contents.cc +++ b/chrome/browser/tab_contents/web_contents.cc @@ -363,30 +363,16 @@ SiteInstance* WebContents::GetSiteInstance() const { } bool WebContents::ShouldDisplayURL() { - 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(); + DOMUI* dom_ui = GetDOMUIForCurrentState(); + if (dom_ui) + return !dom_ui->should_hide_url(); return true; } bool WebContents::ShouldDisplayFavIcon() { - 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(); + DOMUI* dom_ui = GetDOMUIForCurrentState(); + if (dom_ui) + return !dom_ui->hide_favicon(); return true; } @@ -532,21 +518,24 @@ void WebContents::HideContents() { } bool WebContents::IsBookmarkBarAlwaysVisible() { - // 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; + // 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. } + // 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; + return false; // Default. } void WebContents::SetDownloadShelfVisible(bool visible) { @@ -563,12 +552,9 @@ void WebContents::PopupNotificationVisibilityChanged(bool visible) { } bool WebContents::FocusLocationBarByDefault() { - // 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(); + DOMUI* dom_ui = GetDOMUIForCurrentState(); + if (dom_ui) + return dom_ui->focus_location_bar_by_default(); return false; } @@ -2018,3 +2004,43 @@ 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 58d4fb4..3407a93 100644 --- a/chrome/browser/tab_contents/web_contents.h +++ b/chrome/browser/tab_contents/web_contents.h @@ -582,6 +582,10 @@ 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. |