Remove deprecated base::ListValue::Reserve().

Adjust callers to call base::Value::ListStorage::reserve(), or use an
alternative method to do what they want. Remove a few more deprecated
base::Value calls along the way when convenient.

Also update comments in base/values.h to remove references to methods
that no longer exist.

Bug: 646113
Change-Id: Id28a964676e9119687936050a28a19dfa60bc1c6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2437730Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813045}

base/values.cc

@@ -1442,10 +1442,6 @@ void ListValue::Clear() {
   list().clear();
 }
 
-void ListValue::Reserve(size_t n) {
-  list().reserve(n);
-}
-
 bool ListValue::Set(size_t index, std::unique_ptr<Value> in_value) {
   if (!in_value)
     return false;

base/values.h

@@ -621,10 +621,10 @@ class BASE_EXPORT DictionaryValue : public Value {
   Value* SetString(StringPiece path, StringPiece in_value);
   // DEPRECATED, use Value::SetStringPath().
   Value* SetString(StringPiece path, const string16& in_value);
-  // DEPRECATED, use Value::SetPath() or Value::SetDictPath()
+  // DEPRECATED, use Value::SetPath().
   DictionaryValue* SetDictionary(StringPiece path,
                                  std::unique_ptr<DictionaryValue> in_value);
-  // DEPRECATED, use Value::SetPath() or Value::SetListPath()
+  // DEPRECATED, use Value::SetPath().
   ListValue* SetList(StringPiece path, std::unique_ptr<ListValue> in_value);
 
   // Like Set(), but without special treatment of '.'.  This allows e.g. URLs to
@@ -801,11 +801,6 @@ class BASE_EXPORT ListValue : public Value {
   // DEPRECATED, use GetList()::empty() instead.
   bool empty() const { return list().empty(); }
 
-  // Reserves storage for at least |n| values.
-  // DEPRECATED, first construct a base::Value::ListStorage and use
-  // base::Value::ListStorage::reserve() instead.
-  void Reserve(size_t n);
-
   // Sets the list item at the given index to be the Value specified by
   // the value given.  If the index beyond the current end of the list, null
   // Values will be used to pad out the list.

chrome/browser/extensions/menu_manager.cc

@@ -669,14 +669,8 @@ void MenuManager::ExecuteCommand(content::BrowserContext* context,
                            webview_guest->view_instance_id());
   }
 
-  auto args = std::make_unique<base::ListValue>();
-  args->Reserve(2);
-  args->Append(std::move(properties));
-  // |properties| is invalidated at this time, which is why |args| needs to be
-  // queried for the pointer. The obtained pointer is guaranteed to stay valid
-  // even after further Appends, because enough storage was reserved above.
-  base::DictionaryValue* raw_properties = nullptr;
-  args->GetDictionary(0, &raw_properties);
+  base::Value::ListStorage args;
+  args.push_back(base::Value::FromUniquePtrValue(std::move(properties)));
 
   // Add the tab info to the argument list.
   // No tab info in a platform app.
@@ -685,7 +679,7 @@ void MenuManager::ExecuteCommand(content::BrowserContext* context,
     if (web_contents) {
       int frame_id = ExtensionApiFrameIdMap::GetFrameId(render_frame_host);
       if (frame_id != ExtensionApiFrameIdMap::kInvalidFrameId)
-        raw_properties->SetInteger("frameId", frame_id);
+        args[0].SetIntKey("frameId", frame_id);
 
       // We intentionally don't scrub the tab data here, since the user chose to
       // invoke the extension on the page.
@@ -693,26 +687,26 @@ void MenuManager::ExecuteCommand(content::BrowserContext* context,
       // on permissions.
       ExtensionTabUtil::ScrubTabBehavior scrub_tab_behavior = {
           ExtensionTabUtil::kDontScrubTab, ExtensionTabUtil::kDontScrubTab};
-      args->Append(ExtensionTabUtil::CreateTabObject(
-                       web_contents, scrub_tab_behavior, extension)
-                       ->ToValue());
+      args.push_back(base::Value::FromUniquePtrValue(
+          ExtensionTabUtil::CreateTabObject(web_contents, scrub_tab_behavior,
+                                            extension)
+              ->ToValue()));
     } else {
-      args->Append(std::make_unique<base::DictionaryValue>());
+      args.push_back(base::DictionaryValue());
     }
   }
 
   if (item->type() == MenuItem::CHECKBOX ||
       item->type() == MenuItem::RADIO) {
     bool was_checked = item->checked();
-    raw_properties->SetBoolean("wasChecked", was_checked);
+    args[0].SetBoolKey("wasChecked", was_checked);
 
     // RADIO items always get set to true when you click on them, but CHECKBOX
     // items get their state toggled.
-    bool checked =
-        (item->type() == MenuItem::RADIO) ? true : !was_checked;
+    bool checked = item->type() == MenuItem::RADIO || !was_checked;
 
     item->SetChecked(checked);
-    raw_properties->SetBoolean("checked", item->checked());
+    args[0].SetBoolKey("checked", item->checked());
 
     if (extension)
       WriteToStorage(extension, item->id().extension_key);
@@ -732,7 +726,7 @@ void MenuManager::ExecuteCommand(content::BrowserContext* context,
         webview_guest ? events::WEB_VIEW_INTERNAL_CONTEXT_MENUS
                       : events::CONTEXT_MENUS,
         webview_guest ? kOnWebviewContextMenus : kOnContextMenus,
-        std::unique_ptr<base::ListValue>(args->DeepCopy()), context);
+        std::make_unique<base::ListValue>(args), context);
     event->user_gesture = EventRouter::USER_GESTURE_ENABLED;
     event_router->DispatchEventToExtension(item->extension_id(),
                                            std::move(event));
@@ -744,7 +738,7 @@ void MenuManager::ExecuteCommand(content::BrowserContext* context,
                       : events::CONTEXT_MENUS_ON_CLICKED,
         webview_guest ? api::chrome_web_view_internal::OnClicked::kEventName
                       : api::context_menus::OnClicked::kEventName,
-        std::move(args), context);
+        std::make_unique<base::ListValue>(std::move(args)), context);
     event->user_gesture = EventRouter::USER_GESTURE_ENABLED;
     if (webview_guest)
       event->filter_info.instance_id = webview_guest->view_instance_id();

components/client_hints/browser/client_hints.cc

@@ -110,9 +110,8 @@ void ClientHints::PersistClientHints(
   if (expiration_duration <= base::TimeDelta::FromSeconds(0))
     return;
 
-  std::unique_ptr<base::ListValue> expiration_times_list =
-      std::make_unique<base::ListValue>();
-  expiration_times_list->Reserve(client_hints.size());
+  base::Value::ListStorage expiration_times_list;
+  expiration_times_list.reserve(client_hints.size());
 
   // Use wall clock since the expiration time would be persisted across embedder
   // restarts.
@@ -120,12 +119,12 @@ void ClientHints::PersistClientHints(
       (base::Time::Now() + expiration_duration).ToDoubleT();
 
   for (const auto& entry : client_hints)
-    expiration_times_list->AppendInteger(static_cast<int>(entry));
+    expiration_times_list.push_back(base::Value(static_cast<int>(entry)));
 
   auto expiration_times_dictionary = std::make_unique<base::DictionaryValue>();
-  expiration_times_dictionary->SetList("client_hints",
-                                       std::move(expiration_times_list));
-  expiration_times_dictionary->SetDouble("expiration_time", expiration_time);
+  expiration_times_dictionary->SetKey(
+      "client_hints", base::Value(std::move(expiration_times_list)));
+  expiration_times_dictionary->SetDoubleKey("expiration_time", expiration_time);
 
   // TODO(tbansal): crbug.com/735518. Disable updates to client hints settings
   // when cookies are disabled for |primary_origin|.

extensions/renderer/api_activity_logger.cc

@@ -56,24 +56,28 @@ void APIActivityLogger::LogAPICall(
 
   ScriptContext* script_context =
       ScriptContextSet::GetContextByV8Context(context);
-  auto value_args = std::make_unique<base::ListValue>();
   std::unique_ptr<content::V8ValueConverter> converter =
       content::V8ValueConverter::Create();
   ActivityLogConverterStrategy strategy;
   converter->SetFunctionAllowed(true);
   converter->SetStrategy(&strategy);
-  value_args->Reserve(arguments.size());
+
+  base::Value::ListStorage value_args;
+  value_args.reserve(arguments.size());
   // TODO(devlin): This doesn't protect against custom properties, so it might
   // not perfectly reflect the passed arguments.
   for (const auto& arg : arguments) {
     std::unique_ptr<base::Value> converted_arg =
         converter->FromV8Value(arg, context);
-    value_args->Append(converted_arg ? std::move(converted_arg)
-                                     : std::make_unique<base::Value>());
+    if (!converted_arg)
+      converted_arg = std::make_unique<base::Value>();
+    value_args.push_back(
+        base::Value::FromUniquePtrValue(std::move(converted_arg)));
   }
 
   LogInternal(APICALL, script_context->GetExtensionID(), call_name,
-              std::move(value_args), std::string());
+              std::make_unique<base::ListValue>(std::move(value_args)),
+              std::string());
 }
 
 void APIActivityLogger::LogEvent(ScriptContext* script_context,
@@ -114,10 +118,10 @@ void APIActivityLogger::LogForJS(
   }
 
   // Get the array of call arguments.
-  auto arguments = std::make_unique<base::ListValue>();
+  base::Value::ListStorage arguments;
   v8::Local<v8::Array> arg_array = v8::Local<v8::Array>::Cast(args[2]);
   if (arg_array->Length() > 0) {
-    arguments->Reserve(arg_array->Length());
+    arguments.reserve(arg_array->Length());
     std::unique_ptr<content::V8ValueConverter> converter =
         content::V8ValueConverter::Create();
     ActivityLogConverterStrategy strategy;
@@ -128,12 +132,15 @@ void APIActivityLogger::LogForJS(
       // actual error handling.
       std::unique_ptr<base::Value> converted_arg = converter->FromV8Value(
           arg_array->Get(context, i).ToLocalChecked(), context);
-      arguments->Append(converted_arg ? std::move(converted_arg)
-                                      : std::make_unique<base::Value>());
+      if (!converted_arg)
+        converted_arg = std::make_unique<base::Value>();
+      arguments.push_back(
+          base::Value::FromUniquePtrValue(std::move(converted_arg)));
     }
   }
 
-  LogInternal(call_type, extension_id, call_name, std::move(arguments), extra);
+  LogInternal(call_type, extension_id, call_name,
+              std::make_unique<base::ListValue>(std::move(arguments)), extra);
 }
 
 // static

extensions/renderer/bindings/api_signature.cc

@@ -500,8 +500,8 @@ APISignature::JSONParseResult APISignature::ConvertArgumentsIgnoringSchema(
       callback = value.As<v8::Function>();
   }
 
-  auto json = std::make_unique<base::ListValue>();
-  json->Reserve(size);
+  base::Value::ListStorage json;
+  json.reserve(size);
 
   std::unique_ptr<content::V8ValueConverter> converter =
       content::V8ValueConverter::Create();
@@ -520,11 +520,11 @@ APISignature::JSONParseResult APISignature::ConvertArgumentsIgnoringSchema(
       // null). Duplicate that behavior here.
       converted = std::make_unique<base::Value>();
     }
-    json->Append(std::move(converted));
+    json.push_back(base::Value::FromUniquePtrValue(std::move(converted)));
   }
 
   JSONParseResult result;
-  result.arguments = std::move(json);
+  result.arguments = std::make_unique<base::ListValue>(std::move(json));
   result.callback = callback;
   result.async_type = callback.IsEmpty()
                           ? binding::AsyncResponseType::kNone