Improve NTPCatalog::create API and implementation

Change the API to receive a base::Value instead of the deprecated
class base::DictionaryValue (the implementation already check the
type of the passed value so does not assume base::Dictionary type).

Change the API to receive the base::Value by reference instead of
pointer as there is only one caller and it already check that the
pointer is non-null before calling NTPCatalog::create.

Use the base::Value::Find{String,List}Key methods instead of the
base::FindKeyOfType method as it reduce the complexity of the
implementation.

Bug: 646113
Change-Id: Icc3a484619786d5afcf0733ef01c07904635d481
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1609780
Auto-Submit: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659053}

chrome/browser/android/explore_sites/catalog.cc

@@ -7,20 +7,16 @@
 #include <sstream>
 
 #include "base/logging.h"
-#include "base/memory/ptr_util.h"
 #include "base/values.h"
 
 namespace explore_sites {
 
 // static
-std::unique_ptr<NTPCatalog> NTPCatalog::create(
-    const base::DictionaryValue* json) {
-  if (!json || !json->is_dict())
+std::unique_ptr<NTPCatalog> NTPCatalog::create(const base::Value& json) {
+  if (!json.is_dict())
     return nullptr;
 
-  const base::ListValue* categories = static_cast<const base::ListValue*>(
-      json->FindKeyOfType("categories", base::Value::Type::LIST));
-
+  const base::Value* categories = json.FindListKey("categories");
   if (!categories)
     return nullptr;
 
@@ -29,27 +25,21 @@ std::unique_ptr<NTPCatalog> NTPCatalog::create(
     if (!category.is_dict()) {
       return nullptr;
     }
-    const base::DictionaryValue* category_dict =
-        static_cast<const base::DictionaryValue*>(&category);
-    const base::Value* id =
-        category_dict->FindKeyOfType("id", base::Value::Type::STRING);
-    const base::Value* title =
-        category_dict->FindKeyOfType("title", base::Value::Type::STRING);
-    const base::Value* icon_url_str =
-        category_dict->FindKeyOfType("icon_url", base::Value::Type::STRING);
+    const std::string* id = category.FindStringKey("id");
+    const std::string* title = category.FindStringKey("title");
+    const std::string* icon_url_str = category.FindStringKey("icon_url");
 
     if (!id || !title || !icon_url_str)
       continue;
 
-    GURL icon_url(icon_url_str->GetString());
+    GURL icon_url(*icon_url_str);
     if (icon_url.is_empty())
       continue;
 
-    catalog_categories.push_back(
-        {id->GetString(), title->GetString(), icon_url});
+    catalog_categories.push_back({*id, *title, icon_url});
   }
 
-  auto catalog = base::WrapUnique(new NTPCatalog(catalog_categories));
+  auto catalog = std::make_unique<NTPCatalog>(catalog_categories);
   DVLOG(1) << "Catalog parsed: " << catalog->ToString();
 
   return catalog;

chrome/browser/android/explore_sites/catalog.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_ANDROID_EXPLORE_SITES_CATALOG_H_
 #define CHROME_BROWSER_ANDROID_EXPLORE_SITES_CATALOG_H_
 
+#include <memory>
 #include <string>
 #include <utility>
 #include <vector>
@@ -12,7 +13,7 @@
 #include "url/gurl.h"
 
 namespace base {
-class DictionaryValue;
+class Value;
 }
 
 namespace explore_sites {
@@ -34,7 +35,7 @@ class NTPCatalog {
   };
 
   // The NTPCatalog does not take ownership of |json|.
-  static std::unique_ptr<NTPCatalog> create(const base::DictionaryValue* json);
+  static std::unique_ptr<NTPCatalog> create(const base::Value& json);
 
   explicit NTPCatalog(const std::vector<Category>& category_list);
   ~NTPCatalog();

chrome/browser/android/explore_sites/ntp_json_fetcher.cc

@@ -113,8 +113,7 @@ void NTPJsonFetcher::OnJsonParseSuccess(
     return;
   }
 
-  auto catalog = NTPCatalog::create(
-      static_cast<base::DictionaryValue*>(parsed_json.get()));
+  auto catalog = NTPCatalog::create(*parsed_json);
   std::move(callback_).Run(std::move(catalog));
 }