diff options
| -rw-r--r-- | chrome/browser/ui/search/instant_extended_interactive_uitest.cc | 12 | ||||
| -rw-r--r-- | chrome/browser/ui/search/instant_page.cc | 98 | ||||
| -rw-r--r-- | chrome/browser/ui/search/instant_page.h | 34 | ||||
| -rw-r--r-- | chrome/browser/ui/search/instant_page_unittest.cc | 79 | ||||
| -rw-r--r-- | chrome/browser/ui/search/instant_tab.cc | 2 | ||||
| -rw-r--r-- | chrome/browser/ui/search/instant_test_utils.cc | 12 | ||||
| -rw-r--r-- | chrome/browser/ui/search/search_model.cc | 35 | ||||
| -rw-r--r-- | chrome/browser/ui/search/search_model.h | 34 | ||||
| -rw-r--r-- | chrome/browser/ui/search/search_model_unittest.cc | 160 | ||||
| -rw-r--r-- | chrome/browser/ui/search/search_tab_helper.cc | 51 | ||||
| -rw-r--r-- | chrome/browser/ui/search/search_tab_helper.h | 27 | ||||
| -rw-r--r-- | chrome/chrome_tests_unit.gypi | 3 |
12 files changed, 94 insertions, 453 deletions
diff --git a/chrome/browser/ui/search/instant_extended_interactive_uitest.cc b/chrome/browser/ui/search/instant_extended_interactive_uitest.cc index 1385fc2..e0354bb 100644 --- a/chrome/browser/ui/search/instant_extended_interactive_uitest.cc +++ b/chrome/browser/ui/search/instant_extended_interactive_uitest.cc @@ -366,15 +366,13 @@ IN_PROC_BROWSER_TEST_F(InstantExtendedTest, MAYBE_UsesOverlayIfTabNotReady) { ASSERT_NO_FATAL_FAILURE(SetupInstant(browser())); FocusOmniboxAndWaitForInstantOverlayAndNTPSupport(); - // Open a new tab and navigate to instant URL. Start typing before InstantTab - // is properly hooked up. Should use the overlay. + // Open a new tab and start typing before InstantTab is properly hooked up. + // Should use the overlay. ui_test_utils::NavigateToURLWithDisposition( browser(), - instant_url(), - NEW_BACKGROUND_TAB, - ui_test_utils::BROWSER_TEST_NONE); - EXPECT_EQ(2, browser()->tab_strip_model()->count()); - browser()->tab_strip_model()->ActivateTabAt(1, false); + GURL(chrome::kChromeUINewTabURL), + NEW_FOREGROUND_TAB, + ui_test_utils::BROWSER_TEST_WAIT_FOR_TAB); ASSERT_TRUE(SetOmniboxTextAndWaitForOverlayToShow("query")); // But Instant tab should still exist. diff --git a/chrome/browser/ui/search/instant_page.cc b/chrome/browser/ui/search/instant_page.cc index 029d7cb..9b4a021 100644 --- a/chrome/browser/ui/search/instant_page.cc +++ b/chrome/browser/ui/search/instant_page.cc @@ -5,9 +5,6 @@ #include "chrome/browser/ui/search/instant_page.h" #include "base/utf_string_conversions.h" -#include "chrome/browser/search/search.h" -#include "chrome/browser/ui/search/search_model.h" -#include "chrome/browser/ui/search/search_tab_helper.h" #include "chrome/common/render_messages.h" #include "chrome/common/url_constants.h" #include "content/public/browser/navigation_controller.h" @@ -22,8 +19,6 @@ InstantPage::Delegate::~Delegate() { } InstantPage::~InstantPage() { - if (contents()) - GetSearchModel()->RemoveObserver(this); } bool InstantPage::supports_instant() const { @@ -76,6 +71,16 @@ void InstantPage::InitializeFonts() { routing_id(), omnibox_font_name, omnibox_font_size)); } +void InstantPage::DetermineIfPageSupportsInstant() { + if (IsLocal()) { + // Local pages always support Instant. That's why we keep them around. + int page_id = contents()->GetController().GetActiveEntry()->GetPageID(); + OnInstantSupportDetermined(page_id, true); + } else { + Send(new ChromeViewMsg_DetermineIfPageSupportsInstant(routing_id())); + } +} + void InstantPage::SendAutocompleteResults( const std::vector<InstantAutocompleteResult>& results) { Send(new ChromeViewMsg_SearchBoxAutocompleteResults(routing_id(), results)); @@ -124,26 +129,12 @@ void InstantPage::ToggleVoiceSearch() { InstantPage::InstantPage(Delegate* delegate, const std::string& instant_url) : delegate_(delegate), instant_url_(instant_url), - supports_instant_(false) { + supports_instant_(false), + instant_support_determined_(false) { } void InstantPage::SetContents(content::WebContents* contents) { - if (web_contents()) - GetSearchModel()->RemoveObserver(this); - Observe(contents); - - if (!contents) { - supports_instant_ = false; - return; - } - - SearchModel* model = GetSearchModel(); - model->AddObserver(this); - - // Already know whether the page supports instant. - if (model->instant_support() != INSTANT_SUPPORT_UNKNOWN) - SetSupportsInstant(model->instant_support() == INSTANT_SUPPORT_YES); } bool InstantPage::ShouldProcessRenderViewCreated() { @@ -179,10 +170,21 @@ void InstantPage::RenderViewCreated(content::RenderViewHost* render_view_host) { delegate_->InstantPageRenderViewCreated(contents()); } +void InstantPage::DidFinishLoad( + int64 /* frame_id */, + const GURL& /* validated_url */, + bool is_main_frame, + content::RenderViewHost* /* render_view_host */) { + if (is_main_frame && !supports_instant_) + DetermineIfPageSupportsInstant(); +} + bool InstantPage::OnMessageReceived(const IPC::Message& message) { bool handled = true; IPC_BEGIN_MESSAGE_MAP(InstantPage, message) IPC_MESSAGE_HANDLER(ChromeViewHostMsg_SetSuggestions, OnSetSuggestions) + IPC_MESSAGE_HANDLER(ChromeViewHostMsg_InstantSupportDetermined, + OnInstantSupportDetermined) IPC_MESSAGE_HANDLER(ChromeViewHostMsg_ShowInstantOverlay, OnShowInstantOverlay) IPC_MESSAGE_HANDLER(ChromeViewHostMsg_FocusOmnibox, OnFocusOmnibox) @@ -235,50 +237,42 @@ void InstantPage::DidFailProvisionalLoad( delegate_->InstantPageLoadFailed(contents()); } -void InstantPage::ModelChanged(const SearchModel::State& old_state, - const SearchModel::State& new_state) { - if (old_state.instant_support != new_state.instant_support) - SetSupportsInstant(new_state.instant_support == INSTANT_SUPPORT_YES); -} - -SearchModel* InstantPage::GetSearchModel() { - return contents() ? - SearchTabHelper::FromWebContents(contents())->model() : NULL; -} - -void InstantPage::SetSupportsInstant(bool supports_instant) { - // Nothing to do if the page already supports Instant. - if (supports_instant_) - return; - - supports_instant_ = supports_instant; - delegate_->InstantSupportDetermined(contents(), supports_instant); - - // If the page doesn't support Instant, stop listening to it. - if (!supports_instant) - SetContents(NULL); -} - void InstantPage::OnSetSuggestions( int page_id, const std::vector<InstantSuggestion>& suggestions) { if (!contents()->IsActiveEntry(page_id)) return; - SearchTabHelper::FromWebContents(contents())->InstantSupportChanged(true); + OnInstantSupportDetermined(page_id, true); if (!ShouldProcessSetSuggestions()) return; delegate_->SetSuggestions(contents(), suggestions); } +void InstantPage::OnInstantSupportDetermined(int page_id, + bool supports_instant) { + if (!contents()->IsActiveEntry(page_id) || supports_instant_) { + // Nothing to do if the page already supports Instant. + return; + } + + instant_support_determined_ = true; + supports_instant_ = supports_instant; + delegate_->InstantSupportDetermined(contents(), supports_instant); + + // If the page doesn't support Instant, stop listening to it. + if (!supports_instant) + Observe(NULL); +} + void InstantPage::OnShowInstantOverlay(int page_id, int height, InstantSizeUnits units) { if (!contents()->IsActiveEntry(page_id)) return; - SearchTabHelper::FromWebContents(contents())->InstantSupportChanged(true); + OnInstantSupportDetermined(page_id, true); delegate_->LogDropdownShown(); if (!ShouldProcessShowInstantOverlay()) return; @@ -290,7 +284,7 @@ void InstantPage::OnFocusOmnibox(int page_id, OmniboxFocusState state) { if (!contents()->IsActiveEntry(page_id)) return; - SearchTabHelper::FromWebContents(contents())->InstantSupportChanged(true); + OnInstantSupportDetermined(page_id, true); if (!ShouldProcessFocusOmnibox()) return; @@ -305,7 +299,7 @@ void InstantPage::OnSearchBoxNavigate(int page_id, if (!contents()->IsActiveEntry(page_id)) return; - SearchTabHelper::FromWebContents(contents())->InstantSupportChanged(true); + OnInstantSupportDetermined(page_id, true); if (!ShouldProcessNavigateToURL()) return; @@ -314,19 +308,13 @@ void InstantPage::OnSearchBoxNavigate(int page_id, } void InstantPage::OnDeleteMostVisitedItem(const GURL& url) { - // TODO(kmadhusu): Call delegate_->DeleteMostVisitedItem() only if the message - // is for the active entry and if the page should process the message. delegate_->DeleteMostVisitedItem(url); } void InstantPage::OnUndoMostVisitedDeletion(const GURL& url) { - // TODO(kmadhusu): Call delegate_->UndoMostVisitedDeletion() only if the - // message is for the active entry and if the page should process the message. delegate_->UndoMostVisitedDeletion(url); } void InstantPage::OnUndoAllMostVisitedDeletions() { - // TODO(kmadhusu): Call delegate_->UndoAllMostVisitedDeletions() only if the - // message is for the active entry and if the page should process the message. delegate_->UndoAllMostVisitedDeletions(); } diff --git a/chrome/browser/ui/search/instant_page.h b/chrome/browser/ui/search/instant_page.h index 7361169..56d1291 100644 --- a/chrome/browser/ui/search/instant_page.h +++ b/chrome/browser/ui/search/instant_page.h @@ -11,7 +11,6 @@ #include "base/compiler_specific.h" #include "base/gtest_prod_util.h" #include "base/string16.h" -#include "chrome/browser/ui/search/search_model_observer.h" #include "chrome/common/instant_types.h" #include "chrome/common/omnibox_focus_state.h" #include "content/public/browser/web_contents_observer.h" @@ -33,8 +32,7 @@ class Rect; // Instant/Embedded Search API (http://dev.chromium.org/embeddedsearch). // InstantPage is not used directly but via one of its derived classes: // InstantOverlay, InstantNTP and InstantTab. -class InstantPage : public content::WebContentsObserver, - public SearchModelObserver { +class InstantPage : public content::WebContentsObserver { public: // InstantPage calls its delegate in response to messages received from the // page. Each method is called with the |contents| corresponding to the page @@ -121,6 +119,12 @@ class InstantPage : public content::WebContentsObserver, // support suddenly). virtual bool supports_instant() const; + // True if Instant support has been tested and determined for this page at + // least once. Note that Instant support may change in the future. + bool instant_support_determined() const { + return instant_support_determined_; + } + // Returns true if the page is the local NTP (i.e. its URL is // chrome::kChromeSearchLocalNTPURL). virtual bool IsLocal() const; @@ -155,6 +159,10 @@ class InstantPage : public content::WebContentsObserver, // Tells the page about the font information. void InitializeFonts(); + // Tells the renderer to determine if the page supports the Instant API, which + // results in a call to InstantSupportDetermined() when the reply is received. + void DetermineIfPageSupportsInstant(); + // Tells the page about the available autocomplete results. void SendAutocompleteResults( const std::vector<InstantAutocompleteResult>& results); @@ -222,6 +230,11 @@ class InstantPage : public content::WebContentsObserver, // Overridden from content::WebContentsObserver: virtual void RenderViewCreated( content::RenderViewHost* render_view_host) OVERRIDE; + virtual void DidFinishLoad( + int64 frame_id, + const GURL& validated_url, + bool is_main_frame, + content::RenderViewHost* render_view_host) OVERRIDE; virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE; virtual void RenderViewGone(base::TerminationStatus status) OVERRIDE; virtual void DidCommitProvisionalLoadForFrame( @@ -241,18 +254,9 @@ class InstantPage : public content::WebContentsObserver, const string16& error_description, content::RenderViewHost* render_view_host) OVERRIDE; - // Overridden from SearchModelObserver: - virtual void ModelChanged(const SearchModel::State& old_state, - const SearchModel::State& new_state) OVERRIDE; - - // Helper to look up the SearchModel for this page. - SearchModel* GetSearchModel(); - - // Update the status of Instant support. - void SetSupportsInstant(bool supports_instant); - void OnSetSuggestions(int page_id, const std::vector<InstantSuggestion>& suggestions); + void OnInstantSupportDetermined(int page_id, bool supports_instant); void OnShowInstantOverlay(int page_id, int height, InstantSizeUnits units); @@ -268,10 +272,8 @@ class InstantPage : public content::WebContentsObserver, Delegate* const delegate_; const std::string instant_url_; - - // TODO(kmadhusu): Remove |supports_instant_| from here and get the details - // from SearchModel. Refer to crbug.com/246323 for more details. bool supports_instant_; + bool instant_support_determined_; DISALLOW_COPY_AND_ASSIGN(InstantPage); }; diff --git a/chrome/browser/ui/search/instant_page_unittest.cc b/chrome/browser/ui/search/instant_page_unittest.cc index 9c79dd0..592c510 100644 --- a/chrome/browser/ui/search/instant_page_unittest.cc +++ b/chrome/browser/ui/search/instant_page_unittest.cc @@ -4,14 +4,10 @@ #include "chrome/browser/ui/search/instant_page.h" -#include "base/command_line.h" #include "base/memory/scoped_ptr.h" -#include "chrome/browser/ui/search/search_tab_helper.h" -#include "chrome/common/chrome_switches.h" #include "chrome/common/render_messages.h" #include "chrome/common/url_constants.h" #include "chrome/test/base/chrome_render_view_host_test_harness.h" -#include "content/public/browser/navigation_entry.h" #include "content/public/browser/web_contents.h" #include "content/public/test/mock_render_process_host.h" #include "googleurl/src/gurl.h" @@ -68,33 +64,20 @@ class FakePage : public InstantPage { } using InstantPage::SetContents; - - private: - DISALLOW_COPY_AND_ASSIGN(FakePage); }; } // namespace class InstantPageTest : public ChromeRenderViewHostTestHarness { public: - virtual void SetUp() OVERRIDE; - scoped_ptr<FakePage> page; FakePageDelegate delegate; }; -void InstantPageTest::SetUp() { - CommandLine::ForCurrentProcess()->AppendSwitch( - switches::kEnableInstantExtendedAPI); - ChromeRenderViewHostTestHarness::SetUp(); - SearchTabHelper::CreateForWebContents(web_contents()); - page.reset(new FakePage(&delegate, "")); - page->SetContents(web_contents()); -} - TEST_F(InstantPageTest, IsLocal) { - EXPECT_FALSE(page->supports_instant()); + page.reset(new FakePage(&delegate, "")); EXPECT_FALSE(page->IsLocal()); + page->SetContents(web_contents()); NavigateAndCommit(GURL(chrome::kChromeSearchLocalNtpUrl)); EXPECT_TRUE(page->IsLocal()); NavigateAndCommit(GURL("http://example.com")); @@ -104,23 +87,25 @@ TEST_F(InstantPageTest, IsLocal) { } TEST_F(InstantPageTest, DetermineIfPageSupportsInstant_Local) { + page.reset(new FakePage(&delegate, "")); EXPECT_FALSE(page->supports_instant()); + page->SetContents(web_contents()); NavigateAndCommit(GURL(chrome::kChromeSearchLocalNtpUrl)); EXPECT_TRUE(page->IsLocal()); EXPECT_CALL(delegate, InstantSupportDetermined(web_contents(), true)) .Times(1); - SearchTabHelper::FromWebContents(web_contents())-> - DetermineIfPageSupportsInstant(); + page->DetermineIfPageSupportsInstant(); EXPECT_TRUE(page->supports_instant()); } TEST_F(InstantPageTest, DetermineIfPageSupportsInstant_NonLocal) { + page.reset(new FakePage(&delegate, "")); EXPECT_FALSE(page->supports_instant()); - NavigateAndCommit(GURL("chrome-search://foo/bar")); + page->SetContents(web_contents()); + NavigateAndCommit(GURL("http://example.com/")); EXPECT_FALSE(page->IsLocal()); process()->sink().ClearMessages(); - SearchTabHelper::FromWebContents(web_contents())-> - DetermineIfPageSupportsInstant(); + page->DetermineIfPageSupportsInstant(); const IPC::Message* message = process()->sink().GetFirstMessageMatching( ChromeViewMsg_DetermineIfPageSupportsInstant::ID); ASSERT_TRUE(message != NULL); @@ -155,49 +140,3 @@ TEST_F(InstantPageTest, DispatchRequestToUndoAllMostVisitedDeletions) { ChromeViewHostMsg_SearchBoxUndoAllMostVisitedDeletions( rvh()->GetRoutingID()))); } - -TEST_F(InstantPageTest, PageURLDoesntBelongToInstantRenderer) { - EXPECT_FALSE(page->supports_instant()); - - // Navigate to a page URL that doesn't belong to Instant renderer. - // SearchTabHelper::DeterminerIfPageSupportsInstant() should return - // immediately without dispatching any message to the renderer. - NavigateAndCommit(GURL("http://www.example.com")); - EXPECT_FALSE(page->IsLocal()); - process()->sink().ClearMessages(); - EXPECT_CALL(delegate, InstantSupportDetermined(web_contents(), false)) - .Times(1); - - SearchTabHelper::FromWebContents(web_contents())-> - DetermineIfPageSupportsInstant(); - const IPC::Message* message = process()->sink().GetFirstMessageMatching( - ChromeViewMsg_DetermineIfPageSupportsInstant::ID); - ASSERT_TRUE(message == NULL); - EXPECT_FALSE(page->supports_instant()); -} - -// Test to verify that ChromeViewMsg_DetermineIfPageSupportsInstant message -// reply handler updates the instant support state in InstantPage. -TEST_F(InstantPageTest, PageSupportsInstant) { - EXPECT_FALSE(page->supports_instant()); - NavigateAndCommit(GURL("chrome-search://foo/bar")); - process()->sink().ClearMessages(); - SearchTabHelper::FromWebContents(web_contents())-> - DetermineIfPageSupportsInstant(); - const IPC::Message* message = process()->sink().GetFirstMessageMatching( - ChromeViewMsg_DetermineIfPageSupportsInstant::ID); - ASSERT_TRUE(message != NULL); - EXPECT_EQ(web_contents()->GetRoutingID(), message->routing_id()); - - EXPECT_CALL(delegate, InstantSupportDetermined(web_contents(), true)) - .Times(1); - - // Assume the page supports instant. Invoke the message reply handler to make - // sure the InstantPage is notified about the instant support state. - const content::NavigationEntry* entry = - web_contents()->GetController().GetActiveEntry(); - EXPECT_TRUE(entry); - SearchTabHelper::FromWebContents(web_contents())-> - OnInstantSupportDetermined(entry->GetPageID(), true); - EXPECT_TRUE(page->supports_instant()); -} diff --git a/chrome/browser/ui/search/instant_tab.cc b/chrome/browser/ui/search/instant_tab.cc index 9ebb814..d92688a 100644 --- a/chrome/browser/ui/search/instant_tab.cc +++ b/chrome/browser/ui/search/instant_tab.cc @@ -14,6 +14,8 @@ InstantTab::~InstantTab() { void InstantTab::Init(content::WebContents* contents) { SetContents(contents); + if (!contents->IsWaitingForResponse()) + DetermineIfPageSupportsInstant(); } bool InstantTab::ShouldProcessAboutToNavigateMainFrame() { diff --git a/chrome/browser/ui/search/instant_test_utils.cc b/chrome/browser/ui/search/instant_test_utils.cc index b6e1d88..0a58371 100644 --- a/chrome/browser/ui/search/instant_test_utils.cc +++ b/chrome/browser/ui/search/instant_test_utils.cc @@ -122,8 +122,10 @@ void InstantTestBase::FocusOmniboxAndWaitForInstantOverlaySupport() { chrome::NOTIFICATION_INSTANT_OVERLAY_SUPPORT_DETERMINED, content::NotificationService::AllSources()); FocusOmnibox(); - if (!instant()->overlay() || !instant()->overlay()->supports_instant()) + if (!instant()->overlay() || + !instant()->overlay()->instant_support_determined()) { observer.Wait(); + } } void InstantTestBase::FocusOmniboxAndWaitForInstantOverlayAndNTPSupport() { @@ -134,10 +136,14 @@ void InstantTestBase::FocusOmniboxAndWaitForInstantOverlayAndNTPSupport() { chrome::NOTIFICATION_INSTANT_OVERLAY_SUPPORT_DETERMINED, content::NotificationService::AllSources()); FocusOmnibox(); - if (!instant()->ntp() || !instant()->ntp()->supports_instant()) + if (!instant()->ntp() || + !instant()->ntp()->instant_support_determined()) { ntp_observer.Wait(); - if (!instant()->overlay() || !instant()->overlay()->supports_instant()) + } + if (!instant()->overlay() || + !instant()->overlay()->instant_support_determined()) { overlay_observer.Wait(); + } } void InstantTestBase::SetOmniboxText(const std::string& text) { diff --git a/chrome/browser/ui/search/search_model.cc b/chrome/browser/ui/search/search_model.cc index da4bcff..53d0fcc 100644 --- a/chrome/browser/ui/search/search_model.cc +++ b/chrome/browser/ui/search/search_model.cc @@ -7,24 +7,6 @@ #include "chrome/browser/search/search.h" #include "chrome/browser/ui/search/search_model_observer.h" -SearchModel::State::State() - : top_bars_visible(true), - instant_support(INSTANT_SUPPORT_UNKNOWN) { -} - -SearchModel::State::State(const SearchMode& mode, - bool top_bars_visible, - InstantSupportState instant_support) - : mode(mode), - top_bars_visible(top_bars_visible), - instant_support(instant_support) { -} - -bool SearchModel::State::operator==(const State& rhs) const { - return mode == rhs.mode && top_bars_visible == rhs.top_bars_visible && - instant_support == rhs.instant_support; -} - SearchModel::SearchModel() { } @@ -95,23 +77,6 @@ void SearchModel::SetTopBarsVisible(bool visible) { ModelChanged(old_state, state_)); } -void SearchModel::SetSupportsInstant(bool supports_instant) { - DCHECK(chrome::IsInstantExtendedAPIEnabled()) - << "Please do not try to set the SearchModel mode without first " - << "checking if Search is enabled."; - - InstantSupportState new_instant_support = supports_instant ? - INSTANT_SUPPORT_YES : INSTANT_SUPPORT_NO; - - if (state_.instant_support == new_instant_support) - return; - - const State old_state = state_; - state_.instant_support = new_instant_support; - FOR_EACH_OBSERVER(SearchModelObserver, observers_, - ModelChanged(old_state, state_)); -} - void SearchModel::AddObserver(SearchModelObserver* observer) { observers_.AddObserver(observer); } diff --git a/chrome/browser/ui/search/search_model.h b/chrome/browser/ui/search/search_model.h index 07c26ef..3f77b87 100644 --- a/chrome/browser/ui/search/search_model.h +++ b/chrome/browser/ui/search/search_model.h @@ -11,36 +11,25 @@ class SearchModelObserver; -// Represents whether a page supports Instant. -enum InstantSupportState { - INSTANT_SUPPORT_NO, - INSTANT_SUPPORT_YES, - INSTANT_SUPPORT_UNKNOWN, -}; - // An observable model for UI components that care about search model state // changes. class SearchModel { public: struct State { - State(); - State(const SearchMode& mode, - bool top_bars_visible, - InstantSupportState instant_support); + State() : top_bars_visible(true) {} + State(const SearchMode& mode, bool top_bars_visible) + : mode(mode), + top_bars_visible(top_bars_visible) {} - bool operator==(const State& rhs) const; + bool operator==(const State& rhs) const { + return mode == rhs.mode && top_bars_visible == rhs.top_bars_visible; + } // The display mode of UI elements such as the toolbar, the tab strip, etc. SearchMode mode; // The visibility of top bars such as bookmark and info bars. bool top_bars_visible; - - // Does the current page support Instant? - // - // TODO(kmadhusu): Use bool instead of a tri-state variable. Refer to - // crbug.com/246323 for more details. - InstantSupportState instant_support; }; SearchModel(); @@ -69,15 +58,6 @@ class SearchModel { // Get the visibility of top bars. bool top_bars_visible() const { return state_.top_bars_visible; } - // Set whether the page supports Instant. Change notifications are sent to - // observers. - void SetSupportsInstant(bool supports_instant); - - // Get whether the page supports Instant. - InstantSupportState instant_support() const { - return state_.instant_support; - } - // Add and remove observers. void AddObserver(SearchModelObserver* observer); void RemoveObserver(SearchModelObserver* observer); diff --git a/chrome/browser/ui/search/search_model_unittest.cc b/chrome/browser/ui/search/search_model_unittest.cc deleted file mode 100644 index 88ed3aa..0000000 --- a/chrome/browser/ui/search/search_model_unittest.cc +++ /dev/null @@ -1,160 +0,0 @@ -// Copyright 2013 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/ui/search/search_model.h" - -#include "base/command_line.h" -#include "chrome/browser/ui/search/search_model_observer.h" -#include "chrome/browser/ui/search/search_tab_helper.h" -#include "chrome/common/chrome_switches.h" -#include "chrome/common/search_types.h" -#include "chrome/test/base/chrome_render_view_host_test_harness.h" - -namespace { - -class MockSearchModelObserver : public SearchModelObserver { - public: - MockSearchModelObserver(); - virtual ~MockSearchModelObserver(); - - virtual void ModelChanged(const SearchModel::State& old_state, - const SearchModel::State& new_state) OVERRIDE; - - void VerifySearchModelStates(const SearchModel::State& expected_old_state, - const SearchModel::State& expected_new_state); - - void VerifyNotificationCount(int expected_count); - - private: - // How many times we've seen search model changed notifications. - int modelchanged_notification_count_; - - SearchModel::State actual_old_state_; - SearchModel::State actual_new_state_; - - DISALLOW_COPY_AND_ASSIGN(MockSearchModelObserver); -}; - -MockSearchModelObserver::MockSearchModelObserver() - : modelchanged_notification_count_(0) { -} - -MockSearchModelObserver::~MockSearchModelObserver() { -} - -void MockSearchModelObserver::ModelChanged( - const SearchModel::State& old_state, - const SearchModel::State& new_state) { - actual_old_state_ = old_state; - actual_new_state_ = new_state; - modelchanged_notification_count_++; -} - -void MockSearchModelObserver::VerifySearchModelStates( - const SearchModel::State& expected_old_state, - const SearchModel::State& expected_new_state) { - EXPECT_TRUE(actual_old_state_ == expected_old_state); - EXPECT_TRUE(actual_new_state_ == expected_new_state); -} - -void MockSearchModelObserver::VerifyNotificationCount(int expected_count) { - EXPECT_EQ(modelchanged_notification_count_, expected_count); -} - -} // namespace - -class SearchModelTest : public ChromeRenderViewHostTestHarness { - public: - virtual void SetUp() OVERRIDE; - virtual void TearDown() OVERRIDE; - - MockSearchModelObserver mock_observer; - SearchModel* model; -}; - -void SearchModelTest::SetUp() { - CommandLine::ForCurrentProcess()->AppendSwitch( - switches::kEnableInstantExtendedAPI); - ChromeRenderViewHostTestHarness::SetUp(); - SearchTabHelper::CreateForWebContents(web_contents()); - SearchTabHelper* search_tab_helper = - SearchTabHelper::FromWebContents(web_contents()); - ASSERT_TRUE(search_tab_helper != NULL); - model = search_tab_helper->model(); - model->AddObserver(&mock_observer); -} - -void SearchModelTest::TearDown() { - model->RemoveObserver(&mock_observer); - ChromeRenderViewHostTestHarness::TearDown(); -} - -TEST_F(SearchModelTest, UpdateSearchModelInstantSupport) { - mock_observer.VerifyNotificationCount(0); - EXPECT_TRUE(model->instant_support() == INSTANT_SUPPORT_UNKNOWN); - SearchModel::State expected_old_state = model->state(); - SearchModel::State expected_new_state(model->state()); - expected_new_state.instant_support = INSTANT_SUPPORT_YES; - - model->SetSupportsInstant(true); - mock_observer.VerifySearchModelStates(expected_old_state, expected_new_state); - mock_observer.VerifyNotificationCount(1); - EXPECT_TRUE(model->instant_support() == INSTANT_SUPPORT_YES); - - expected_old_state = expected_new_state; - expected_new_state.instant_support = INSTANT_SUPPORT_NO; - model->SetSupportsInstant(false); - mock_observer.VerifySearchModelStates(expected_old_state, expected_new_state); - mock_observer.VerifyNotificationCount(2); - - // Notify the observer only if the search model state is changed. - model->SetSupportsInstant(false); - EXPECT_TRUE(model->state() == expected_new_state); - EXPECT_TRUE(model->instant_support() == INSTANT_SUPPORT_NO); - mock_observer.VerifyNotificationCount(2); -} - -TEST_F(SearchModelTest, UpdateSearchModelMode) { - mock_observer.VerifyNotificationCount(0); - SearchMode search_mode(SearchMode::MODE_NTP, SearchMode::ORIGIN_NTP); - SearchModel::State expected_old_state = model->state(); - SearchModel::State expected_new_state(model->state()); - expected_new_state.mode = search_mode; - - model->SetMode(search_mode); - mock_observer.VerifySearchModelStates(expected_old_state, expected_new_state); - mock_observer.VerifyNotificationCount(1); - - search_mode.mode = SearchMode::MODE_SEARCH_RESULTS; - expected_old_state = expected_new_state; - expected_new_state.mode = search_mode; - model->SetMode(search_mode); - mock_observer.VerifySearchModelStates(expected_old_state, expected_new_state); - mock_observer.VerifyNotificationCount(2); - EXPECT_TRUE(model->state() == expected_new_state); -} - -TEST_F(SearchModelTest, UpdateTopBarVisibility) { - mock_observer.VerifyNotificationCount(0); - EXPECT_TRUE(model->top_bars_visible()); - - SearchModel::State expected_old_state = model->state(); - SearchModel::State expected_new_state(model->state()); - expected_new_state.top_bars_visible = false; - - model->SetTopBarsVisible(false); - mock_observer.VerifySearchModelStates(expected_old_state, expected_new_state); - mock_observer.VerifyNotificationCount(1); - EXPECT_FALSE(model->top_bars_visible()); -} - -TEST_F(SearchModelTest, UpdateSearchModelState) { - SearchModel::State expected_new_state(model->state()); - expected_new_state.top_bars_visible = false; - expected_new_state.instant_support = INSTANT_SUPPORT_NO; - EXPECT_FALSE(model->state() == expected_new_state); - model->SetState(expected_new_state); - mock_observer.VerifyNotificationCount(1); - EXPECT_TRUE(model->state() == expected_new_state); -} diff --git a/chrome/browser/ui/search/search_tab_helper.cc b/chrome/browser/ui/search/search_tab_helper.cc index 29e0e83c..5086f81 100644 --- a/chrome/browser/ui/search/search_tab_helper.cc +++ b/chrome/browser/ui/search/search_tab_helper.cc @@ -4,14 +4,12 @@ #include "chrome/browser/ui/search/search_tab_helper.h" -#include "chrome/browser/profiles/profile.h" #include "chrome/browser/search/search.h" #include "chrome/common/render_messages.h" #include "chrome/common/url_constants.h" #include "content/public/browser/navigation_entry.h" #include "content/public/browser/notification_service.h" #include "content/public/browser/notification_types.h" -#include "content/public/browser/web_contents.h" DEFINE_WEB_CONTENTS_USER_DATA_KEY(SearchTabHelper); @@ -32,14 +30,6 @@ bool IsSearchResults(const content::WebContents* contents) { return !chrome::GetSearchTerms(contents).empty(); } -// TODO(kmadhusu): Move this helper from anonymous namespace to chrome -// namespace and remove InstantPage::IsLocal(). -bool IsLocal(const content::WebContents* contents) { - return contents && - (contents->GetURL() == GURL(chrome::kChromeSearchLocalNtpUrl) || - contents->GetURL() == GURL(chrome::kChromeSearchLocalGoogleNtpUrl)); -} - } // namespace SearchTabHelper::SearchTabHelper(content::WebContents* web_contents) @@ -85,13 +75,6 @@ void SearchTabHelper::NavigationEntryUpdated() { UpdateMode(); } -void SearchTabHelper::InstantSupportChanged(bool supports_instant) { - if (!is_search_enabled_) - return; - - model_.SetSupportsInstant(supports_instant); -} - void SearchTabHelper::Observe( int type, const content::NotificationSource& source, @@ -107,22 +90,11 @@ bool SearchTabHelper::OnMessageReceived(const IPC::Message& message) { OnSearchBoxShowBars) IPC_MESSAGE_HANDLER(ChromeViewHostMsg_SearchBoxHideBars, OnSearchBoxHideBars) - IPC_MESSAGE_HANDLER(ChromeViewHostMsg_InstantSupportDetermined, - OnInstantSupportDetermined) IPC_MESSAGE_UNHANDLED(handled = false) IPC_END_MESSAGE_MAP() return handled; } -void SearchTabHelper::DidFinishLoad( - int64 /* frame_id */, - const GURL& /* validated_url */, - bool is_main_frame, - content::RenderViewHost* /* render_view_host */) { - if (is_main_frame) - DetermineIfPageSupportsInstant(); -} - void SearchTabHelper::UpdateMode() { SearchMode::Type type = SearchMode::MODE_DEFAULT; SearchMode::Origin origin = SearchMode::ORIGIN_DEFAULT; @@ -145,33 +117,12 @@ void SearchTabHelper::UpdateMode() { // OmniboxEditModel::SetInputInProgress() which is called from // OmniboxEditModel::Revert(). model_.SetState(SearchModel::State(SearchMode(type, origin), - model_.state().top_bars_visible, - model_.instant_support())); + model_.state().top_bars_visible)); } else { model_.SetMode(SearchMode(type, origin)); } } -void SearchTabHelper::DetermineIfPageSupportsInstant() { - Profile* profile = - Profile::FromBrowserContext(web_contents_->GetBrowserContext()); - if (!chrome::ShouldAssignURLToInstantRenderer(web_contents_->GetURL(), - profile)) { - InstantSupportChanged(false); - } else if (IsLocal(web_contents_)) { - // Local pages always support Instant. - InstantSupportChanged(true); - } else { - Send(new ChromeViewMsg_DetermineIfPageSupportsInstant(routing_id())); - } -} - -void SearchTabHelper::OnInstantSupportDetermined(int page_id, - bool supports_instant) { - if (web_contents()->IsActiveEntry(page_id)) - InstantSupportChanged(supports_instant); -} - void SearchTabHelper::OnSearchBoxShowBars(int page_id) { if (web_contents()->IsActiveEntry(page_id)) model_.SetTopBarsVisible(true); diff --git a/chrome/browser/ui/search/search_tab_helper.h b/chrome/browser/ui/search/search_tab_helper.h index d7f260f..7b0ff13 100644 --- a/chrome/browser/ui/search/search_tab_helper.h +++ b/chrome/browser/ui/search/search_tab_helper.h @@ -7,7 +7,6 @@ #include "base/basictypes.h" #include "base/compiler_specific.h" -#include "base/gtest_prod_util.h" #include "chrome/browser/ui/search/search_model.h" #include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" @@ -18,8 +17,6 @@ namespace content { class WebContents; } -class InstantPageTest; - // Per-tab search "helper". Acts as the owner and controller of the tab's // search UI model. class SearchTabHelper : public content::NotificationObserver, @@ -45,19 +42,8 @@ class SearchTabHelper : public content::NotificationObserver, // the notification system and shouldn't call this method. void NavigationEntryUpdated(); - // Invoked to update the Instant support state. - void InstantSupportChanged(bool supports_instant); - private: friend class content::WebContentsUserData<SearchTabHelper>; - friend class InstantPageTest; - FRIEND_TEST_ALL_PREFIXES(InstantPageTest, - DetermineIfPageSupportsInstant_Local); - FRIEND_TEST_ALL_PREFIXES(InstantPageTest, - DetermineIfPageSupportsInstant_NonLocal); - FRIEND_TEST_ALL_PREFIXES(InstantPageTest, - PageURLDoesntBelongToInstantRenderer); - FRIEND_TEST_ALL_PREFIXES(InstantPageTest, PageSupportsInstant); explicit SearchTabHelper(content::WebContents* web_contents); @@ -68,23 +54,10 @@ class SearchTabHelper : public content::NotificationObserver, // Overridden from contents::WebContentsObserver: virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE; - virtual void DidFinishLoad( - int64 frame_id, - const GURL& validated_url, - bool is_main_frame, - content::RenderViewHost* render_view_host) OVERRIDE; // Sets the mode of the model based on the current URL of web_contents(). void UpdateMode(); - // Tells the renderer to determine if the page supports the Instant API, which - // results in a call to OnInstantSupportDetermined() when the reply - // is received. - void DetermineIfPageSupportsInstant(); - - // Handler for when Instant support has been determined. - void OnInstantSupportDetermined(int page_id, bool supports_instant); - // Handlers for SearchBox API to show and hide top bars (bookmark and info // bars). void OnSearchBoxShowBars(int page_id); diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 90b2cd6..4ab38eb 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -1497,7 +1497,6 @@ 'browser/ui/search/instant_controller_unittest.cc', 'browser/ui/search/instant_page_unittest.cc', 'browser/ui/search/search_delegate_unittest.cc', - 'browser/ui/search/search_model_unittest.cc', 'browser/ui/startup/session_crashed_infobar_delegate_unittest.cc', 'browser/ui/sync/one_click_signin_helper_unittest.cc', 'browser/ui/sync/profile_signin_confirmation_helper_unittest.cc', @@ -2320,9 +2319,7 @@ 'browser/ui/fullscreen/fullscreen_controller_state_unittest.cc', 'browser/ui/fullscreen/fullscreen_controller_unittest.cc', 'browser/ui/search/instant_controller_unittest.cc', - 'browser/ui/search/instant_page_unittest.cc', 'browser/ui/search/search_delegate_unittest.cc', - 'browser/ui/search/search_model_unittest.cc', 'browser/ui/tab_contents/tab_contents_iterator_unittest.cc', 'browser/ui/toolbar/recent_tabs_sub_menu_model_unittest.cc', 'browser/ui/toolbar/toolbar_model_unittest.cc', |