Remove ListValue::Append(raw ptr) on Mac and iOS

Remove ListValue::Append(raw ptr) on Mac and iOS

ListValue::Append(raw ptr) is deprecated and the unique_ptr version should be
used instead. This CL achieves that on Mac and iOS specifically.

The CL also changes some related calls to DictionaryValue::Set to take
a unique_ptr instead of a raw one.

BUG=581865
Review-Url: https://codereview.chromium.org/2817603003
Cr-Commit-Position: refs/heads/master@{#464862}
Committed: https://chromium.googlesource.com/chromium/src/+/aec0f17718b8e13656d7c9f9fe795a0b8066ffcc

[vabr (Chromium), 2017-04-12 08:29:39]

The CQ bit was checked by vabr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-12 08:29:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-12 08:44:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-12 08:44:03]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[vabr (Chromium), 2017-04-12 09:08:31]

The CQ bit was checked by vabr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-12 09:08:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[vabr (Chromium), 2017-04-12 09:11:26]

Description was changed from

==========
Remove ListValue::Append(raw ptr) on Mac

ListValue::Append(raw ptr) is deprecated and the unique_ptr version should be
used instead. This CL achieves that on Mac specifically.

BUG=581865
==========

to

==========
Remove ListValue::Append(raw ptr) on Mac and iOS

ListValue::Append(raw ptr) is deprecated and the unique_ptr version should be
used instead. This CL achieves that on Mac and iOS specifically.

BUG=581865
==========

[commit-bot: I haz the power, 2017-04-12 09:37:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-12 09:37:52]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[vabr (Chromium), 2017-04-12 09:45:05]

The CQ bit was checked by vabr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-12 09:45:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-12 10:05:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-12 10:05:52]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[vabr (Chromium), 2017-04-12 10:28:06]

The CQ bit was checked by vabr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-12 10:28:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[vabr (Chromium), 2017-04-12 11:33:15]

Description was changed from

==========
Remove ListValue::Append(raw ptr) on Mac and iOS

ListValue::Append(raw ptr) is deprecated and the unique_ptr version should be
used instead. This CL achieves that on Mac and iOS specifically.

BUG=581865
==========

to

==========
Remove ListValue::Append(raw ptr) on Mac and iOS

ListValue::Append(raw ptr) is deprecated and the unique_ptr version should be
used instead. This CL achieves that on Mac and iOS specifically.

The CL also changes some related calls to DictionaryValue::Set to take
a unique_ptr instead of a raw one.

BUG=581865
==========

[vabr (Chromium), 2017-04-12 11:34:48]

vabr@chromium.org changed reviewers:
+ brettw@chromium.org, jdoerrie@chromium.org, jochen@chromium.org,
sdefresne@chromium.org

[vabr (Chromium), 2017-04-12 11:34:49]

Hi all,

Please review:

jdoerie@: the whole patch
brettw@: //base

Please rubberstamp:
jochen@: //components and //content
sdefresne@: //ios

Thanks!
Vaclav

[jochen (gone - plz use gerrit), 2017-04-12 11:35:54]

lgtm

[commit-bot: I haz the power, 2017-04-12 11:47:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-12 11:47:54]

Dry run: This issue passed the CQ dry run.

[jdoerrie, 2017-04-12 12:37:47]

LGTM, just a few nits :)

https://codereview.chromium.org/2817603003/diff/60001/components/policy/core/...
File components/policy/core/common/mac_util_unittest.cc (right):

https://codereview.chromium.org/2817603003/diff/60001/components/policy/core/...
components/policy/core/common/mac_util_unittest.cc:43: root.Set("emptyl",
base::MakeUnique<base::Value>(list));
Given that list is empty, you can simply use

root.Set("emptyl", base::MakeUnique<base::ListValue>());

or the more verbose

root.Set("emptyl", base::MakeUnique<base::Value>(base::Value::Type::LIST));

instead and declare list below after.

https://codereview.chromium.org/2817603003/diff/60001/components/policy/core/...
components/policy/core/common/mac_util_unittest.cc:52: root.Set("emptyd",
base::MakeUnique<base::Value>(dict));
root.Set("emptyd", base::MakeUnique<base::DictionaryValue>());

or

root.Set("dict", base::MakeUnique<base::Value>(base::Value::Type::DICTIONARY));

https://codereview.chromium.org/2817603003/diff/60001/components/wifi/fake_wi...
File components/wifi/fake_wifi_service.cc (right):

https://codereview.chromium.org/2817603003/diff/60001/components/wifi/fake_wi...
components/wifi/fake_wifi_service.cc:11: #include
"base/message_loop/message_loop.h"
#include "base/values.h"

https://codereview.chromium.org/2817603003/diff/60001/components/wifi/wifi_se...
File components/wifi/wifi_service_mac.mm (right):

https://codereview.chromium.org/2817603003/diff/60001/components/wifi/wifi_se...
components/wifi/wifi_service_mac.mm:19: #include
"base/message_loop/message_loop.h"
#include "base/values.h"

https://codereview.chromium.org/2817603003/diff/60001/content/browser/accessi...
File content/browser/accessibility/accessibility_tree_formatter_mac.mm (right):

https://codereview.chromium.org/2817603003/diff/60001/content/browser/accessi...
content/browser/accessibility/accessibility_tree_formatter_mac.mm:14: #include
"base/strings/utf_string_conversions.h"
#include "base/values.h"

https://codereview.chromium.org/2817603003/diff/60001/content/common/font_lis...
File content/common/font_list_mac.mm (right):

https://codereview.chromium.org/2817603003/diff/60001/content/common/font_lis...
content/common/font_list_mac.mm:41:
font_item->Append(base::MakeUnique<base::Value>(loc_family));
Consider the more compact AppendString, ie:

font_item->AppendString(family);
font_item->AppendString(loc_family);

https://codereview.chromium.org/2817603003/diff/60001/ios/chrome/browser/pass...
File ios/chrome/browser/passwords/password_controller.mm (right):

https://codereview.chromium.org/2817603003/diff/60001/ios/chrome/browser/pass...
ios/chrome/browser/passwords/password_controller.mm:23: #include
"base/strings/utf_string_conversions.h"
#include "base/values.h"

https://codereview.chromium.org/2817603003/diff/60001/ios/chrome/browser/ui/s...
File ios/chrome/browser/ui/settings/block_popups_collection_view_controller.mm
(right):

https://codereview.chromium.org/2817603003/diff/60001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/settings/block_popups_collection_view_controller.mm:286:
base::MakeUnique<base::Value>(entries[i].primary_pattern.ToString()));
Consider using AppendString and removing the added #include.

https://codereview.chromium.org/2817603003/diff/60001/ios/chrome/browser/ui/w...
File
ios/chrome/browser/ui/webui/sync_internals/sync_internals_message_handler.cc
(right):

https://codereview.chromium.org/2817603003/diff/60001/ios/chrome/browser/ui/w...
ios/chrome/browser/ui/webui/sync_internals/sync_internals_message_handler.cc:130:
base::MakeUnique<base::Value>(ModelTypeToString(it.Get())));
Consider using AppendString and removing the added #include.

[vabr (Chromium), 2017-04-12 12:54:09]

The CQ bit was checked by vabr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-12 12:54:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[vabr (Chromium), 2017-04-12 12:55:04]

Thanks, Jan! Comments addressed.

Once Brett is OK with the base change I will attempt landing, TBR-ing //ios if
needed.

Cheers,
Vaclav

https://codereview.chromium.org/2817603003/diff/60001/components/policy/core/...
File components/policy/core/common/mac_util_unittest.cc (right):

https://codereview.chromium.org/2817603003/diff/60001/components/policy/core/...
components/policy/core/common/mac_util_unittest.cc:43: root.Set("emptyl",
base::MakeUnique<base::Value>(list));
On 2017/04/12 12:37:46, jdoerrie wrote:
> Given that list is empty, you can simply use
> 
> root.Set("emptyl", base::MakeUnique<base::ListValue>());
> 
> or the more verbose
> 
> root.Set("emptyl", base::MakeUnique<base::Value>(base::Value::Type::LIST));
> 
> instead and declare list below after.

Done.

https://codereview.chromium.org/2817603003/diff/60001/components/policy/core/...
components/policy/core/common/mac_util_unittest.cc:52: root.Set("emptyd",
base::MakeUnique<base::Value>(dict));
On 2017/04/12 12:37:46, jdoerrie wrote:
> root.Set("emptyd", base::MakeUnique<base::DictionaryValue>());
> 
> or
> 
> root.Set("dict",
base::MakeUnique<base::Value>(base::Value::Type::DICTIONARY));

Done.

https://codereview.chromium.org/2817603003/diff/60001/components/wifi/fake_wi...
File components/wifi/fake_wifi_service.cc (right):

https://codereview.chromium.org/2817603003/diff/60001/components/wifi/fake_wi...
components/wifi/fake_wifi_service.cc:11: #include
"base/message_loop/message_loop.h"
On 2017/04/12 12:37:46, jdoerrie wrote:
> #include "base/values.h"

Done.

https://codereview.chromium.org/2817603003/diff/60001/components/wifi/wifi_se...
File components/wifi/wifi_service_mac.mm (right):

https://codereview.chromium.org/2817603003/diff/60001/components/wifi/wifi_se...
components/wifi/wifi_service_mac.mm:19: #include
"base/message_loop/message_loop.h"
On 2017/04/12 12:37:46, jdoerrie wrote:
> #include "base/values.h"

Done.

https://codereview.chromium.org/2817603003/diff/60001/content/browser/accessi...
File content/browser/accessibility/accessibility_tree_formatter_mac.mm (right):

https://codereview.chromium.org/2817603003/diff/60001/content/browser/accessi...
content/browser/accessibility/accessibility_tree_formatter_mac.mm:14: #include
"base/strings/utf_string_conversions.h"
On 2017/04/12 12:37:46, jdoerrie wrote:
> #include "base/values.h"

Done.

https://codereview.chromium.org/2817603003/diff/60001/content/common/font_lis...
File content/common/font_list_mac.mm (right):

https://codereview.chromium.org/2817603003/diff/60001/content/common/font_lis...
content/common/font_list_mac.mm:41:
font_item->Append(base::MakeUnique<base::Value>(loc_family));
On 2017/04/12 12:37:46, jdoerrie wrote:
> Consider the more compact AppendString, ie:
> 
> font_item->AppendString(family);
> font_item->AppendString(loc_family);

Good point, I completely missed the existence of those convenience methods.
Thanks!

https://codereview.chromium.org/2817603003/diff/60001/ios/chrome/browser/pass...
File ios/chrome/browser/passwords/password_controller.mm (right):

https://codereview.chromium.org/2817603003/diff/60001/ios/chrome/browser/pass...
ios/chrome/browser/passwords/password_controller.mm:23: #include
"base/strings/utf_string_conversions.h"
On 2017/04/12 12:37:46, jdoerrie wrote:
> #include "base/values.h"

Done.

https://codereview.chromium.org/2817603003/diff/60001/ios/chrome/browser/ui/s...
File ios/chrome/browser/ui/settings/block_popups_collection_view_controller.mm
(right):

https://codereview.chromium.org/2817603003/diff/60001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/settings/block_popups_collection_view_controller.mm:286:
base::MakeUnique<base::Value>(entries[i].primary_pattern.ToString()));
On 2017/04/12 12:37:46, jdoerrie wrote:
> Consider using AppendString and removing the added #include.

Done.

https://codereview.chromium.org/2817603003/diff/60001/ios/chrome/browser/ui/w...
File
ios/chrome/browser/ui/webui/sync_internals/sync_internals_message_handler.cc
(right):

https://codereview.chromium.org/2817603003/diff/60001/ios/chrome/browser/ui/w...
ios/chrome/browser/ui/webui/sync_internals/sync_internals_message_handler.cc:130:
base::MakeUnique<base::Value>(ModelTypeToString(it.Get())));
On 2017/04/12 12:37:46, jdoerrie wrote:
> Consider using AppendString and removing the added #include.

Done.

[commit-bot: I haz the power, 2017-04-12 14:12:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-12 14:12:09]

Dry run: This issue passed the CQ dry run.

[sdefresne, 2017-04-12 14:19:48]

ios lgtm

[brettw, 2017-04-14 17:28:25]

lgtm

[vabr (Chromium), 2017-04-14 21:18:17]

The CQ bit was checked by vabr@chromium.org

[vabr (Chromium), 2017-04-14 21:18:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
jdoerrie@chromium.org, jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/2817603003/#ps80001
(title: "Comments")

[commit-bot: I haz the power, 2017-04-14 21:18:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-14 23:46:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-14 23:46:47]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[vabr (Chromium), 2017-04-15 07:28:30]

The CQ bit was checked by vabr@chromium.org

[commit-bot: I haz the power, 2017-04-15 07:28:46]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-15 08:59:23]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1492241310080740,
"parent_rev": "8f55866589a385c959b0e9f0a3bd1f70e4139a66", "commit_rev":
"aec0f17718b8e13656d7c9f9fe795a0b8066ffcc"}

[commit-bot: I haz the power, 2017-04-15 09:00:32]

Description was changed from

==========
Remove ListValue::Append(raw ptr) on Mac and iOS

ListValue::Append(raw ptr) is deprecated and the unique_ptr version should be
used instead. This CL achieves that on Mac and iOS specifically.

The CL also changes some related calls to DictionaryValue::Set to take
a unique_ptr instead of a raw one.

BUG=581865
==========

to

==========
Remove ListValue::Append(raw ptr) on Mac and iOS

ListValue::Append(raw ptr) is deprecated and the unique_ptr version should be
used instead. This CL achieves that on Mac and iOS specifically.

The CL also changes some related calls to DictionaryValue::Set to take
a unique_ptr instead of a raw one.

BUG=581865

Review-Url: https://codereview.chromium.org/2817603003
Cr-Commit-Position: refs/heads/master@{#464862}
Committed:
https://chromium.googlesource.com/chromium/src/+/aec0f17718b8e13656d7c9f9fe79...
==========

[commit-bot: I haz the power, 2017-04-15 09:00:34]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/aec0f17718b8e13656d7c9f9fe79...