Chromium Code Reviews| Index: chrome/browser/instant/instant_controller.cc |
| diff --git a/chrome/browser/instant/instant_controller.cc b/chrome/browser/instant/instant_controller.cc |
| index f3146a5d324ce0429e460109f88df97bf6c3657a..781873ea36ce721377b5c715221aab17daa0a86d 100644 |
| --- a/chrome/browser/instant/instant_controller.cc |
| +++ b/chrome/browser/instant/instant_controller.cc |
| @@ -14,12 +14,12 @@ |
| #include "chrome/browser/history/history_service_factory.h" |
| #include "chrome/browser/history/history_tab_helper.h" |
| #include "chrome/browser/instant/instant_loader.h" |
| +#include "chrome/browser/instant/instant_tab.h" |
| #include "chrome/browser/platform_util.h" |
| #include "chrome/browser/search_engines/template_url_service.h" |
| #include "chrome/browser/search_engines/template_url_service_factory.h" |
| #include "chrome/browser/ui/browser_instant_controller.h" |
| #include "chrome/browser/ui/search/search_tab_helper.h" |
| -#include "chrome/browser/ui/tab_contents/tab_contents.h" |
| #include "chrome/common/chrome_notification_types.h" |
| #include "chrome/common/chrome_switches.h" |
| #include "content/public/browser/navigation_entry.h" |
| @@ -48,16 +48,16 @@ const int kMaxInstantSupportFailures = 10; |
| const int kStaleLoaderTimeoutMS = 3 * 3600 * 1000; |
| void AddSessionStorageHistogram(bool extended_enabled, |
| - const TabContents* tab1, |
| - const TabContents* tab2) { |
| + const content::WebContents* tab1, |
| + const content::WebContents* tab2) { |
| base::Histogram* histogram = base::BooleanHistogram::FactoryGet( |
| std::string("Instant.SessionStorageNamespace") + |
| (extended_enabled ? "_Extended" : "_Instant"), |
| base::Histogram::kUmaTargetedHistogramFlag); |
| const content::SessionStorageNamespaceMap& session_storage_map1 = |
| - tab1->web_contents()->GetController().GetSessionStorageNamespaceMap(); |
| + tab1->GetController().GetSessionStorageNamespaceMap(); |
| const content::SessionStorageNamespaceMap& session_storage_map2 = |
| - tab2->web_contents()->GetController().GetSessionStorageNamespaceMap(); |
| + tab2->GetController().GetSessionStorageNamespaceMap(); |
| bool is_session_storage_the_same = |
| session_storage_map1.size() == session_storage_map2.size(); |
| if (is_session_storage_the_same) { |
| @@ -90,7 +90,7 @@ string16 Normalize(const string16& str) { |
| } |
| bool NormalizeAndStripPrefix(string16* text, const string16& prefix) { |
| - const string16 norm_prefix = Normalize(prefix); |
| + string16 norm_prefix = Normalize(prefix); |
|
samarth
2012/12/03 19:46:03
FWIW, I find this kind of const usage useful (as o
sreeram
2012/12/04 08:10:52
Ack. I do vacillate between too much and too littl
|
| string16 norm_text = Normalize(*text); |
| if (norm_prefix.size() <= norm_text.size() && |
| norm_text.compare(0, norm_prefix.size(), norm_prefix) == 0) { |
| @@ -111,9 +111,8 @@ gfx::NativeView GetViewGainingFocus(gfx::NativeView view_gaining_focus) { |
| views::FocusManager* focus_manager = widget->GetFocusManager(); |
| if (focus_manager && focus_manager->is_changing_focus() && |
| focus_manager->GetFocusedView() && |
| - focus_manager->GetFocusedView()->GetWidget()) { |
| + focus_manager->GetFocusedView()->GetWidget()) |
| return focus_manager->GetFocusedView()->GetWidget()->GetNativeView(); |
| - } |
| } |
| #endif |
| return view_gaining_focus; |
| @@ -161,9 +160,9 @@ InstantController::~InstantController() { |
| bool InstantController::Update(const AutocompleteMatch& match, |
| const string16& user_text, |
| const string16& full_text, |
| - const bool verbatim, |
| - const bool user_input_in_progress, |
| - const bool omnibox_popup_is_open) { |
| + bool verbatim, |
| + bool user_input_in_progress, |
| + bool omnibox_popup_is_open) { |
| if (!extended_enabled_ && !instant_enabled_) |
| return false; |
| @@ -176,11 +175,12 @@ bool InstantController::Update(const AutocompleteMatch& match, |
| DCHECK(!omnibox_popup_is_open || user_input_in_progress); |
| // If the popup is closed, there should be no inline autocompletion. |
| - DCHECK(omnibox_popup_is_open || user_text == full_text) << user_text << "|" |
| - << full_text; |
| + DCHECK(omnibox_popup_is_open || user_text.empty() || user_text == full_text) |
| + << user_text << "|" << full_text; |
| // If there's inline autocompletion, the query has to be verbatim. |
| - DCHECK(user_text == full_text || verbatim) << user_text << "|" << full_text; |
| + DCHECK(user_text.empty() || user_text == full_text || verbatim) |
| + << user_text << "|" << full_text; |
| // If there's no text in the omnibox, the user can't have typed any. |
| DCHECK(!full_text.empty() || user_text.empty()) << user_text; |
| @@ -198,9 +198,34 @@ bool InstantController::Update(const AutocompleteMatch& match, |
| search_mode_.mode = chrome::search::Mode::MODE_DEFAULT; |
| // If there's no active tab, the browser is closing. |
| - const TabContents* const active_tab = browser_->GetActiveTabContents(); |
| + const content::WebContents* active_tab = browser_->GetActiveWebContents(); |
| if (!active_tab) { |
| - Hide(true); |
| + Hide(); |
| + return false; |
| + } |
| + |
| + const TemplateURL* template_url = match.GetTemplateURL( |
| + Profile::FromBrowserContext(active_tab->GetBrowserContext()), false); |
| + bool match_is_search = AutocompleteMatch::IsSearchType(match.type); |
| + |
| + // Ensure we have a loader or tab that can process this |match|. |
| + if (extended_enabled_) { |
| + // In extended mode, use the first among the following that succeeds: |
| + // 1. A committed search results page. |
| + // 2. An Instant loader for |template_url|. |
| + // 3. If the |match| is a URL or a blank search query, the Instant loader |
| + // for the default search engine. |
| + // TODO(sreeram): If |template_url| doesn't match the |instant_tab_| URL, |
| + // we shouldn't blindly use the committed tab. |
| + if (!instant_tab_ && !ResetLoader(template_url, active_tab) && |
| + ((match_is_search && !user_text.empty()) || !CreateDefaultLoader())) { |
|
Jered
2012/11/29 19:57:05
Should this check full_text.empty() as it used to?
Jered
2012/11/29 19:57:05
It took me a second to unwind this if. Maybe
bool
sreeram
2012/12/04 08:10:52
No, user_text is the right thing here. If we are i
sreeram
2012/12/04 08:10:52
Done.
|
| + Hide(); |
| + return false; |
| + } |
| + } else if (!ResetLoader(template_url, active_tab)) { |
| + // In non-extended mode, if the TemplateURL of the |match| doesn't have a |
| + // valid Instant URL, do not proceed. |
| + Hide(); |
| return false; |
| } |
| @@ -208,7 +233,7 @@ bool InstantController::Update(const AutocompleteMatch& match, |
| // |
| // # OPIO UIIP full_text Notes |
| // - ---- ---- --------- ----- |
| - // 1 no no blank } Navigation, or user hit Escape. |full_text| is |
| + // 1 no no blank } Navigation, tab-switch or Escape. |full_text| is |
| // 2 no no non-blank } blank if the page is NTP, non-blank otherwise. |
| // |
| // 3 no yes blank User backspaced away all omnibox text. |
| @@ -226,37 +251,44 @@ bool InstantController::Update(const AutocompleteMatch& match, |
| // |
| // In extended mode, #2 and #4 call Hide(). #1 doesn't Hide() as the preview |
| // may be showing custom NTP content, but doesn't Update() either. #3 and #7 |
| - // don't Hide(), but send a blank query to Update(). #8 calls Update(). |
| + // don't Hide(), and send a blank query to Update(). #8 calls Update(). |
| if (extended_enabled_) { |
| if (!omnibox_popup_is_open) { |
| - if (!full_text.empty()) { |
| - Hide(true); |
| - return false; |
| + if (!user_input_in_progress) { |
| + if (!full_text.empty()) { |
| + Hide(); // #2 |
| + // If the user hit Escape when on a search results page, restore the |
| + // original results by resending the query. If the user was not on a |
|
Jered
2012/11/29 19:57:05
What "original" results? Do you mean the committed
sreeram
2012/12/04 08:10:52
I've tried to explain this better in the new patch
|
| + // search results page, SearchModeChanged() would've caught it, and |
| + // |instant_tab_| will be NULL, so we won't accidentally send a URL |
| + // to |instant_tab_|. Except if the user just switched tabs. Hence the |
| + // comparison of WebContents, to catch that case as well. |
| + if (instant_tab_ && instant_tab_->contents() == active_tab) |
| + instant_tab_->Update(full_text, true); |
| + } |
| + return false; // #1 |
|
Jered
2012/11/29 19:57:05
Note this is both #1 and #2.
sreeram
2012/12/04 08:10:52
Ack. See new patch.
|
| + } else { |
| + if (!full_text.empty()) { |
| + Hide(); // #4 |
| + if (match_is_search) { |
| + // The user just switched to a tab with a partial query already in |
| + // the omnibox. This tab may or may not be a search results page |
| + // (SearchModeChanged() hasn't been called yet). So, assume that it |
| + // could be, and store |full_text| so that if the user then hits |
| + // Enter, we'll send the correct query in instant_tab_->Submit(). |
| + last_full_text_ = full_text; |
| + last_match_was_search_ = true; |
| + } |
| + return false; |
| + } |
| } |
| - if (!user_input_in_progress && full_text.empty()) |
| - return false; |
| } |
| } else if (!omnibox_popup_is_open || full_text.empty()) { |
| // Update() can be called if the user clicks the preview while composing |
| // text with an IME. If so, we should commit on mouse up, so don't Hide(). |
| - if (!GetPreviewContents() || !loader_->IsPointerDownFromActivate()) |
| - Hide(true); |
| - return false; |
| - } |
| - |
| - // Ensure we have a loader that can process this match. First, try to use the |
| - // TemplateURL of the |match|. If that's invalid, in non-extended mode, stop. |
| - // In extended mode, try using the default search engine, but only when the |
| - // match is for a URL (i.e., not some other kind of non-Instant search). |
| - // A completely blank query shows up as a search, and we do want to allow |
| - // that, hence the "!full_text.empty()" clause. |
| - Profile* const profile = active_tab->profile(); |
| - const bool match_is_search = AutocompleteMatch::IsSearchType(match.type); |
| - if (!ResetLoader(match.GetTemplateURL(profile, false), active_tab) && |
| - (!extended_enabled_ || (match_is_search && !full_text.empty()) || |
| - !CreateDefaultLoader())) { |
| - Hide(true); |
| + if (!loader_ || !loader_->is_pointer_down_from_activate()) |
| + Hide(); |
| return false; |
| } |
| @@ -287,8 +319,9 @@ bool InstantController::Update(const AutocompleteMatch& match, |
| last_match_was_search_ = match_is_search; |
| url_for_history_ = match.destination_url; |
| - // Store the first interaction time for use with latency histograms. |
| - if (first_interaction_time_.is_null()) |
| + // Store the first interaction time for use with latency histograms (but only |
| + // if we are talking to the |loader_| and not a committed tab). |
| + if (!instant_tab_ && first_interaction_time_.is_null()) |
| first_interaction_time_ = base::Time::Now(); |
| // Allow search suggestions. In extended mode, SearchModeChanged() will set |
| @@ -296,7 +329,10 @@ bool InstantController::Update(const AutocompleteMatch& match, |
| if (!extended_enabled_) |
| search_mode_.mode = chrome::search::Mode::MODE_SEARCH_SUGGESTIONS; |
| - loader_->Update(extended_enabled_ ? user_text : full_text, verbatim); |
| + if (instant_tab_) |
| + instant_tab_->Update(user_text, verbatim); |
| + else |
| + loader_->Update(extended_enabled_ ? user_text : full_text, verbatim); |
| content::NotificationService::current()->Notify( |
| chrome::NOTIFICATION_INSTANT_CONTROLLER_UPDATED, |
| @@ -338,7 +374,7 @@ void InstantController::HandleAutocompleteResults( |
| if (!extended_enabled_) |
| return; |
| - if (!GetPreviewContents()) |
| + if (!instant_tab_ && !loader_) |
| return; |
| DVLOG(1) << "AutocompleteResults:"; |
| @@ -360,22 +396,29 @@ void InstantController::HandleAutocompleteResults( |
| } |
| } |
| - loader_->SendAutocompleteResults(results); |
| + if (instant_tab_) |
| + instant_tab_->SendAutocompleteResults(results); |
| + else |
| + loader_->SendAutocompleteResults(results); |
| } |
| bool InstantController::OnUpOrDownKeyPressed(int count) { |
| if (!extended_enabled_) |
| return false; |
| - if (!GetPreviewContents()) |
| + if (!instant_tab_ && !loader_) |
| return false; |
| - loader_->OnUpOrDownKeyPressed(count); |
| - return true; |
| + if (instant_tab_) |
| + instant_tab_->UpOrDownKeyPressed(count); |
| + else |
| + loader_->UpOrDownKeyPressed(count); |
| + |
| + return false; |
|
Jered
2012/11/29 19:57:05
Should this be return true;?
sreeram
2012/12/04 08:10:52
Indeed. Done. ('Twas a typo.)
|
| } |
| -TabContents* InstantController::GetPreviewContents() const { |
| - return loader_ ? loader_->preview_contents() : NULL; |
| +content::WebContents* InstantController::GetPreviewContents() const { |
| + return loader_ ? loader_->contents() : NULL; |
| } |
| bool InstantController::IsCurrent() const { |
| @@ -386,11 +429,25 @@ bool InstantController::CommitIfCurrent(InstantCommitType type) { |
| if (!extended_enabled_ && !instant_enabled_) |
| return false; |
| + // If we are on a committed search results page, send a submit event to the |
| + // page, but otherwise, nothing else to do. |
|
Jered
2012/11/29 19:57:05
From the "nothing else to do" part, I would expect
sreeram
2012/12/04 08:10:52
Done. It worked the way it was before, since IsCur
|
| + if (instant_tab_ && last_match_was_search_ && |
| + type == INSTANT_COMMIT_PRESSED_ENTER) { |
| + instant_tab_->Submit(last_full_text_); |
| + return true; |
| + } |
| + |
| if (!IsCurrent()) |
| return false; |
| DVLOG(1) << "CommitIfCurrent"; |
| - TabContents* preview = loader_->ReleasePreviewContents(type, last_full_text_); |
| + |
| + if (type == INSTANT_COMMIT_FOCUS_LOST) |
| + loader_->Cancel(last_full_text_); |
| + else |
| + loader_->Submit(last_full_text_); |
| + |
| + content::WebContents* preview = loader_->ReleaseContents(); |
| if (extended_enabled_) { |
| // Consider what's happening: |
| @@ -405,7 +462,7 @@ bool InstantController::CommitIfCurrent(InstantCommitType type) { |
| // TODO(samarth,beaudoin): Instead of this hack, we should add a new field |
| // to NavigationEntry to keep track of what the correct query, if any, is. |
| content::NavigationEntry* entry = |
| - preview->web_contents()->GetController().GetVisibleEntry(); |
| + preview->GetController().GetVisibleEntry(); |
| std::string url = entry->GetVirtualURL().spec(); |
| if (!google_util::IsInstantExtendedAPIGoogleSearchUrl(url) && |
| google_util::IsGoogleDomainUrl(url, google_util::ALLOW_SUBDOMAIN, |
| @@ -413,8 +470,8 @@ bool InstantController::CommitIfCurrent(InstantCommitType type) { |
| entry->SetVirtualURL(GURL( |
| url + "#q=" + |
| net::EscapeQueryParamValue(UTF16ToUTF8(last_full_text_), true))); |
| - chrome::search::SearchTabHelper::FromWebContents( |
| - preview->web_contents())->NavigationEntryUpdated(); |
| + chrome::search::SearchTabHelper::FromWebContents(preview)-> |
| + NavigationEntryUpdated(); |
| } |
| } |
| @@ -424,13 +481,12 @@ bool InstantController::CommitIfCurrent(InstantCommitType type) { |
| const history::HistoryAddPageArgs& last_navigation = |
| loader_->last_navigation(); |
| if (!last_navigation.url.is_empty()) { |
| - content::NavigationEntry* entry = |
| - preview->web_contents()->GetController().GetActiveEntry(); |
| + content::NavigationEntry* entry = preview->GetController().GetActiveEntry(); |
| DCHECK_EQ(last_navigation.url, entry->GetURL()); |
| // Add the page to history. |
| HistoryTabHelper* history_tab_helper = |
| - HistoryTabHelper::FromWebContents(preview->web_contents()); |
| + HistoryTabHelper::FromWebContents(preview); |
| history_tab_helper->UpdateHistoryForNavigation(last_navigation); |
| // Update the page title. |
| @@ -439,34 +495,38 @@ bool InstantController::CommitIfCurrent(InstantCommitType type) { |
| // Add a fake history entry with a non-Instant search URL, so that search |
| // terms extraction (for autocomplete history matches) works. |
| - if (HistoryService* history = HistoryServiceFactory::GetForProfile( |
| - preview->profile(), Profile::EXPLICIT_ACCESS)) { |
| + HistoryService* history = HistoryServiceFactory::GetForProfile( |
| + Profile::FromBrowserContext(preview->GetBrowserContext()), |
| + Profile::EXPLICIT_ACCESS); |
| + if (history) { |
| history->AddPage(url_for_history_, base::Time::Now(), NULL, 0, GURL(), |
| history::RedirectList(), last_transition_type_, |
| history::SOURCE_BROWSED, false); |
| } |
| - preview->web_contents()->GetController().PruneAllButActive(); |
| + preview->GetController().PruneAllButActive(); |
| if (type != INSTANT_COMMIT_PRESSED_ALT_ENTER) { |
| - const TabContents* active_tab = browser_->GetActiveTabContents(); |
| + content::WebContents* active_tab = browser_->GetActiveWebContents(); |
| AddSessionStorageHistogram(extended_enabled_, active_tab, preview); |
| - preview->web_contents()->GetController().CopyStateFromAndPrune( |
| - &active_tab->web_contents()->GetController()); |
| + preview->GetController().CopyStateFromAndPrune( |
| + &active_tab->GetController()); |
| } |
| - DeleteLoader(); |
| - |
| // Browser takes ownership of the preview. |
| browser_->CommitInstant(preview, type == INSTANT_COMMIT_PRESSED_ALT_ENTER); |
| content::NotificationService::current()->Notify( |
| chrome::NOTIFICATION_INSTANT_COMMITTED, |
| - content::Source<content::WebContents>(preview->web_contents()), |
| + content::Source<content::WebContents>(preview), |
| content::NotificationService::NoDetails()); |
| + // Hide explicitly. See comments in Hide() for why calling it won't work. |
| model_.SetPreviewState(chrome::search::Mode(), 0, INSTANT_SIZE_PERCENT); |
| + // Delay deletion as we could've gotten here from an InstantLoader method. |
| + MessageLoop::current()->DeleteSoon(FROM_HERE, loader_.release()); |
| + |
| // Try to create another loader immediately so that it is ready for the next |
| // user interaction. |
| CreateDefaultLoader(); |
| @@ -490,14 +550,14 @@ void InstantController::OmniboxLostFocus(gfx::NativeView view_gaining_focus) { |
| } |
| #if defined(OS_MACOSX) |
| - if (!loader_->IsPointerDownFromActivate()) |
| - Hide(true); |
| + if (!loader_->is_pointer_down_from_activate()) |
| + Hide(); |
| #else |
| if (IsViewInContents(GetViewGainingFocus(view_gaining_focus), |
| - GetPreviewContents()->web_contents())) |
| + loader_->contents())) |
| CommitIfCurrent(INSTANT_COMMIT_FOCUS_LOST); |
| else |
| - Hide(true); |
| + Hide(); |
| #endif |
| } |
| @@ -508,8 +568,7 @@ void InstantController::OmniboxGotFocus() { |
| if (!extended_enabled_ && !instant_enabled_) |
| return; |
| - if (!GetPreviewContents()) |
| - CreateDefaultLoader(); |
| + CreateDefaultLoader(); |
| } |
| void InstantController::SearchModeChanged( |
| @@ -525,14 +584,21 @@ void InstantController::SearchModeChanged( |
| if (new_mode.is_search_suggestions()) { |
| // The preview is showing NTP content, but it's not appropriate anymore. |
| - if (model_.mode().is_ntp() && !new_mode.is_origin_ntp()) |
| - Hide(false); |
| + if (model_.mode().is_ntp() && !new_mode.is_origin_ntp()) { |
| + // Hide() clears |last_full_text_|. Restore it so that if the user just |
| + // switched to a tab with a partial query, we don't lose it. |
| + string16 text = last_full_text_; |
| + Hide(); |
| + last_full_text_ = text; |
| + } |
| } else { |
| - Hide(true); |
| + Hide(); |
| } |
| - if (GetPreviewContents()) |
| + if (loader_) |
| loader_->SearchModeChanged(new_mode); |
| + |
| + ResetInstantTab(); |
| } |
| void InstantController::ActiveTabChanged() { |
| @@ -546,21 +612,31 @@ void InstantController::ActiveTabChanged() { |
| // going from search_suggestions to search_suggestions (i.e., partial queries |
| // on both old and new tabs). |
| if (search_mode_.is_search_suggestions() && |
| - model_.mode().is_search_suggestions()) |
| - Hide(false); |
| + model_.mode().is_search_suggestions()) { |
| + // Hide() clears |last_full_text_|. Restore it so that if the user just |
| + // switched to a tab with a partial query, we don't lose it. |
| + string16 text = last_full_text_; |
| + Hide(); |
| + last_full_text_ = text; |
| + } |
| + |
| + if (extended_enabled_) |
| + ResetInstantTab(); |
| } |
| void InstantController::SetInstantEnabled(bool instant_enabled) { |
| instant_enabled_ = instant_enabled; |
| - if (!extended_enabled_ && !instant_enabled_) |
| - DeleteLoader(); |
| + HideInternal(); |
| + loader_.reset(); |
| + if (extended_enabled_ || instant_enabled_) |
| + CreateDefaultLoader(); |
| } |
| void InstantController::ThemeChanged(const ThemeBackgroundInfo& theme_info) { |
| if (!extended_enabled_) |
| return; |
| - if (GetPreviewContents()) |
| + if (loader_) |
| loader_->SendThemeBackgroundInfo(theme_info); |
| } |
| @@ -568,15 +644,20 @@ void InstantController::ThemeAreaHeightChanged(int height) { |
| if (!extended_enabled_) |
| return; |
| - if (GetPreviewContents()) |
| + if (loader_) |
| loader_->SendThemeAreaHeight(height); |
| } |
| void InstantController::SetSuggestions( |
| - InstantLoader* loader, |
| + const content::WebContents* contents, |
| const std::vector<InstantSuggestion>& suggestions) { |
| DVLOG(1) << "SetSuggestions"; |
| - if (loader_ != loader || !search_mode_.is_search_suggestions()) |
| + |
| + // Ignore if we are not currently accepting search suggestions, or if they are |
| + // coming from the wrong loader. |
| + if (!search_mode_.is_search_suggestions() || |
| + (instant_tab_ && instant_tab_->contents() != contents) || |
| + (!instant_tab_ && loader_ && loader_->contents() != contents)) |
|
samarth
2012/12/03 19:46:03
Yikes! The last two clauses appear all over the pl
sreeram
2012/12/04 08:10:52
Actually, it's just in two places - the only two c
|
| return; |
| InstantSuggestion suggestion; |
| @@ -646,87 +727,88 @@ void InstantController::SetSuggestions( |
| Show(INSTANT_SHOWN_QUERY_SUGGESTIONS, 100, INSTANT_SIZE_PERCENT); |
| } |
| -void InstantController::CommitInstantLoader(InstantLoader* loader) { |
| - if (loader_ == loader) |
| - CommitIfCurrent(INSTANT_COMMIT_FOCUS_LOST); |
| +void InstantController::InstantSupportDetermined( |
| + const content::WebContents* contents, |
| + bool supports_instant) { |
| + if (instant_tab_ && instant_tab_->contents() == contents) { |
| + if (!supports_instant) |
|
Jered
2012/11/29 19:57:05
Wow, I hope not. Do we not want to update the blac
sreeram
2012/12/04 08:10:52
No. The blacklist is to make sure that we are not
|
| + MessageLoop::current()->DeleteSoon(FROM_HERE, instant_tab_.release()); |
| + return; |
| + } |
| + |
| + if (loader_ && loader_->contents() == contents) { |
| + if (supports_instant) { |
| + blacklisted_urls_.erase(loader_->instant_url()); |
| + } else { |
| + ++blacklisted_urls_[loader_->instant_url()]; |
| + HideInternal(); |
| + delete loader_->ReleaseContents(); |
| + MessageLoop::current()->DeleteSoon(FROM_HERE, loader_.release()); |
| + CreateDefaultLoader(); |
|
Jered
2012/11/29 19:57:05
Is this CreateDefaultLoader() call new? I think th
sreeram
2012/12/04 08:10:52
Yeah, the call is new. The infinite loop can't hap
|
| + } |
| + content::NotificationService::current()->Notify( |
| + chrome::NOTIFICATION_INSTANT_SUPPORT_DETERMINED, |
| + content::Source<InstantController>(this), |
| + content::NotificationService::NoDetails()); |
| + } |
| } |
| -void InstantController::ShowInstantPreview(InstantLoader* loader, |
| - InstantShownReason reason, |
| +void InstantController::ShowInstantPreview(InstantShownReason reason, |
| int height, |
| InstantSizeUnits units) { |
| - if (loader_ == loader && extended_enabled_) |
| + if (extended_enabled_) |
| Show(reason, height, units); |
| } |
| -void InstantController::InstantSupportDetermined(InstantLoader* loader, |
| - bool supports_instant) { |
| - if (supports_instant) { |
| - blacklisted_urls_.erase(loader->instant_url()); |
| - } else { |
| - ++blacklisted_urls_[loader->instant_url()]; |
| - if (loader_ == loader) |
| - DeleteLoader(); |
| - } |
| - |
| - content::NotificationService::current()->Notify( |
| - chrome::NOTIFICATION_INSTANT_SUPPORT_DETERMINED, |
| - content::Source<InstantController>(this), |
| - content::NotificationService::NoDetails()); |
| -} |
| - |
| -void InstantController::SwappedTabContents(InstantLoader* loader) { |
| - if (loader_ == loader) |
| - model_.SetPreviewContents(GetPreviewContents()); |
| +void InstantController::SwappedWebContents() { |
| + model_.SetPreviewContents(GetPreviewContents()); |
| } |
| -void InstantController::InstantLoaderContentsFocused(InstantLoader* loader) { |
| +void InstantController::InstantLoaderContentsFocused() { |
| #if defined(USE_AURA) |
| // On aura the omnibox only receives a focus lost if we initiate the focus |
| // change. This does that. |
| - if (loader_ == loader && !model_.mode().is_default()) |
| + if (!model_.mode().is_default()) |
| browser_->InstantPreviewFocused(); |
| #endif |
| } |
| bool InstantController::ResetLoader(const TemplateURL* template_url, |
| - const TabContents* active_tab) { |
| + const content::WebContents* active_tab) { |
| std::string instant_url; |
| if (!GetInstantURL(template_url, &instant_url)) |
| return false; |
| - if (GetPreviewContents() && loader_->instant_url() != instant_url) |
| - DeleteLoader(); |
| - |
| - if (!GetPreviewContents()) { |
| - loader_.reset(new InstantLoader(this, instant_url, active_tab)); |
| - loader_->Init(); |
| + if (loader_ && loader_->instant_url() == instant_url) |
| + return true; |
| - // Ensure the searchbox API has the correct theme-related info and context. |
| - if (extended_enabled_) { |
| - browser_->UpdateThemeInfoForPreview(); |
| - loader_->SearchModeChanged(search_mode_); |
| - } |
| + HideInternal(); |
| + loader_.reset(new InstantLoader(this, instant_url)); |
| + loader_->InitContents(active_tab); |
| - // Reset the loader timer. |
| - stale_loader_timer_.Start( |
| - FROM_HERE, |
| - base::TimeDelta::FromMilliseconds(kStaleLoaderTimeoutMS), this, |
| - &InstantController::OnStaleLoader); |
| + // Ensure the searchbox API has the correct theme-related info and context. |
| + if (extended_enabled_) { |
| + browser_->UpdateThemeInfoForPreview(); |
| + loader_->SearchModeChanged(search_mode_); |
| } |
| + // Restart the stale loader timer. |
| + stale_loader_timer_.Start(FROM_HERE, |
| + base::TimeDelta::FromMilliseconds(kStaleLoaderTimeoutMS), this, |
| + &InstantController::OnStaleLoader); |
| + |
| return true; |
| } |
| bool InstantController::CreateDefaultLoader() { |
| // If there's no active tab, the browser is closing. |
| - const TabContents* active_tab = browser_->GetActiveTabContents(); |
| + const content::WebContents* active_tab = browser_->GetActiveWebContents(); |
| if (!active_tab) |
| return false; |
| - const TemplateURL* template_url = |
| - TemplateURLServiceFactory::GetForProfile(active_tab->profile())-> |
| - GetDefaultSearchProvider(); |
| + const TemplateURL* template_url = TemplateURLServiceFactory::GetForProfile( |
| + Profile::FromBrowserContext(active_tab->GetBrowserContext()))-> |
| + GetDefaultSearchProvider(); |
| return ResetLoader(template_url, active_tab); |
| } |
| @@ -737,65 +819,59 @@ void InstantController::OnStaleLoader() { |
| // omnibox loses focus. |
| if (!stale_loader_timer_.IsRunning() && !is_omnibox_focused_ && |
| model_.mode().is_default()) { |
| - DeleteLoader(); |
| + loader_.reset(); |
| CreateDefaultLoader(); |
| } |
| } |
| -void InstantController::DeleteLoader() { |
| - // Clear all state, except |last_transition_type_| as it's used during commit. |
| - last_user_text_.clear(); |
| - last_full_text_.clear(); |
| - last_verbatim_ = false; |
| - last_suggestion_ = InstantSuggestion(); |
| - last_match_was_search_ = false; |
| - if (!extended_enabled_) |
| - search_mode_.mode = chrome::search::Mode::MODE_DEFAULT; |
| - omnibox_bounds_ = gfx::Rect(); |
| - last_omnibox_bounds_ = gfx::Rect(); |
| - update_bounds_timer_.Stop(); |
| - stale_loader_timer_.Stop(); |
| - url_for_history_ = GURL(); |
| - first_interaction_time_ = base::Time(); |
| - if (GetPreviewContents()) { |
| - model_.SetPreviewState(chrome::search::Mode(), 0, INSTANT_SIZE_PERCENT); |
| - loader_->CleanupPreviewContents(); |
| +void InstantController::ResetInstantTab() { |
| + if (search_mode_.is_origin_search()) { |
| + content::WebContents* active_tab = browser_->GetActiveWebContents(); |
| + if (!instant_tab_ || active_tab != instant_tab_->contents()) |
| + instant_tab_.reset(new InstantTab(this, active_tab)); |
| + |
| + // We are now using |instant_tab_| instead of |loader_|, so Hide() the |
| + // latter, but preserve |last_full_text_|. |
|
Jered
2012/11/29 19:57:05
This happens in 3 places. Perhaps extract a method
sreeram
2012/12/04 08:10:52
Ack. No longer repeated in the new patch.
|
| + string16 text = last_full_text_; |
| + Hide(); |
| + last_full_text_ = text; |
| + } else { |
| + instant_tab_.reset(); |
| } |
| +} |
| - // Schedule the deletion for later, since we may have gotten here from a call |
| - // within a |loader_| method (i.e., it's still on the stack). If we deleted |
| - // the loader immediately, things would still be fine so long as the caller |
| - // doesn't access any instance members after we return, but why rely on that? |
| - MessageLoop::current()->DeleteSoon(FROM_HERE, loader_.release()); |
| +void InstantController::Hide() { |
| + HideInternal(); |
| + OnStaleLoader(); |
| } |
| -void InstantController::Hide(bool clear_query) { |
| - DVLOG(1) << "Hide: clear_query=" << clear_query; |
| +void InstantController::HideInternal() { |
| + DVLOG(1) << "Hide"; |
| - // The only time when the preview is not already in the desired MODE_DEFAULT |
| - // state and GetPreviewContents() returns NULL is when we are in the commit |
| - // path. In that case, don't change the state just yet; otherwise we may |
| - // cause the preview to hide unnecessarily. Instead, the state will be set |
| - // correctly after the commit is done. |
| - if (GetPreviewContents()) |
| + // If GetPreviewContents() returns NULL, either we're already in the desired |
| + // MODE_DEFAULT state, or we're in the commit path. For the latter, don't |
| + // change the state just yet; else we may hide the preview unnecessarily. |
| + // Instead, the state will be set correctly after the commit is done. |
| + if (GetPreviewContents()) { |
| model_.SetPreviewState(chrome::search::Mode(), 0, INSTANT_SIZE_PERCENT); |
| - // Clear the first interaction timestamp for later use. |
| - first_interaction_time_ = base::Time(); |
| - |
| - if (clear_query) { |
| - if (GetPreviewContents() && !last_full_text_.empty()) |
| - loader_->Update(string16(), true); |
| - last_user_text_.clear(); |
| - last_full_text_.clear(); |
| + // Send a message asking the preview to clear out old results. |
| + if (!last_full_text_.empty()) { |
| + last_full_text_.clear(); |
| + loader_->Update(last_full_text_, true); |
| + } |
| } |
| - OnStaleLoader(); |
| + // Clear the first interaction timestamp for later use. |
| + first_interaction_time_ = base::Time(); |
| } |
| void InstantController::Show(InstantShownReason reason, |
| int height, |
| InstantSizeUnits units) { |
| + if (instant_tab_) |
| + return; |
| + |
| DVLOG(1) << "Show: reason=" << reason << " height=" << height << " units=" |
| << units; |
| @@ -808,8 +884,8 @@ void InstantController::Show(InstantShownReason reason, |
| !search_mode_.is_search_suggestions()) |
| return; |
| - // If the preview is being shown because of the first set of suggestions to |
| - // arrive for this query editing session, record a histogram value. |
| + // If the preview is being shown for the first time since the user started |
| + // typing, record a histogram value. |
| if (!first_interaction_time_.is_null() && model_.mode().is_default()) { |
| base::TimeDelta delta = base::Time::Now() - first_interaction_time_; |
| UMA_HISTOGRAM_TIMES("Instant.TimeToFirstShow", delta); |
| @@ -820,8 +896,8 @@ void InstantController::Show(InstantShownReason reason, |
| // - The page wants to show custom NTP content. |
| // - The page is over a website other than search or an NTP, and is not |
| // already showing at 100% height. |
| - const bool is_full_height = |
| - model_.height() == 100 && model_.height_units() == INSTANT_SIZE_PERCENT; |
| + bool is_full_height = model_.height() == 100 && |
| + model_.height_units() == INSTANT_SIZE_PERCENT; |
| if (height == 0 || |
| reason == INSTANT_SHOWN_CUSTOM_NTP_CONTENT || |
| (search_mode_.is_origin_default() && !is_full_height)) |
| @@ -831,8 +907,8 @@ void InstantController::Show(InstantShownReason reason, |
| } |
| void InstantController::SendBoundsToPage() { |
| - if (last_omnibox_bounds_ == omnibox_bounds_ || |
| - !GetPreviewContents() || loader_->IsPointerDownFromActivate()) |
| + if (last_omnibox_bounds_ == omnibox_bounds_ || !loader_ || |
| + loader_->is_pointer_down_from_activate()) |
| return; |
| last_omnibox_bounds_ = omnibox_bounds_; |
| @@ -886,8 +962,8 @@ bool InstantController::GetInstantURL(const TemplateURL* template_url, |
| return false; |
| if (!url_obj.SchemeIsSecure()) { |
| - const std::string new_scheme = "https"; |
| - const std::string new_port = "443"; |
| + std::string new_scheme = "https"; |
|
Jered
2012/11/29 19:57:05
There is probably a constant for this somewhere.
sreeram
2012/12/04 08:10:52
I'd think so too, but I couldn't find one!
|
| + std::string new_port = "443"; |
|
Jered
2012/11/29 19:57:05
This too I'd expect.
sreeram
2012/12/04 08:10:52
Likewise. (Note that this is worse, since it needs
|
| GURL::Replacements secure; |
| secure.SetSchemeStr(new_scheme); |
| secure.SetPortStr(new_port); |