Make chrome.omnibox.setDefaultSuggestion work when called inside

chrome/browser/autocomplete/keyword_provider.cc

Index: chrome/browser/autocomplete/keyword_provider.cc
diff --git a/chrome/browser/autocomplete/keyword_provider.cc b/chrome/browser/autocomplete/keyword_provider.cc
index 7c974b9b79d75b0f1551e3c9d360048aa8c91995..c4215c32a0239d492cca4a8715ac246a7eac5c12 100644
--- a/chrome/browser/autocomplete/keyword_provider.cc
+++ b/chrome/browser/autocomplete/keyword_provider.cc
@@ -61,6 +61,9 @@ KeywordProvider::KeywordProvider(ACProviderListener* listener, Profile* profile)
   // suggestions are meant for us.
   registrar_.Add(this, NotificationType::EXTENSION_OMNIBOX_SUGGESTIONS_READY,
                  Source<Profile>(profile->GetOriginalProfile()));
+  registrar_.Add(this,
+                 NotificationType::EXTENSION_OMNIBOX_DEFAULT_SUGGESTION_CHANGED,
+                 Source<Profile>(profile->GetOriginalProfile()));
   registrar_.Add(this, NotificationType::EXTENSION_OMNIBOX_INPUT_ENTERED,
                  Source<Profile>(profile));
 }
@@ -433,56 +436,78 @@ AutocompleteMatch KeywordProvider::CreateAutocompleteMatch(
 void KeywordProvider::Observe(NotificationType type,
                               const NotificationSource& source,
                               const NotificationDetails& details) {
-  if (type == NotificationType::EXTENSION_OMNIBOX_INPUT_ENTERED) {
-    // Input has been accepted, so we're done with this input session. Ensure
-    // we don't send the OnInputCancelled event.
-    current_keyword_extension_id_.clear();
-    return;
-  }
+  TemplateURLModel* model = profile_ ? profile_->GetTemplateURLModel() : model_;
+  const AutocompleteInput& input = extension_suggest_last_input_;
 
-  // TODO(mpcomplete): consider clamping the number of suggestions to
-  // AutocompleteProvider::kMaxMatches.
-  DCHECK(type == NotificationType::EXTENSION_OMNIBOX_SUGGESTIONS_READY);
+  switch (type.value) {
+    case NotificationType::EXTENSION_OMNIBOX_INPUT_ENTERED:
+      // Input has been accepted, so we're done with this input session. Ensure
+      // we don't send the OnInputCancelled event.
+      current_keyword_extension_id_.clear();
+      break;

[Peter Kasting, 2010/12/06 20:05:53]

Nit: Using "return" instead of "break" makes life a little easier on the reader
(I don't have to scan through the rest of the code looking for post-switch
functionality).  (3 places)

[Matt Perry, 2010/12/07 00:10:18]

Done.

+
+    case NotificationType::EXTENSION_OMNIBOX_DEFAULT_SUGGESTION_CHANGED: {
+      // It's possible to change the default suggestion while not in an editing
+      // session.
+      std::wstring keyword, remaining_input;
+      if (matches_.empty() || current_keyword_extension_id_.empty() ||
+          !ExtractKeywordFromInput(input, &keyword, &remaining_input))

[Peter Kasting, 2010/12/06 20:05:53]

Is ExtractKeywordFromInput() supposed to guaranteed-succeed here, like in
EXTENSION_OMNIBOX_SUGGESTIONS_READY below?

[Matt Perry, 2010/12/07 00:10:18]

I *think* it should as long as the first 2 clauses of the conditional are false,
but I wasn't entirely sure about all the edge cases.

+        return;
+
+      const TemplateURL* template_url(model->GetTemplateURLForKeyword(keyword));
+      ApplyDefaultSuggestionForExtensionKeyword(profile_, template_url,
+                                                WideToUTF16(remaining_input),
+                                                &matches_[0]);
+      listener_->OnProviderUpdate(true);
+      break;
+    }
 
-  const ExtensionOmniboxSuggestions& suggestions =
-      *Details<ExtensionOmniboxSuggestions>(details).ptr();
-  if (suggestions.request_id != current_input_id_)
-    return;  // This is an old result. Just ignore.
+    case NotificationType::EXTENSION_OMNIBOX_SUGGESTIONS_READY: {
+      const ExtensionOmniboxSuggestions& suggestions =
+        *Details<ExtensionOmniboxSuggestions>(details).ptr();
+      if (suggestions.request_id != current_input_id_)
+        return;  // This is an old result. Just ignore.
+
+      // TODO(mpcomplete): consider clamping the number of suggestions to

[Peter Kasting, 2010/12/06 20:05:53]

Nit: Doesn't this TODO belong on the next block?  Also, why not go ahead and
implement this?

[Matt Perry, 2010/12/07 00:10:18]

I've moved the TODO. I think I'm going to wait until
http://code.google.com/p/chromium/issues/detail?id=56705 is fixed before doing
anything here.

+      // AutocompleteProvider::kMaxMatches.
+      std::wstring keyword, remaining_input;
+      if (!ExtractKeywordFromInput(input, &keyword, &remaining_input)) {
+        NOTREACHED();
+        return;
+      }
 
-  const AutocompleteInput& input = extension_suggest_last_input_;
-  std::wstring keyword, remaining_input;
-  if (!ExtractKeywordFromInput(input, &keyword, &remaining_input)) {
-    NOTREACHED();
-    return;
-  }
+      for (size_t i = 0; i < suggestions.suggestions.size(); ++i) {
+        const ExtensionOmniboxSuggestion& suggestion =
+            suggestions.suggestions[i];

[Peter Kasting, 2010/12/06 20:05:53]

Nit: You could probably avoid this temp by using an iterator in the loop.  (Then
it would be "i->" instead of "suggestion.".)

[Matt Perry, 2010/12/07 00:10:18]

I also need the index for the relevance calculation below.

+        // We want to order these suggestions in descending order, so start with
+        // the relevance of the first result (added synchronously in Start()),
+        // and subtract 1 for each subsequent suggestion from the extension.
+        // We know that |complete| is true, because we wouldn't get results from
+        // the extension unless the full keyword had been typed.
+        int first_relevance = CalculateRelevance(input.type(), true,
+            input.prefer_keyword(), input.allow_exact_keyword_match());
+        extension_suggest_matches_.push_back(CreateAutocompleteMatch(
+            model, keyword, input, keyword.length(),
+            UTF16ToWide(suggestion.content), first_relevance - (i + 1)));
+
+        AutocompleteMatch* match = &extension_suggest_matches_.back();
+        match->contents.assign(UTF16ToWide(suggestion.description));
+        match->contents_class = suggestion.description_styles;
+        match->description.clear();
+        match->description_class.clear();
+      }
 
-  TemplateURLModel* model =
-      profile_ ? profile_->GetTemplateURLModel() : model_;
-
-  for (size_t i = 0; i < suggestions.suggestions.size(); ++i) {
-    const ExtensionOmniboxSuggestion& suggestion = suggestions.suggestions[i];
-    // We want to order these suggestions in descending order, so start with
-    // the relevance of the first result (added synchronously in Start()),
-    // and subtract 1 for each subsequent suggestion from the extension.
-    // We know that |complete| is true, because we wouldn't get results from
-    // the extension unless the full keyword had been typed.
-    int first_relevance = CalculateRelevance(input.type(), true,
-        input.prefer_keyword(), input.allow_exact_keyword_match());
-    extension_suggest_matches_.push_back(CreateAutocompleteMatch(
-        model, keyword, input, keyword.length(),
-        UTF16ToWide(suggestion.content), first_relevance - (i + 1)));
-
-    AutocompleteMatch* match = &extension_suggest_matches_.back();
-    match->contents.assign(UTF16ToWide(suggestion.description));
-    match->contents_class = suggestion.description_styles;
-    match->description.clear();
-    match->description_class.clear();
-  }
+      done_ = true;
+      matches_.insert(matches_.end(), extension_suggest_matches_.begin(),
+                      extension_suggest_matches_.end());
+      listener_->OnProviderUpdate(!extension_suggest_matches_.empty());
+      break;
+    }
 
-  done_ = true;
-  matches_.insert(matches_.end(), extension_suggest_matches_.begin(),
-                  extension_suggest_matches_.end());
-  listener_->OnProviderUpdate(!extension_suggest_matches_.empty());
+    default:
+      NOTREACHED();
+      break;
+  }
 }
 
 void KeywordProvider::EnterExtensionKeywordMode(