summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--chrome/browser/autocomplete/autocomplete.cc21
-rw-r--r--chrome/browser/autocomplete/autocomplete.h10
-rw-r--r--chrome/browser/autocomplete/autocomplete_edit.cc80
-rw-r--r--chrome/browser/autocomplete/autocomplete_edit.h3
-rw-r--r--chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc37
-rw-r--r--chrome/browser/autocomplete/autocomplete_popup_model.cc43
-rw-r--r--chrome/browser/autocomplete/autocomplete_popup_model.h15
-rw-r--r--chrome/browser/autocomplete/autocomplete_unittest.cc2
-rw-r--r--chrome/common/notification_type.h13
9 files changed, 111 insertions, 113 deletions
diff --git a/chrome/browser/autocomplete/autocomplete.cc b/chrome/browser/autocomplete/autocomplete.cc
index bd8cf16..5f8f681 100644
--- a/chrome/browser/autocomplete/autocomplete.cc
+++ b/chrome/browser/autocomplete/autocomplete.cc
@@ -872,13 +872,9 @@ void AutocompleteController::Stop(bool clear_result) {
done_ = true;
if (clear_result && !result_.empty()) {
result_.Reset();
- NotificationService::current()->Notify(
- NotificationType::AUTOCOMPLETE_CONTROLLER_RESULT_UPDATED,
- Source<AutocompleteController>(this),
- Details<const AutocompleteResult>(&result_));
- // NOTE: We don't notify AUTOCOMPLETE_CONTROLLER_DEFAULT_MATCH_UPDATED since
- // we're trying to only clear the popup, not touch the edit... this is all
- // a mess and should be cleaned up :(
+ // NOTE: We pass in false since we're trying to only clear the popup, not
+ // touch the edit... this is all a mess and should be cleaned up :(
+ NotifyChanged(false);
}
}
@@ -951,16 +947,7 @@ void AutocompleteController::NotifyChanged(bool notify_default_match) {
NotificationService::current()->Notify(
NotificationType::AUTOCOMPLETE_CONTROLLER_RESULT_UPDATED,
Source<AutocompleteController>(this),
- Details<const AutocompleteResult>(&result_));
- if (notify_default_match) {
- // This notification must be sent after the other so the popup has time to
- // update its state before the edit calls into it.
- // TODO(pkasting): Eliminate this ordering requirement.
- NotificationService::current()->Notify(
- NotificationType::AUTOCOMPLETE_CONTROLLER_DEFAULT_MATCH_UPDATED,
- Source<AutocompleteController>(this),
- Details<const AutocompleteResult>(&result_));
- }
+ Details<bool>(&notify_default_match));
}
void AutocompleteController::CheckIfDone() {
diff --git a/chrome/browser/autocomplete/autocomplete.h b/chrome/browser/autocomplete/autocomplete.h
index ea6382c..148209f 100644
--- a/chrome/browser/autocomplete/autocomplete.h
+++ b/chrome/browser/autocomplete/autocomplete.h
@@ -615,12 +615,10 @@ class AutocompleteController : public ACProviderListener {
// return matches which are synchronously available, which should mean that
// all providers will be done immediately.
//
- // The controller will fire AUTOCOMPLETE_CONTROLLER_DEFAULT_MATCH_UPDATED from
- // inside this call, and unless the query is stopped, will fire at least one
- // (and perhaps more) AUTOCOMPLETE_CONTROLLER_RESULT_UPDATED later as more
- // matches come in (even if the query completes synchronously). Listeners
- // should use the result set provided in the accompanying Details object to
- // update themselves.
+ // The controller will fire AUTOCOMPLETE_CONTROLLER_RESULT_UPDATED from
+ // inside this call at least once. If matches are available later on that
+ // result in changing the result set AUTOCOMPLETE_CONTROLLER_RESULT_UPDATED
+ // is sent again.
void Start(const string16& text,
const string16& desired_tld,
bool prevent_inline_autocomplete,
diff --git a/chrome/browser/autocomplete/autocomplete_edit.cc b/chrome/browser/autocomplete/autocomplete_edit.cc
index d8681d7..5f3aeef 100644
--- a/chrome/browser/autocomplete/autocomplete_edit.cc
+++ b/chrome/browser/autocomplete/autocomplete_edit.cc
@@ -15,6 +15,7 @@
#include "chrome/browser/autocomplete/autocomplete_edit_view.h"
#include "chrome/browser/autocomplete/autocomplete_match.h"
#include "chrome/browser/autocomplete/autocomplete_popup_model.h"
+#include "chrome/browser/autocomplete/autocomplete_popup_view.h"
#include "chrome/browser/autocomplete/keyword_provider.h"
#include "chrome/browser/browser_list.h"
#include "chrome/browser/command_updater.h"
@@ -82,7 +83,7 @@ AutocompleteEditModel::~AutocompleteEditModel() {
void AutocompleteEditModel::SetPopupModel(AutocompletePopupModel* popup_model) {
popup_ = popup_model;
registrar_.Add(this,
- NotificationType::AUTOCOMPLETE_CONTROLLER_DEFAULT_MATCH_UPDATED,
+ NotificationType::AUTOCOMPLETE_CONTROLLER_RESULT_UPDATED,
Source<AutocompleteController>(popup_->autocomplete_controller()));
}
@@ -147,8 +148,6 @@ bool AutocompleteEditModel::UpdatePermanentText(
void AutocompleteEditModel::SetUserText(const string16& text) {
SetInputInProgress(true);
- keyword_.clear();
- is_keyword_hint_ = false;
InternalSetUserText(text);
paste_state_ = NONE;
has_temporary_text_ = false;
@@ -688,14 +687,6 @@ void AutocompleteEditModel::PopupBoundsChangedTo(const gfx::Rect& bounds) {
controller_->OnPopupBoundsChanged(bounds);
}
-void AutocompleteEditModel::OnPopupClosed() {
- // Accepts the temporary text as the user text, because it makes little
- // sense to have temporary text when the popup is closed.
- InternalSetUserText(UserTextFromDisplayText(view_->GetText()));
- has_temporary_text_ = false;
- PopupBoundsChangedTo(gfx::Rect());
-}
-
// Return true if the suggestion type warrants a TCP/IP preconnection.
// i.e., it is now highly likely that the user will select the related domain.
static bool IsPreconnectable(AutocompleteMatch::Type type) {
@@ -718,36 +709,53 @@ static bool IsPreconnectable(AutocompleteMatch::Type type) {
void AutocompleteEditModel::Observe(NotificationType type,
const NotificationSource& source,
const NotificationDetails& details) {
- DCHECK_EQ(NotificationType::AUTOCOMPLETE_CONTROLLER_DEFAULT_MATCH_UPDATED,
+ DCHECK_EQ(NotificationType::AUTOCOMPLETE_CONTROLLER_RESULT_UPDATED,
type.value);
- string16 inline_autocomplete_text;
- string16 keyword;
- bool is_keyword_hint = false;
- const AutocompleteResult* result =
- Details<const AutocompleteResult>(details).ptr();
- const AutocompleteResult::const_iterator match(result->default_match());
- if (match != result->end()) {
- if ((match->inline_autocomplete_offset != string16::npos) &&
- (match->inline_autocomplete_offset < match->fill_into_edit.length())) {
- inline_autocomplete_text =
- match->fill_into_edit.substr(match->inline_autocomplete_offset);
- }
-
- if (!match->destination_url.SchemeIs(chrome::kExtensionScheme)) {
- // Warm up DNS Prefetch cache, or preconnect to a search service.
- chrome_browser_net::AnticipateOmniboxUrl(match->destination_url,
- IsPreconnectable(match->type));
+ const bool was_open = popup_->view()->IsOpen();
+ if (*(Details<bool>(details).ptr())) {
+ string16 inline_autocomplete_text;
+ string16 keyword;
+ bool is_keyword_hint = false;
+ const AutocompleteResult& result =
+ popup_->autocomplete_controller()->result();
+ const AutocompleteResult::const_iterator match(result.default_match());
+ if (match != result.end()) {
+ if ((match->inline_autocomplete_offset != string16::npos) &&
+ (match->inline_autocomplete_offset <
+ match->fill_into_edit.length())) {
+ inline_autocomplete_text =
+ match->fill_into_edit.substr(match->inline_autocomplete_offset);
+ }
+
+ if (!match->destination_url.SchemeIs(chrome::kExtensionScheme)) {
+ // Warm up DNS Prefetch cache, or preconnect to a search service.
+ chrome_browser_net::AnticipateOmniboxUrl(match->destination_url,
+ IsPreconnectable(match->type));
+ }
+
+ // We could prefetch the alternate nav URL, if any, but because there
+ // can be many of these as a user types an initial series of characters,
+ // the OS DNS cache could suffer eviction problems for minimal gain.
+
+ is_keyword_hint = popup_->GetKeywordForMatch(*match, &keyword);
}
-
- // We could prefetch the alternate nav URL, if any, but because there
- // can be many of these as a user types an initial series of characters,
- // the OS DNS cache could suffer eviction problems for minimal gain.
-
- is_keyword_hint = popup_->GetKeywordForMatch(*match, &keyword);
+ popup_->OnResultChanged();
+ OnPopupDataChanged(inline_autocomplete_text, NULL, keyword,
+ is_keyword_hint);
+ } else {
+ popup_->OnResultChanged();
}
- OnPopupDataChanged(inline_autocomplete_text, NULL, keyword, is_keyword_hint);
+ if (popup_->view()->IsOpen()) {
+ PopupBoundsChangedTo(popup_->view()->GetTargetBounds());
+ } else if (was_open) {
+ // Accepts the temporary text as the user text, because it makes little
+ // sense to have temporary text when the popup is closed.
+ InternalSetUserText(UserTextFromDisplayText(view_->GetText()));
+ has_temporary_text_ = false;
+ PopupBoundsChangedTo(gfx::Rect());
+ }
}
void AutocompleteEditModel::InternalSetUserText(const string16& text) {
diff --git a/chrome/browser/autocomplete/autocomplete_edit.h b/chrome/browser/autocomplete/autocomplete_edit.h
index b407b11..8a94be4 100644
--- a/chrome/browser/autocomplete/autocomplete_edit.h
+++ b/chrome/browser/autocomplete/autocomplete_edit.h
@@ -324,9 +324,6 @@ class AutocompleteEditModel : public NotificationObserver {
// Invoked when the popup is going to change its bounds to |bounds|.
void PopupBoundsChangedTo(const gfx::Rect& bounds);
- // Called when the popup is closed.
- void OnPopupClosed();
-
private:
enum PasteState {
NONE, // Most recent edit was not a paste.
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc b/chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc
index 0e9c8c2..0cb8fdf 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc
@@ -994,6 +994,33 @@ class AutocompleteEditViewTest : public InProcessBrowserTest,
ASSERT_FALSE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_LOCATION_BAR));
}
+
+ void PersistKeywordModeOnTabSwitch() {
+ AutocompleteEditView* edit_view = NULL;
+ ASSERT_NO_FATAL_FAILURE(GetAutocompleteEditView(&edit_view));
+
+ // Trigger keyword hint mode.
+ ASSERT_NO_FATAL_FAILURE(SendKeySequence(kSearchKeywordKeys));
+ ASSERT_TRUE(edit_view->model()->is_keyword_hint());
+ ASSERT_EQ(kSearchKeyword, UTF16ToUTF8(edit_view->model()->keyword()));
+
+ // Trigger keyword mode.
+ ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_TAB, false, false, false));
+ ASSERT_FALSE(edit_view->model()->is_keyword_hint());
+ ASSERT_EQ(kSearchKeyword, UTF16ToUTF8(edit_view->model()->keyword()));
+
+ // Input something as search text.
+ ASSERT_NO_FATAL_FAILURE(SendKeySequence(kSearchTextKeys));
+
+ // Create a new tab.
+ browser()->NewTab();
+
+ // Switch back to the first tab.
+ browser()->SelectTabContentsAt(0, true);
+
+ // Make sure we're still in keyword mode.
+ ASSERT_EQ(kSearchKeyword, UTF16ToUTF8(edit_view->model()->keyword()));
+ }
};
// Test if ctrl-* accelerators are workable in omnibox.
@@ -1059,6 +1086,11 @@ IN_PROC_BROWSER_TEST_F(AutocompleteEditViewTest, TabMoveCursorToEnd) {
TabMoveCursorToEndTest();
}
+IN_PROC_BROWSER_TEST_F(AutocompleteEditViewTest,
+ PersistKeywordModeOnTabSwitch) {
+ PersistKeywordModeOnTabSwitch();
+}
+
#if defined(OS_LINUX)
// TODO(oshima): enable these tests for views-implmentation when
// these featuers are supported.
@@ -1251,4 +1283,9 @@ IN_PROC_BROWSER_TEST_F(AutocompleteEditViewViewsTest,
TabMoveCursorToEndTest();
}
+IN_PROC_BROWSER_TEST_F(AutocompleteEditViewViewsTest,
+ PersistKeywordModeOnTabSwitch) {
+ PersistKeywordModeOnTabSwitch();
+}
+
#endif
diff --git a/chrome/browser/autocomplete/autocomplete_popup_model.cc b/chrome/browser/autocomplete/autocomplete_popup_model.cc
index 9e2d43b..6809d0c 100644
--- a/chrome/browser/autocomplete/autocomplete_popup_model.cc
+++ b/chrome/browser/autocomplete/autocomplete_popup_model.cc
@@ -18,8 +18,6 @@
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/search_engines/template_url.h"
#include "chrome/browser/search_engines/template_url_model.h"
-#include "chrome/common/notification_details.h"
-#include "chrome/common/notification_source.h"
#include "ui/gfx/rect.h"
///////////////////////////////////////////////////////////////////////////////
@@ -35,8 +33,6 @@ AutocompletePopupModel::AutocompletePopupModel(
profile_(profile),
hovered_line_(kNoMatch),
selected_line_(kNoMatch) {
- registrar_.Add(this, NotificationType::AUTOCOMPLETE_CONTROLLER_RESULT_UPDATED,
- Source<AutocompleteController>(controller_.get()));
}
AutocompletePopupModel::~AutocompletePopupModel() {
@@ -287,38 +283,27 @@ void AutocompletePopupModel::TryDeletingCurrentItem() {
}
}
-void AutocompletePopupModel::Observe(NotificationType type,
- const NotificationSource& source,
- const NotificationDetails& details) {
- DCHECK_EQ(NotificationType::AUTOCOMPLETE_CONTROLLER_RESULT_UPDATED,
- type.value);
+const SkBitmap* AutocompletePopupModel::GetSpecialIconForMatch(
+ const AutocompleteMatch& match) const {
+ if (!match.template_url || !match.template_url->IsExtensionKeyword())
+ return NULL;
- const AutocompleteResult* result =
- Details<const AutocompleteResult>(details).ptr();
- selected_line_ = result->default_match() == result->end() ?
- kNoMatch : static_cast<size_t>(result->default_match() - result->begin());
+ return &profile_->GetExtensionService()->GetOmniboxPopupIcon(
+ match.template_url->GetExtensionId());
+}
+
+void AutocompletePopupModel::OnResultChanged() {
+ const AutocompleteResult& result = controller_->result();
+ selected_line_ = result.default_match() == result.end() ?
+ kNoMatch : static_cast<size_t>(result.default_match() - result.begin());
// There had better not be a nonempty result set with no default match.
- CHECK((selected_line_ != kNoMatch) || result->empty());
+ CHECK((selected_line_ != kNoMatch) || result.empty());
manually_selected_match_.Clear();
// If we're going to trim the window size to no longer include the hovered
// line, turn hover off. Practically, this shouldn't happen, but it
// doesn't hurt to be defensive.
- if ((hovered_line_ != kNoMatch) && (result->size() <= hovered_line_))
+ if ((hovered_line_ != kNoMatch) && (result.size() <= hovered_line_))
SetHoveredLine(kNoMatch);
- const bool was_open = view_->IsOpen();
view_->UpdatePopupAppearance();
- if (view_->IsOpen())
- edit_model_->PopupBoundsChangedTo(view_->GetTargetBounds());
- else if (was_open)
- edit_model_->OnPopupClosed();
-}
-
-const SkBitmap* AutocompletePopupModel::GetSpecialIconForMatch(
- const AutocompleteMatch& match) const {
- if (!match.template_url || !match.template_url->IsExtensionKeyword())
- return NULL;
-
- return &profile_->GetExtensionService()->GetOmniboxPopupIcon(
- match.template_url->GetExtensionId());
}
diff --git a/chrome/browser/autocomplete/autocomplete_popup_model.h b/chrome/browser/autocomplete/autocomplete_popup_model.h
index 37687a5..6402ff2 100644
--- a/chrome/browser/autocomplete/autocomplete_popup_model.h
+++ b/chrome/browser/autocomplete/autocomplete_popup_model.h
@@ -8,8 +8,6 @@
#include "base/scoped_ptr.h"
#include "chrome/browser/autocomplete/autocomplete.h"
-#include "chrome/common/notification_observer.h"
-#include "chrome/common/notification_registrar.h"
class AutocompleteEditModel;
class AutocompleteEditView;
@@ -18,7 +16,7 @@ class SkBitmap;
class AutocompletePopupView;
-class AutocompletePopupModel : public NotificationObserver {
+class AutocompletePopupModel {
public:
AutocompletePopupModel(AutocompletePopupView* popup_view,
AutocompleteEditModel* edit_model,
@@ -128,23 +126,20 @@ class AutocompletePopupModel : public NotificationObserver {
Profile* profile() const { return profile_; }
+ // Invoked from the edit model any time the result set of the controller
+ // changes.
+ void OnResultChanged();
+
// The token value for selected_line_, hover_line_ and functions dealing with
// a "line number" that indicates "no line".
static const size_t kNoMatch = -1;
private:
- // NotificationObserver
- virtual void Observe(NotificationType type,
- const NotificationSource& source,
- const NotificationDetails& details);
-
AutocompletePopupView* view_;
AutocompleteEditModel* edit_model_;
scoped_ptr<AutocompleteController> controller_;
- NotificationRegistrar registrar_;
-
// Profile for current tab.
Profile* profile_;
diff --git a/chrome/browser/autocomplete/autocomplete_unittest.cc b/chrome/browser/autocomplete/autocomplete_unittest.cc
index 332fedf..93ce9da 100644
--- a/chrome/browser/autocomplete/autocomplete_unittest.cc
+++ b/chrome/browser/autocomplete/autocomplete_unittest.cc
@@ -226,7 +226,7 @@ void AutocompleteProviderTest::Observe(NotificationType type,
const NotificationSource& source,
const NotificationDetails& details) {
if (controller_->done()) {
- result_.CopyFrom(*(Details<const AutocompleteResult>(details).ptr()));
+ result_.CopyFrom(controller_->result());
MessageLoop::current()->Quit();
}
}
diff --git a/chrome/common/notification_type.h b/chrome/common/notification_type.h
index 174138c..0c390939 100644
--- a/chrome/common/notification_type.h
+++ b/chrome/common/notification_type.h
@@ -784,19 +784,10 @@ class NotificationType {
// Autocomplete ------------------------------------------------------------
- // Sent by the autocomplete controller at least once per query, each time
- // new matches are available, subject to rate-limiting/coalescing to reduce
- // the number of updates. The details hold the AutocompleteResult that
- // observers should use if they want to see the updated matches.
+ // Sent by the autocomplete controller each time the result set updates.
+ // The details is a boolean indicating if the default match has changed.
AUTOCOMPLETE_CONTROLLER_RESULT_UPDATED,
- // Sent by the autocomplete controller immediately after synchronous matches
- // become available, and thereafter at the same time that
- // AUTOCOMPLETE_CONTROLLER_RESULT_UPDATED is sent. The details hold the
- // AutocompleteResult that observers should use if they want to see the
- // up-to-date matches.
- AUTOCOMPLETE_CONTROLLER_DEFAULT_MATCH_UPDATED,
-
// This is sent when an item of the Omnibox popup is selected. The source
// is the profile.
OMNIBOX_OPENED_URL,