Add a class to parse answer json.

components/omnibox/search_suggestion_parser.cc

 // 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.
 
 #include "components/omnibox/search_suggestion_parser.h"
 
 #include "base/i18n/icu_string_conversions.h"
 #include "base/json/json_string_value_serializer.h"
 #include "base/json/json_writer.h"
 #include "base/logging.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "components/omnibox/autocomplete_input.h"
 #include "components/omnibox/url_prefix.h"
 #include "components/url_fixer/url_fixer.h"
 #include "net/base/net_util.h"
 #include "net/http/http_response_headers.h"
 #include "net/url_request/url_fetcher.h"
 #include "url/url_constants.h"
@@ skipping 37 matching lines @@
 // SearchSuggestionParser::SuggestResult ---------------------------------------
 
 SearchSuggestionParser::SuggestResult::SuggestResult(
     const base::string16& suggestion,
     AutocompleteMatchType::Type type,
     const base::string16& match_contents,
     const base::string16& match_contents_prefix,
     const base::string16& annotation,
     const base::string16& answer_contents,
     const base::string16& answer_type,
+    const SuggestionAnswer* answer,
     const std::string& suggest_query_params,
     const std::string& deletion_url,
     bool from_keyword_provider,
     int relevance,
     bool relevance_from_server,
     bool should_prefetch,
     const base::string16& input_text)
     : Result(from_keyword_provider,
              relevance,
              relevance_from_server,
              type,
              deletion_url),
       suggestion_(suggestion),
       match_contents_prefix_(match_contents_prefix),
       annotation_(annotation),
       suggest_query_params_(suggest_query_params),
       answer_contents_(answer_contents),
       answer_type_(answer_type),
+      answer_(SuggestionAnswer::Copy(answer)),
       should_prefetch_(should_prefetch) {
   match_contents_ = match_contents;
   DCHECK(!match_contents_.empty());
   ClassifyMatchContents(true, input_text);
 }
 
+SearchSuggestionParser::SuggestResult::SuggestResult(
+    const SuggestResult& result)
+    : Result(result),
+      suggestion_(result.suggestion_),
+      match_contents_prefix_(result.match_contents_prefix_),
+      annotation_(result.annotation_),
+      suggest_query_params_(result.suggest_query_params_),
+      answer_contents_(result.answer_contents_),
+      answer_type_(result.answer_type_),
+      answer_(SuggestionAnswer::Copy(result.answer_.get())),
+      should_prefetch_(result.should_prefetch_) {
+}
+
 SearchSuggestionParser::SuggestResult::~SuggestResult() {}
 
+SearchSuggestionParser::SuggestResult&
+    SearchSuggestionParser::SuggestResult::operator=(
+        const SuggestResult& result) {

[groby-ooo-7-16, 2014/10/29 23:51:16]

Idiomatic name for operator='s parameter is rhs. ("Right hand side"). I'd prefer
that.

[Justin Donnelly, 2014/10/30 00:58:22]

Done.

+  if (this == &result)
+    return *this;
+
+  Result::operator=(result);

[groby-ooo-7-16, 2014/10/29 23:51:16]

Gah. It's valid C++, but horrible to read.

static_cast<Result>(*this) = rhs;

or at least
this->Result::operator=(rhs)

[Justin Donnelly, 2014/10/30 00:58:22]

Done.

+  suggestion_ = result.suggestion_;
+  match_contents_prefix_ = result.match_contents_prefix_;
+  annotation_ = result.annotation_;
+  suggest_query_params_ = result.suggest_query_params_;
+  answer_contents_ = result.answer_contents_;
+  answer_type_ = result.answer_type_;
+  answer_ = SuggestionAnswer::Copy(result.answer_.get());
+  should_prefetch_ = result.should_prefetch_;
+
+  return *this;
+}
+
 void SearchSuggestionParser::SuggestResult::ClassifyMatchContents(
     const bool allow_bolding_all,
     const base::string16& input_text) {
   if (input_text.empty()) {
     // In case of zero-suggest results, do not highlight matches.
     match_contents_class_.push_back(
         ACMatchClassification(0, ACMatchClassification::NONE));
     return;
   }
 
@@ skipping 328 matching lines @@
         results->navigation_results.push_back(NavigationResult(
             scheme_classifier, url, match_type, title, deletion_url,
             is_keyword_result, relevance, relevances != NULL, input.text(),
             languages));
       }
     } else {
       base::string16 match_contents = suggestion;
       base::string16 match_contents_prefix;
       base::string16 annotation;
       base::string16 answer_contents;
-      base::string16 answer_type;
+      base::string16 answer_type_str;
+      scoped_ptr<SuggestionAnswer> answer;
       std::string suggest_query_params;
 
       if (suggestion_details) {
         suggestion_details->GetDictionary(index, &suggestion_detail);
         if (suggestion_detail) {
           suggestion_detail->GetString("t", &match_contents);
           suggestion_detail->GetString("mp", &match_contents_prefix);
           // Error correction for bad data from server.
           if (match_contents.empty())
             match_contents = suggestion;
           suggestion_detail->GetString("a", &annotation);
           suggestion_detail->GetString("q", &suggest_query_params);
 
           // Extract Answers, if provided.
           const base::DictionaryValue* answer_json = NULL;
-          if (suggestion_detail->GetDictionary("ansa", &answer_json)) {
+          int answer_type;
+          if (suggestion_detail->GetDictionary("ansa", &answer_json) &&
+              suggestion_detail->GetString("ansb", &answer_type_str) &&
+              base::StringToInt(answer_type_str, &answer_type) &&
+              SuggestionAnswer::ParseAnswer(answer_json, &answer)) {
             match_type = AutocompleteMatchType::SEARCH_SUGGEST_ANSWER;
-            GetAnswersImageURLs(answer_json, &results->answers_image_urls);
+            answer->set_type(answer_type);
+
             std::string contents;
             base::JSONWriter::Write(answer_json, &contents);
             answer_contents = base::UTF8ToUTF16(contents);
-            suggestion_detail->GetString("ansb", &answer_type);
+
+            answer->GetImageURLs(&results->answers_image_urls);
           }
         }
       }
 
       bool should_prefetch = static_cast<int>(index) == prefetch_index;
       // TODO(kochi): Improve calculator suggestion presentation.
       results->suggest_results.push_back(SuggestResult(
           base::CollapseWhitespace(suggestion, false), match_type,
           base::CollapseWhitespace(match_contents, false),
-          match_contents_prefix, annotation, answer_contents, answer_type,
-          suggest_query_params, deletion_url, is_keyword_result, relevance,
-          relevances != NULL, should_prefetch, trimmed_input));
+          match_contents_prefix, annotation, answer_contents, answer_type_str,
+          answer.get(), suggest_query_params, deletion_url, is_keyword_result,

[groby-ooo-7-16, 2014/10/29 23:51:16]

It bugs me that this is yet another unnecessary copy...

[Justin Donnelly, 2014/10/30 00:58:22]

Done.

+          relevance, relevances != NULL, should_prefetch, trimmed_input));
     }
   }
   results->relevances_from_server = relevances != NULL;
   return true;
 }
-
-// static
-void SearchSuggestionParser::GetAnswersImageURLs(
-    const base::DictionaryValue* answer_json,
-    std::vector<GURL>* urls) {
-  DCHECK(answer_json);
-
-  const base::ListValue* lines = NULL;
-  if (!answer_json->GetList("l", &lines) || !lines || lines->GetSize() == 0)
-    return;
-
-  for (base::ListValue::const_iterator iter = lines->begin();
-       iter != lines->end();
-       ++iter) {
-    const base::DictionaryValue* line = NULL;
-    if (!(*iter)->GetAsDictionary(&line) || !line)
-      continue;
-
-    std::string image_host_and_path;
-    if (!line->GetString("il.i.d", &image_host_and_path) ||
-        image_host_and_path.empty())
-      continue;
-    // Concatenate scheme and host/path using only ':' as separator. This is
-    // due to the results delivering strings of the form '//host/path', which
-    // is web-speak for "use the enclosing page's scheme", but not a valid path
-    // of an URL.
-    GURL image_url(
-        GURL(std::string(url::kHttpsScheme) + ":" + image_host_and_path));
-    if (image_url.is_valid())
-      urls->push_back(image_url);
-  }
-}