Add a class to parse answer json.

components/omnibox/suggestion_answer.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/suggestion_answer.h"
+
+#include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/values.h"
+#include "url/url_constants.h"
+
+namespace {
+
+// All of these are defined here (even though most are only used once each) so
+// the format details are easy to locate and update or compare to the spec doc.
+static const char* kAnswerJsonLines = "l";
+static const char* kAnswerJsonImageLine = "il";

[groby-ooo-7-16, 2014/10/30 20:28:29]

text constants should always be const char arrays, e.g.

static const char kAnswerImageLine[] = "il";

[Justin Donnelly, 2014/10/30 21:38:03]

Done.

+static const char* kAnswerJsonText = "t";
+static const char* kAnswerJsonAdditionalText = "at";
+static const char* kAnswerJsonStatusText = "st";
+static const char* kAnswerJsonTextType = "tt";
+static const char* kAnswerJsonImage = "i";
+static const char* kAnswerJsonImageData = "i.d";
+
+}  // namespace
+
+// SuggestionAnswer::TextField -------------------------------------------------
+
+SuggestionAnswer::TextField::TextField() : type_(-1) {}
+SuggestionAnswer::TextField::~TextField() {}
+
+// static
+bool SuggestionAnswer::TextField::ParseTextField(
+    const base::DictionaryValue* field_json, TextField* text_field) {
+  return field_json->GetString(kAnswerJsonText, &text_field->text_) &&
+      !text_field->text_.empty() &&
+      field_json->GetInteger(kAnswerJsonTextType, &text_field->type_);
+}
+
+bool SuggestionAnswer::TextField::Equals(const TextField& field) const {
+  return type_ == field.type_ && text_ == field.text_;
+}
+
+// SuggestionAnswer::ImageLine -------------------------------------------------
+
+SuggestionAnswer::ImageLine::ImageLine() {}
+SuggestionAnswer::ImageLine::ImageLine(const ImageLine& line)
+    : text_fields_(line.text_fields_),
+      additional_text_(line.additional_text_ ?
+                       new TextField(*line.additional_text_) : nullptr),
+      status_text_(line.status_text_ ?
+                   new TextField(*line.status_text_) : nullptr),
+      image_url_(line.image_url_) {}
+
+SuggestionAnswer::ImageLine::~ImageLine() {}
+
+// static
+bool SuggestionAnswer::ImageLine::ParseImageLine(
+    const base::DictionaryValue* line_json, ImageLine* image_line) {
+  const base::DictionaryValue* inner_json;
+  if (!line_json->GetDictionary(kAnswerJsonImageLine, &inner_json))
+    return false;
+
+  const base::ListValue* fields_json;
+  if (!inner_json->GetList(kAnswerJsonText, &fields_json) ||
+      fields_json->GetSize() == 0)
+    return false;
+
+  for (size_t i = 0; i < fields_json->GetSize(); ++i) {
+    const base::DictionaryValue* field_json;
+    TextField text_field;
+    if (!fields_json->GetDictionary(i, &field_json) ||
+        !TextField::ParseTextField(field_json, &text_field))
+      return false;
+    image_line->text_fields_.push_back(text_field);
+  }
+
+  if (inner_json->HasKey(kAnswerJsonAdditionalText)) {
+    image_line->additional_text_.reset(new TextField());
+    const base::DictionaryValue* field_json;
+    if (!inner_json->GetDictionary(kAnswerJsonAdditionalText, &field_json) ||
+        !TextField::ParseTextField(field_json,
+                                   image_line->additional_text_.get()))
+      return false;
+  }
+
+  if (inner_json->HasKey(kAnswerJsonStatusText)) {
+    image_line->status_text_.reset(new TextField());
+    const base::DictionaryValue* field_json;
+    if (!inner_json->GetDictionary(kAnswerJsonStatusText, &field_json) ||
+        !TextField::ParseTextField(field_json, image_line->status_text_.get()))
+      return false;
+  }
+
+  if (inner_json->HasKey(kAnswerJsonImage)) {
+    base::string16 url_string;
+    if (!inner_json->GetString(kAnswerJsonImageData, &url_string) ||
+        url_string.empty())
+      return false;
+    // If necessary, 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.  The GWS frontend commonly (always?)
+    // redirects to HTTPS so we just default to that here.
+    image_line->image_url_ = GURL(
+        StartsWith(url_string, base::ASCIIToUTF16("//"), false) ?
+            (base::ASCIIToUTF16(url::kHttpsScheme) + base::ASCIIToUTF16(":") +
+                url_string) :
+            url_string);
+
+    if (!image_line->image_url_.is_valid())
+      return false;
+  }
+
+  return true;
+}
+
+bool SuggestionAnswer::ImageLine::Equals(const ImageLine& line) const {
+  if (text_fields_.size() != line.text_fields_.size())

[groby-ooo-7-16, 2014/10/30 20:28:29]

I'd argue this is a valid reason why TextField needs operator== - all this just
becomes
if (text_fields_ != line.text_fields)...

IIRC, Peter argued against that, so don't change, but let's have that discussion
in a separate CL.

[Peter Kasting, 2014/10/30 20:32:56]

I'm very sympathetic to that, and in fact in general I personally believe
overloading operators is a sane thing to do that makes code much better.

Unfortunately, I think this is pretty clearly banned by the Google style guide,
which really only allows overloading operator==() if it's required for use in an
STL algorithm (e.g. find_if()).  I don't see a way around that.

This is an area where I disagree strongly with the style guide but feel bound to
enforce it as written.

[groby-ooo-7-16, 2014/10/30 20:46:12]

Grumble mumble. I'd say that the code base _happily_ ignores this - a quick
search in CS reveals 750 declarations of operator== - but yeah, this discussion
needs to be had around the style guide, not here. 

[Justin Donnelly, 2014/10/30 21:38:03]

Acknowledged.

+    return false;
+  for (size_t i = 0; i < text_fields_.size(); ++i) {
+    if (!text_fields_[i].Equals(line.text_fields_[i]))
+      return false;
+  }
+
+  if (additional_text_ || line.additional_text_) {
+    if (!additional_text_ || !line.additional_text_)
+      return false;
+    if (!additional_text_->Equals(*line.additional_text_))
+      return false;
+  }
+
+  if (status_text_ || line.status_text_) {
+    if (!status_text_ || !line.status_text_)
+      return false;
+    if (!status_text_->Equals(*line.status_text_))
+      return false;
+  }
+
+  return image_url_ == line.image_url_;
+}
+
+// SuggestionAnswer ------------------------------------------------------------
+
+SuggestionAnswer::SuggestionAnswer() : type_(-1) {}
+SuggestionAnswer::SuggestionAnswer(const SuggestionAnswer& answer)
+    : first_line_(answer.first_line_),
+      second_line_(answer.second_line_),
+      type_(answer.type_) {}
+
+SuggestionAnswer::~SuggestionAnswer() {}
+
+// static
+scoped_ptr<SuggestionAnswer> SuggestionAnswer::ParseAnswer(
+    const base::DictionaryValue* answer_json) {
+  scoped_ptr<SuggestionAnswer> result(new SuggestionAnswer());

[groby-ooo-7-16, 2014/10/30 20:28:29]

shorter:

auto result=make_scoped_ptr(new SuggestionAnswer);

[Justin Donnelly, 2014/10/30 21:38:03]

Done.

+
+  const base::ListValue* lines_json;
+  if (!answer_json->GetList(kAnswerJsonLines, &lines_json) ||
+      lines_json->GetSize() != 2)
+    return nullptr;
+
+  const base::DictionaryValue* first_line_json;
+  if (!lines_json->GetDictionary(0, &first_line_json) ||
+      !ImageLine::ParseImageLine(first_line_json, &result->first_line_))
+    return nullptr;
+
+  const base::DictionaryValue* second_line_json;
+  if (!lines_json->GetDictionary(1, &second_line_json) ||
+      !ImageLine::ParseImageLine(second_line_json, &result->second_line_))
+    return nullptr;
+
+  return result.Pass();
+}
+
+bool SuggestionAnswer::Equals(const SuggestionAnswer& answer) const {
+  return type_ == answer.type_ &&
+         first_line_.Equals(answer.first_line_) &&
+         second_line_.Equals(answer.second_line_);
+}
+
+void SuggestionAnswer::AddImageURLsTo(std::vector<GURL>* urls) const {
+  if (first_line_.image_url().is_valid())
+    urls->push_back(first_line_.image_url());
+  if (second_line_.image_url().is_valid())
+    urls->push_back(second_line_.image_url());
+}