Make ValidateCddForPrintPreview take and return base::Value.

This eliminates some calls to |base::Value::Clone|.

Change-Id: If142585263b1d8080385a063f2dae68263f29087
Reviewed-on: https://chromium-review.googlesource.com/c/1383051
Commit-Queue: Vladislav Kuzkokov <vkuzkokov@chromium.org>
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#627986}

chrome/browser/ui/webui/print_preview/extension_printer_handler.cc

@@ -294,13 +294,11 @@ void ExtensionPrinterHandler::WrapGetCapabilityCallback(
     GetCapabilityCallback callback,
     const base::DictionaryValue& capability) {
   base::Value capabilities(base::Value::Type::DICTIONARY);
-  std::unique_ptr<base::DictionaryValue> cdd =
-      ValidateCddForPrintPreview(capability);
+  base::Value cdd = ValidateCddForPrintPreview(capability.Clone());
   // Leave |capabilities| empty if |cdd| is empty.
-  if (!cdd->empty()) {
-    capabilities.SetKey(kSettingCapabilities,
-                        base::Value::FromUniquePtrValue(std::move(cdd)));
-  }
+  if (!cdd.DictEmpty())
+    capabilities.SetKey(kSettingCapabilities, std::move(cdd));
+
   std::move(callback).Run(std::move(capabilities));
 }
 

chrome/browser/ui/webui/print_preview/print_preview_utils.cc

@@ -110,37 +110,37 @@ void SystemDialogDone(const base::Value& error) {
 
 }  // namespace
 
-std::unique_ptr<base::DictionaryValue> ValidateCddForPrintPreview(
-    const base::DictionaryValue& cdd) {
-  auto validated_cdd =
-      base::DictionaryValue::From(base::Value::ToUniquePtrValue(cdd.Clone()));
-  const base::Value* caps = cdd.FindKey(kPrinter);
-  if (!caps || !caps->is_dict())
-    return validated_cdd;
-  validated_cdd->RemoveKey(kPrinter);
-  auto out_caps = std::make_unique<base::DictionaryValue>();
-  for (const auto& capability : caps->DictItems()) {
+base::Value ValidateCddForPrintPreview(base::Value cdd) {
+  base::Value* caps =
+      cdd.FindKeyOfType(kPrinter, base::Value::Type::DICTIONARY);
+  if (!caps)
+    return cdd;
+
+  base::Value out_caps(base::Value::Type::DICTIONARY);
+  for (auto capability : caps->DictItems()) {
     const auto& key = capability.first;
-    const base::Value& value = capability.second;
+    base::Value* value = &capability.second;
+
+    base::Value* list = nullptr;
+    if (value->is_dict())
+      list = value->FindKeyOfType(kOptionKey, base::Value::Type::LIST);
+    else if (value->is_list())
+      list = value;
 
-    const base::Value* list = nullptr;
-    if (value.is_dict())
-      list = value.FindKeyOfType(kOptionKey, base::Value::Type::LIST);
-    else if (value.is_list())
-      list = &value;
     if (!list) {
-      out_caps->SetKey(key, value.Clone());
+      out_caps.SetKey(key, std::move(*value));
       continue;
     }
 
     bool is_vendor_capability = key == kVendorCapabilityKey;
-    base::Value out_list = GetFilteredList(
-        list, is_vendor_capability ? VendorCapabilityInvalid : ValueIsNull);
-    if (out_list.GetList().empty())  // leave out empty lists.
+    base::EraseIf(list->GetList(),
+                  is_vendor_capability ? VendorCapabilityInvalid : ValueIsNull);
+    if (list->GetList().empty())  // leave out empty lists.
       continue;
+
     if (is_vendor_capability) {
       // Need to also filter the individual capability lists.
-      for (auto& vendor_option : out_list.GetList()) {
+      for (auto& vendor_option : list->GetList()) {
         const base::Value* option_type =
             vendor_option.FindKeyOfType(kTypeKey, base::Value::Type::STRING);
         if (option_type->GetString() != kSelectString)
@@ -148,22 +148,21 @@ std::unique_ptr<base::DictionaryValue> ValidateCddForPrintPreview(
 
         base::Value* options_dict = vendor_option.FindKeyOfType(
             kSelectCapKey, base::Value::Type::DICTIONARY);
-        const base::Value* options_list =
+        base::Value* options_list =
             options_dict->FindKeyOfType(kOptionKey, base::Value::Type::LIST);
-        options_dict->SetKey(kOptionKey,
-                             GetFilteredList(options_list, ValueIsNull));
+        base::EraseIf(options_list->GetList(), ValueIsNull);
       }
     }
-    if (value.is_dict()) {
+    if (value->is_dict()) {
       base::Value option_dict(base::Value::Type::DICTIONARY);
-      option_dict.SetKey(kOptionKey, std::move(out_list));
-      out_caps->SetKey(key, std::move(option_dict));
+      option_dict.SetKey(kOptionKey, std::move(*list));
+      out_caps.SetKey(key, std::move(option_dict));
     } else {
-      out_caps->SetKey(key, std::move(out_list));
+      out_caps.SetKey(key, std::move(*list));
     }
   }
-  validated_cdd->SetDictionary(kPrinter, std::move(out_caps));
-  return validated_cdd;
+  cdd.SetKey(kPrinter, std::move(out_caps));
+  return cdd;
 }
 
 void ConvertPrinterListForCallback(

chrome/browser/ui/webui/print_preview/print_preview_utils.h

@@ -34,11 +34,10 @@ void ConvertPrinterListForCallback(
     PrinterHandler::GetPrintersDoneCallback done_callback,
     const PrinterList& printer_list);
 
-// Returns a unique_ptr to a sanitized version of |cdd| to prevent possible JS
+// Returns a sanitized version of |cdd| to prevent possible JS
 // errors in Print Preview. Will remove null items from lists or options lists
 // and remove any lists/options that are empty or only contain null values.
-std::unique_ptr<base::DictionaryValue> ValidateCddForPrintPreview(
-    const base::DictionaryValue& cdd);
+base::Value ValidateCddForPrintPreview(base::Value cdd);
 
 // Starts a local print of |print_data| with print settings dictionary
 // |job_settings|. Runs |callback| on failure or success.

chrome/browser/ui/webui/print_preview/print_preview_utils_unittest.cc

@@ -189,7 +189,7 @@ void ValidateVendorCaps(const base::Value* printer_out,
   }
 }
 
-void ValidatePrinter(const base::DictionaryValue* cdd_out,
+void ValidatePrinter(const base::Value* cdd_out,
                      const base::DictionaryValue& printer) {
   const base::Value* printer_out =
       cdd_out->FindKeyOfType(kPrinter, base::Value::Type::DICTIONARY);
@@ -217,8 +217,8 @@ TEST_F(PrintPreviewUtilsTest, FullCddPassthrough) {
   base::DictionaryValue printer = GetCapabilitiesFull();
   base::DictionaryValue cdd;
   cdd.SetKey(kPrinter, printer.Clone());
-  auto cdd_out = ValidateCddForPrintPreview(cdd);
-  ValidatePrinter(cdd_out.get(), printer);
+  auto cdd_out = ValidateCddForPrintPreview(std::move(cdd));
+  ValidatePrinter(&cdd_out, printer);
 }
 
 TEST_F(PrintPreviewUtilsTest, FilterBadList) {
@@ -230,8 +230,8 @@ TEST_F(PrintPreviewUtilsTest, FilterBadList) {
   printer.SetKey(kMediaSizes, base::Value(list_media));
   base::DictionaryValue cdd;
   cdd.SetKey(kPrinter, printer.Clone());
-  auto cdd_out = ValidateCddForPrintPreview(cdd);
-  ValidatePrinter(cdd_out.get(), printer);
+  auto cdd_out = ValidateCddForPrintPreview(std::move(cdd));
+  ValidatePrinter(&cdd_out, printer);
 }
 
 TEST_F(PrintPreviewUtilsTest, FilterBadOptionOneElement) {
@@ -245,8 +245,8 @@ TEST_F(PrintPreviewUtilsTest, FilterBadOptionOneElement) {
   printer.SetKey(kDpi, std::move(options));
   base::DictionaryValue cdd;
   cdd.SetKey(kPrinter, printer.Clone());
-  auto cdd_out = ValidateCddForPrintPreview(cdd);
-  ValidatePrinter(cdd_out.get(), printer);
+  auto cdd_out = ValidateCddForPrintPreview(std::move(cdd));
+  ValidatePrinter(&cdd_out, printer);
 }
 
 TEST_F(PrintPreviewUtilsTest, FilterBadOptionAllElement) {
@@ -260,8 +260,8 @@ TEST_F(PrintPreviewUtilsTest, FilterBadOptionAllElement) {
   printer.SetKey(kDpi, std::move(options));
   base::DictionaryValue cdd;
   cdd.SetKey(kPrinter, printer.Clone());
-  auto cdd_out = ValidateCddForPrintPreview(cdd);
-  ValidatePrinter(cdd_out.get(), printer);
+  auto cdd_out = ValidateCddForPrintPreview(std::move(cdd));
+  ValidatePrinter(&cdd_out, printer);
 }
 
 TEST_F(PrintPreviewUtilsTest, FilterBadVendorCapabilityAllElement) {
@@ -277,8 +277,8 @@ TEST_F(PrintPreviewUtilsTest, FilterBadVendorCapabilityAllElement) {
   select_cap_0->SetKey(kOptionKey, base::Value(option_list));
   base::DictionaryValue cdd;
   cdd.SetKey(kPrinter, printer.Clone());
-  auto cdd_out = ValidateCddForPrintPreview(cdd);
-  ValidatePrinter(cdd_out.get(), printer);
+  auto cdd_out = ValidateCddForPrintPreview(std::move(cdd));
+  ValidatePrinter(&cdd_out, printer);
 }
 
 TEST_F(PrintPreviewUtilsTest, FilterBadVendorCapabilityOneElement) {
@@ -305,8 +305,8 @@ TEST_F(PrintPreviewUtilsTest, FilterBadVendorCapabilityOneElement) {
 
   base::DictionaryValue cdd;
   cdd.SetKey(kPrinter, printer.Clone());
-  auto cdd_out = ValidateCddForPrintPreview(cdd);
-  ValidatePrinter(cdd_out.get(), printer);
+  auto cdd_out = ValidateCddForPrintPreview(std::move(cdd));
+  ValidatePrinter(&cdd_out, printer);
 }
 
 }  // namespace printing

chrome/browser/ui/webui/print_preview/privet_printer_handler.cc

@@ -191,7 +191,7 @@ void PrivetPrinterHandler::OnGotCapabilities(
   printer_info_and_caps.SetDictionary(kSettingCapabilities,
                                       std::move(capabilities_copy));
   std::move(capabilities_callback_)
-      .Run(std::move(*ValidateCddForPrintPreview(printer_info_and_caps)));
+      .Run(ValidateCddForPrintPreview(std::move(printer_info_and_caps)));
   privet_capabilities_operation_.reset();
 }