[AiS] Fix parsing for Answers Images.

components/omnibox/search_suggestion_parser_unittest.cc

Index: components/omnibox/search_suggestion_parser_unittest.cc
diff --git a/components/omnibox/search_suggestion_parser_unittest.cc b/components/omnibox/search_suggestion_parser_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..0e193f4f27bd0e6f8ba33cb542a4fe75cc7b7092
--- /dev/null
+++ b/components/omnibox/search_suggestion_parser_unittest.cc
@@ -0,0 +1,50 @@
+// 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/json/json_reader.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/values.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+namespace {
+
+scoped_ptr<base::DictionaryValue> AsDictionary(const std::string& json) {
+  base::Value* value = base::JSONReader::Read(json);
+  base::DictionaryValue* dict;
+  if (value && value->GetAsDictionary(&dict))
+    return scoped_ptr<base::DictionaryValue>(dict);
+
+  delete value;

[Mark P, 2014/08/15 22:49:16]

Am I misunderstanding the flow here?  Isn't |value| normally not going to get
deleted because you return early?

[groby-ooo-7-16, 2014/08/22 00:43:58]

It's a bit tricky due to base::Value's behavior. What happens is that the lines
above are essentially a typesafe cast. So dict equals value, and thus gets
returned via the scoped_ptr (and the scoped_ptr assumes ownership)

The following would be clearer, but carries duplicate allocation cost, and it
relies on manual typecasting.

scoped_ptr<base::Value> value(base::JSONReder::Read(json));
if (value->Type() == TYPE_DICTIONARY)
  return scoped_ptr<base::DictionaryValue>(value->DeepCopy());
return scoped_ptr<base::DictionaryValue>(new base::DictionaryValue)

PassAs doesn't work because it's a Downcast

I _suppose_ I could construct the dict like this:
if (value->Type() == TYPE_DICTIONARY)
  return
scoped_ptr<base::DictionaryValue>(static_cast<DictionaryValue>(value.release()));

For my taste, that relies on too many internals, but lmk if you'd prefer this.
.

[Mark P, 2014/08/22 16:21:08]

Thanks for the explanation.
The current code looks good as is.

+  return scoped_ptr<base::DictionaryValue>(new base::DictionaryValue);
+}
+
+}  // namespace
+
+TEST(SearchSuggestionParser, GetAnswersImageURLsWithoutImagelines) {
+  std::vector<GURL> urls;
+
+  // No "l" entry in the dictionary.
+  SearchSuggestionParser::GetAnswersImageURLs(AsDictionary("").get(), &urls);
+  EXPECT_TRUE(urls.empty());
+
+  // Empty "l" entry in the dictionary.
+  SearchSuggestionParser::GetAnswersImageURLs(
+      AsDictionary("{ \"l\" : {} } ").get(), &urls);
+  EXPECT_TRUE(urls.empty());
+}
+
+TEST(SearchSuggestionParser, GetAnswersImageURLsWithValidImage) {
+  std::vector<GURL> urls;
+
+  const char answer_json[] =
+      "{ \"l\" : [{\"il\": { \"i\": {\"d\": "
+      "\"//ssl.gstatic.com/foo.png\",\"t\": 3}}}]}";
+  SearchSuggestionParser::GetAnswersImageURLs(AsDictionary(answer_json).get(),
+                                              &urls);
+  ASSERT_EQ(1U, urls.size());
+  EXPECT_EQ("https://ssl.gstatic.com/foo.png", urls[0].spec());
+}