Omnibox: Prevent Asynchronous Suggestions from Changing Default Match

components/autocomplete/search_suggestion_parser.h

 // Copyright 2014 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.
 
 #ifndef COMPONENTS_AUTOCOMPLETE_SEARCH_SUGGESTION_PARSER_H_
 #define COMPONENTS_AUTOCOMPLETE_SEARCH_SUGGESTION_PARSER_H_
 
 #include <string>
 #include <vector>
 
@@ skipping 36 matching lines @@
     bool from_keyword_provider() const { return from_keyword_provider_; }
 
     const base::string16& match_contents() const { return match_contents_; }
     const ACMatchClassifications& match_contents_class() const {
       return match_contents_class_;
     }
 
     AutocompleteMatchType::Type type() const { return type_; }
     int relevance() const { return relevance_; }
     void set_relevance(int relevance) { relevance_ = relevance; }
+    bool never_allowed_to_be_default_match() const {
+      return never_allowed_to_be_default_match_;
+    }
+    void set_never_allowed_to_be_default_match(
+        bool never_allowed_to_be_default_match) {
+      never_allowed_to_be_default_match_ = never_allowed_to_be_default_match;
+    }
 
     bool relevance_from_server() const { return relevance_from_server_; }
     void set_relevance_from_server(bool relevance_from_server) {
       relevance_from_server_ = relevance_from_server;
     }
 
     const std::string& deletion_url() const { return deletion_url_; }
 
     // Returns the default relevance value for this result (which may
     // be left over from a previous omnibox input) given the current
@@ skipping 16 matching lines @@
     int relevance_;
 
    private:
     // Whether this result's relevance score was fully or partly calculated
     // based on server information, and thus is assumed to be more accurate.
     // This is ultimately used in
     // SearchProvider::ConvertResultsToAutocompleteMatches(), see comments
     // there.
     bool relevance_from_server_;
 
+    // If true, this result should never be allowed to be the default match.

[msw, 2014/08/14 02:57:23]

Hmm, seems like this identifier and the relevant code could be a bit clearer if
this were |received_since_last_keystroke_|, always set true on receipt, and set
to false for those results which survive the next keystroke's SortAndCull (or
whatever the corresponding result/match cleanup for re-use is called nowadays).

[Mark P, 2014/08/15 00:05:56]

Done (received_after_last_keystroke).  I agree that this makes the explanation
of what's going on clearer.
Done.
Now the logic to has moved from Start() (when minimal_changes is false) to
RemoveAllStaleResults(), which I renamed to RemoveOrReviseOldResults().

+    // This field is ignored for verbatim results; those are regardless
+    // always allowed to be the default match.
+    bool never_allowed_to_be_default_match_;
+
     // Optional deletion URL provided with suggestions. Fetching this URL
     // should result in some reasonable deletion behaviour on the server,
     // e.g. deleting this term out of a user's server-side search history.
     std::string deletion_url_;
   };
 
   class SuggestResult : public Result {
    public:
     SuggestResult(const base::string16& suggestion,
                   AutocompleteMatchType::Type type,
@@ skipping 144 matching lines @@
     std::string metadata;
 
     // If the active suggest field trial (if any) has triggered.
     bool field_trial_triggered;
 
     // If the relevance values of the results are from the server.
     bool relevances_from_server;
 
     // URLs of any images in Answers results.
     std::vector<GURL> answers_image_urls;
-
-   private:
-    DISALLOW_COPY_AND_ASSIGN(Results);

[msw, 2014/08/14 02:57:23]

Why are you removing this? To allow the assignment of |default_results_ =
orig_default_results_;|, etc.? I think that's unnecessary in my suggested
feature implementation. This deserved an explicit CL description comment, or at
least a CL review comment explanation... Copying is bad, m'kay? :)

[Mark P, 2014/08/15 00:05:56]

Restored.  I explained why I did this in a previous e-mail.  With my new
approach, this is not necessary.

   };
 
   // Extracts JSON data fetched by |source| and converts it to UTF-8.
   static std::string ExtractJsonData(const net::URLFetcher* source);
 
   // Parses JSON response received from the provider, stripping XSSI
   // protection if needed. Returns the parsed data if successful, NULL
   // otherwise.
   static scoped_ptr<base::Value> DeserializeJsonData(std::string json_data);
 
@@ skipping 12 matching lines @@
 
  private:
   // Gets URLs of any images in Answers results.
   static void GetAnswersImageURLs(const base::DictionaryValue* answer_json,
                                   std::vector<GURL>* urls);
 
   DISALLOW_COPY_AND_ASSIGN(SearchSuggestionParser);
 };
 
 #endif  // COMPONENTS_AUTOCOMPLETE_SEARCH_SUGGESTION_PARSER_H_