Add a class to parse answer json.

components/omnibox/suggestion_answer.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_OMNIBOX_SUGGESTION_ANSWER_H_
+#define COMPONENTS_OMNIBOX_SUGGESTION_ANSWER_H_
+
+#include <string>
+#include <vector>
+
+#include "base/gtest_prod_util.h"
+#include "base/memory/scoped_ptr.h"
+#include "url/gurl.h"
+
+namespace base {
+class DictionaryValue;
+}
+
+// Structured representation of the JSON payload of a suggestion with an answer.
+// An answer has exactly two image lines, so called because they are a
+// combination of text and an optional image URL.  Each image line has 1 or more
+// text fields, each of which is required to contain a string and an integer
+// type.  The text fields are contained in a non-empty vector and two optional
+// named properties, referred to as "additional text" and "status text".
+//
+// When represented in the UI, these elements should be styled and laid out
+// according to the specification at https://goto.google.com/ais_api.
+//
+// Each of the three classes has either an explicit or implicity copy
+// constructor to support copying answer values (via SuggestionAnswer::copy) as
+// members of SuggestResult and AutocompleteMatch.
+class SuggestionAnswer {
+ public:
+  class TextField;
+  typedef std::vector<TextField> TextFields;
+  typedef std::vector<GURL> URLs;
+
+  class TextField {
+   public:
+    TextField();
+    ~TextField();
+
+    static bool ParseTextField(

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

should probably document that Parse can leave the out-param in an undefined
state when false is returned - here and elsewhere.

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

Done.

+        const base::DictionaryValue* field_json, TextField* text_field);

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

minor style nit:
static bool ParseTextField(const base::DictionaryValue* field_json,
                                        TextField* text_field);

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

Done.

+
+    const std::string& text() const { return text_; }
+    int type() const { return type_; }
+
+    bool Equals(const TextField& field) const;
+
+   private:
+    std::string text_;
+    int type_;
+
+    FRIEND_TEST_ALL_PREFIXES(SuggestionAnswerTest, DifferentValuesAreUnequal);

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

I commented in reverse - see below.

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

Acknowledged.

+  };
+
+  class ImageLine {
+   public:
+    ImageLine();
+    explicit ImageLine(const ImageLine& line);
+    ~ImageLine();
+
+    static bool ParseImageLine(
+        const base::DictionaryValue* line_json, ImageLine* image_line);

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

See above

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

Done.

+
+    const TextFields& text_fields() const { return text_fields_; }
+    const TextField* additional_text() const { return additional_text_.get(); }
+    const TextField* status_text() const { return status_text_.get(); }
+    const GURL& image_url() const { return image_url_; }
+
+    bool Equals(const ImageLine& line) const;
+
+   private:
+    TextFields text_fields_;
+    scoped_ptr<TextField> additional_text_;
+    scoped_ptr<TextField> status_text_;

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

Since this has copy ctor/assignment op allowed, you'll need to answer the
question of ownership for the assignment operator, or forbid it. I.e.

private ImageLine& operator=(const ImageLine&);  // to forbid

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

Done.

+    GURL image_url_;
+
+    FRIEND_TEST_ALL_PREFIXES(SuggestionAnswerTest, DifferentValuesAreUnequal);

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

Do you need this? All fields are accessible via getters.

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

Yes, the getters return const values.  If you read through the relevant test
hopefully you'll agree it's worth the friending.  The alternative would be to
have different fully-compliant json strings for each case which would make it
very difficult to see what was different between each one.

[groby-ooo-7-16, 2014/10/30 22:13:58]

Acknowledged.

+  };
+
+  SuggestionAnswer();
+  SuggestionAnswer(const SuggestionAnswer& answer);
+  ~SuggestionAnswer();
+
+  // Parses |answer_json| and populates |answer| with the contents if and only
+  // if the data is well formed and populated with all required elements.
+  static scoped_ptr<SuggestionAnswer> ParseAnswer(
+        const base::DictionaryValue* answer_json);
+
+  // TODO(jdonnelly): Once something like std::optional<T> is available in base/
+  // (see discussion at http://goo.gl/zN2GNy) remove this in favor of having
+  // SuggestResult and AutocompleteMatch use optional<SuggestionAnswer>.
+  static scoped_ptr<SuggestionAnswer> copy(const SuggestionAnswer* source) {
+    return make_scoped_ptr(source ? new SuggestionAnswer(*source) : nullptr);
+  }
+
+  const ImageLine& first_line() const { return first_line_; }
+  const ImageLine& second_line() const { return second_line_; }
+
+  // Answer type accessors.  Valid types are non-negative and defined at
+  // https://goto.google.com/visual_element_configuration.
+  int type() const { return type_; }
+  void set_type(int type) { type_ = type; }
+
+  bool Equals(const SuggestionAnswer& answer) const;
+
+  // Retrieves any image URLs appearing in this answer and adds them to |urls|.
+  void AddImageURLsTo(URLs* urls) const;
+
+ private:
+  ImageLine first_line_;
+  ImageLine second_line_;
+  int type_;
+
+  FRIEND_TEST_ALL_PREFIXES(SuggestionAnswerTest, DifferentValuesAreUnequal);

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

Do you need this? You can access all variables via accessors.

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

Acknowledged.

+};
+
+#endif  // COMPONENTS_OMNIBOX_SUGGESTION_ANSWER_H_