Update usages of std::map to use ScopedPtrMap.

Update usages of std::map to use ScopedPtrMap.

Almost all code that uses a std::map + STLValueDeleter has been modified
to use a ScopedPtrMap. This makes pointer ownership much clearer, and
removes a lot of manual pointer deletion.

BUG=478594
Committed: https://crrev.com/f59c7a8db11a9c5f5abbba3b0fb6f42059e4ce1d
Cr-Commit-Position: refs/heads/master@{#336096}

[Matt Giuca, 2015-06-11 09:00:45]

Patchset #10 (id:170001) has been deleted

[Matt Giuca, 2015-06-12 04:56:39]

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

[commit-bot: I haz the power, 2015-06-12 04:56:47]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1096983002/210001

[commit-bot: I haz the power, 2015-06-12 05:00:51]

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

[commit-bot: I haz the power, 2015-06-12 05:00:52]

Dry run: Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[Matt Giuca, 2015-06-15 05:10:01]

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

[commit-bot: I haz the power, 2015-06-15 05:10:20]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1096983002/230001

[commit-bot: I haz the power, 2015-06-15 05:19:02]

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

[commit-bot: I haz the power, 2015-06-15 05:19:03]

Dry run: Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[Matt Giuca, 2015-06-16 05:01:32]

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

[commit-bot: I haz the power, 2015-06-16 05:01:47]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1096983002/230001

[commit-bot: I haz the power, 2015-06-16 05:12:01]

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

[commit-bot: I haz the power, 2015-06-16 05:12:02]

Dry run: Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[Matt Giuca, 2015-06-16 07:38:28]

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

[commit-bot: I haz the power, 2015-06-16 07:38:55]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1096983002/250001

[commit-bot: I haz the power, 2015-06-16 07:49:39]

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

[commit-bot: I haz the power, 2015-06-16 07:49:40]

Dry run: Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  linux_chromium_gn_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Matt Giuca, 2015-06-17 03:01:13]

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

[commit-bot: I haz the power, 2015-06-17 03:02:05]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1096983002/270001

[commit-bot: I haz the power, 2015-06-17 03:08:17]

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

[commit-bot: I haz the power, 2015-06-17 03:08:18]

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

[Matt Giuca, 2015-06-17 04:00:01]

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

[commit-bot: I haz the power, 2015-06-17 04:00:59]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1096983002/290001

[commit-bot: I haz the power, 2015-06-17 04:11:41]

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

[commit-bot: I haz the power, 2015-06-17 04:11:42]

Dry run: Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Matt Giuca, 2015-06-17 08:02:57]

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

[commit-bot: I haz the power, 2015-06-17 08:04:29]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1096983002/330001

[commit-bot: I haz the power, 2015-06-17 08:20:28]

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

[commit-bot: I haz the power, 2015-06-17 08:20:29]

Dry run: Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[Matt Giuca, 2015-06-18 03:48:37]

Patchset #19 (id:370001) has been deleted

[Matt Giuca, 2015-06-18 03:48:55]

Patchset #18 (id:350001) has been deleted

[Matt Giuca, 2015-06-18 03:49:29]

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

[commit-bot: I haz the power, 2015-06-18 03:50:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1096983002/390001

[commit-bot: I haz the power, 2015-06-18 04:03:16]

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

[commit-bot: I haz the power, 2015-06-18 04:03:17]

Dry run: Try jobs failed on following builders:
  linux_chromium_gn_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Matt Giuca, 2015-06-18 04:26:43]

Patchset #18 (id:390001) has been deleted

[Matt Giuca, 2015-06-18 04:27:19]

Patchset #18 (id:390001) has been deleted

[Matt Giuca, 2015-06-18 04:27:48]

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

[commit-bot: I haz the power, 2015-06-18 04:32:11]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1096983002/410001

[commit-bot: I haz the power, 2015-06-18 05:16:13]

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

[commit-bot: I haz the power, 2015-06-18 05:16:15]

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

[Matt Giuca, 2015-06-18 06:03:35]

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

[commit-bot: I haz the power, 2015-06-18 06:04:35]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1096983002/430001

[commit-bot: I haz the power, 2015-06-18 06:51:43]

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

[commit-bot: I haz the power, 2015-06-18 06:51:45]

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

[Matt Giuca, 2015-06-18 09:03:19]

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

[commit-bot: I haz the power, 2015-06-18 09:03:39]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1096983002/450001

[Matt Giuca, 2015-06-18 09:04:33]

mgiuca@chromium.org changed reviewers:
+ jochen@chromium.org, rsesek@chromium.org, stevenjb@chromium.org,
tapted@chromium.org, tim@chromium.org, toyoshim@chromium.org, vabr@chromium.org

[Matt Giuca, 2015-06-18 09:04:34]

Hi all,

This CL is a follow-up to r334083 where I added a ScopedPtrMap class. Here I am
replacing as many uses of STLValueDeleter as possible with ScopedPtrMap. It's
easier to do this all in one CL because the changes are fairly mechanical and if
there are disagreements about style, it's best to make consistent changes. In
most cases, the ownership is simplified and I was often able to remove ugly
manual deletion logic.

(There are still users of STLDeleteValues and
STLDeleteContainerPairSecondPointers, as well as std::maps with single-reference
linked_ptrs, but those number in the hundreds so I'll tackle those later.)

rsesek@chromium.org: Please review changes in ui/gfx/image/*

vabr@chromium.org: Please review changes in chrome/browser/ui/passwords/*,
components/password_manager/*

toyoshim@chromium.org: Please review changes in media/midi/*

tapted@chromium.org: Please review changes in apps/*

stevenjb@chromium.org: Please review changes in chromeos/dbus/*,
ui/message_center/*

tim@chromium.org: Please review changes in sync/*

jochen@chromium.org: Please review changes in chrome/browser/predictors/*,
components/*

[commit-bot: I haz the power, 2015-06-18 10:09:51]

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

[commit-bot: I haz the power, 2015-06-18 10:09:53]

Dry run: This issue passed the CQ dry run.

[Robert Sesek, 2015-06-18 12:50:21]

image/ LGTM

https://codereview.chromium.org/1096983002/diff/450001/ui/gfx/image/image.h
File ui/gfx/image/image.h (right):

https://codereview.chromium.org/1096983002/diff/450001/ui/gfx/image/image.h#n...
ui/gfx/image/image.h:62: typedef ScopedPtrMap<RepresentationType,
scoped_ptr<internal::ImageRep>>
Optional: You could update this to a using RepresentationMap = …; declaration
(preference for new code).

[jochen (gone - plz use gerrit), 2015-06-18 12:56:46]

lgtm

[vabr (Chromium), 2015-06-18 13:14:12]

chrome/browser/ui/passwords/*,
components/password_manager/* LGTM, I love this change!

Thanks,
Vaclav

[stevenjb, 2015-06-18 15:32:02]

https://codereview.chromium.org/1096983002/diff/450001/chromeos/dbus/gsm_sms_...
File chromeos/dbus/gsm_sms_client.cc (right):

https://codereview.chromium.org/1096983002/diff/450001/chromeos/dbus/gsm_sms_...
chromeos/dbus/gsm_sms_client.cc:219: return it->second;
Does this compile? It seems like this would now return a scoped_ptr?

[tapted, 2015-06-19 00:12:01]

On 2015/06/18 09:04:34, Matt Giuca wrote:
> mailto:tapted@chromium.org: Please review changes in apps/*

apps/saved_files_service lgtm with some nits

https://codereview.chromium.org/1096983002/diff/450001/apps/saved_files_servi...
File apps/saved_files_service.cc (right):

https://codereview.chromium.org/1096983002/diff/450001/apps/saved_files_servi...
apps/saved_files_service.cc:323: auto it = registered_file_entries_.find(id);
`it` becomes a shadowed variable just below, which is confusing. What about

SavedFileEntry* file_entry = registered_file_entries_.get(id);
DCHECK(file_entry);

(or even skip the DCHECK since it's immediately dereferenced and would crash
regardless)

https://codereview.chromium.org/1096983002/diff/450001/apps/saved_files_servi...
apps/saved_files_service.cc:364: auto it = registered_file_entries_.find(id);
nit: just `return registered_file_entries_.get(id);` (this method looks
identical)

https://codereview.chromium.org/1096983002/diff/450001/apps/saved_files_servi...
File apps/saved_files_service.h (right):

https://codereview.chromium.org/1096983002/diff/450001/apps/saved_files_servi...
apps/saved_files_service.h:130: ScopedPtrMap<std::string,
scoped_ptr<SavedFiles>>
should ScopedPtrMap be in namespace base::, same as ScopedPtrHashMap? Maybe good
for a follow-up..

[Matt Giuca, 2015-06-19 04:03:25]

https://codereview.chromium.org/1096983002/diff/450001/apps/saved_files_servi...
File apps/saved_files_service.cc (right):

https://codereview.chromium.org/1096983002/diff/450001/apps/saved_files_servi...
apps/saved_files_service.cc:323: auto it = registered_file_entries_.find(id);
I'm hesitant to use get() because there's no get() on std::hash_map, and we're
trying to migrate to that later on. (So it probably should be considered
deprecated.)

Shadowing happens in similar ways throughout this file and I don't want to go
fixing even minor unrelated issues in a CL of this breadth. I'll just leave it.

https://codereview.chromium.org/1096983002/diff/450001/apps/saved_files_servi...
apps/saved_files_service.cc:364: auto it = registered_file_entries_.find(id);
As above, I think we should consider get() to be deprecated since it won't be
available on std::hash_map.

https://codereview.chromium.org/1096983002/diff/450001/apps/saved_files_servi...
File apps/saved_files_service.h (right):

https://codereview.chromium.org/1096983002/diff/450001/apps/saved_files_servi...
apps/saved_files_service.h:130: ScopedPtrMap<std::string,
scoped_ptr<SavedFiles>>
I've asked Dana this on http://crbug.com/502128. If we're going to change it, I
want to do it *before* this CL because otherwise there will be a lot of churn.

https://codereview.chromium.org/1096983002/diff/450001/ui/gfx/image/image.h
File ui/gfx/image/image.h (right):

https://codereview.chromium.org/1096983002/diff/450001/ui/gfx/image/image.h#n...
ui/gfx/image/image.h:62: typedef ScopedPtrMap<RepresentationType,
scoped_ptr<internal::ImageRep>>
Thanks for the suggestion. I think I won't do this, just for consistency (I have
updated dozens of typedefs throughout this CL and all of them would have the
same rule, but I don't want to make that sweeping change at the same time).

[Takashi Toyoshima, 2015-06-19 05:04:25]

lgtm for media/midi.
+cc agoode@ for midi alsa port, fyi.

[Adam Goode, 2015-06-19 06:19:17]

On 2015/06/19 05:04:25, Takashi Toyoshima (chromium) wrote:
> lgtm for media/midi.
> +cc agoode@ for midi alsa port, fyi.

lgtm for media/midi!
this is way cleaner than before.

[Matt Giuca, 2015-06-22 05:40:41]

timsteele@: ping.

https://codereview.chromium.org/1096983002/diff/450001/apps/saved_files_servi...
File apps/saved_files_service.h (right):

https://codereview.chromium.org/1096983002/diff/450001/apps/saved_files_servi...
apps/saved_files_service.h:130: ScopedPtrMap<std::string,
scoped_ptr<SavedFiles>>
Done in precursor CL https://codereview.chromium.org/1194373002.

https://codereview.chromium.org/1096983002/diff/450001/chromeos/dbus/gsm_sms_...
File chromeos/dbus/gsm_sms_client.cc (right):

https://codereview.chromium.org/1096983002/diff/450001/chromeos/dbus/gsm_sms_...
chromeos/dbus/gsm_sms_client.cc:219: return it->second;
Missed this earlier.

It actually returns a SMSProxy*. Unfortunately, this will break when we move to
an actual std::map<..., scoped_ptr<...>>, because then it will return a
scoped_ptr.

(This is one of the unfortunate consequences of ScopedPtrMap "faking" a map of
scoped_ptrs; we can't make iterators behave correctly so it's slightly
incompatible at this point.)

[stevenjb, 2015-06-23 17:01:03]

lgtm

https://codereview.chromium.org/1096983002/diff/450001/chromeos/dbus/gsm_sms_...
File chromeos/dbus/gsm_sms_client.cc (right):

https://codereview.chromium.org/1096983002/diff/450001/chromeos/dbus/gsm_sms_...
chromeos/dbus/gsm_sms_client.cc:219: return it->second;
On 2015/06/22 05:40:41, Matt Giuca wrote:
> Missed this earlier.
> 
> It actually returns a SMSProxy*. Unfortunately, this will break when we move
to
> an actual std::map<..., scoped_ptr<...>>, because then it will return a
> scoped_ptr.
> 
> (This is one of the unfortunate consequences of ScopedPtrMap "faking" a map of
> scoped_ptrs; we can't make iterators behave correctly so it's slightly
> incompatible at this point.)

For the record, "EW". I see now how ScopedPtrMap::insert has to take a
scoped_ptr<> and ScopedPtrMap::iterator.second has to be a raw pointer, but...
EW.

[Matt Giuca, 2015-06-24 05:21:35]

timsteele: ping?

https://codereview.chromium.org/1096983002/diff/450001/chromeos/dbus/gsm_sms_...
File chromeos/dbus/gsm_sms_client.cc (right):

https://codereview.chromium.org/1096983002/diff/450001/chromeos/dbus/gsm_sms_...
chromeos/dbus/gsm_sms_client.cc:219: return it->second;
Yeah, it can't really be solved until we get real scoped_ptrs in there. But this
is a step in the right direction.

[tim (not reviewing), 2015-06-24 22:05:35]

tim@chromium.org changed reviewers:
+ maniscalco@chromium.org

[tim (not reviewing), 2015-06-24 22:05:37]

Sorry about the delay. I'm not regularly working on chrome anymore.  +maniscalco

[maniscalco, 2015-06-24 23:43:54]

On 2015/06/24 22:05:37, not reviewing wrote:
> Sorry about the delay. I'm not regularly working on chrome anymore. 
+maniscalco

Thanks for doing this!

sync/* LGTM

[Matt Giuca, 2015-06-25 00:25:45]

timsteele@: Thanks, sorry for the confusion (I don't see your name in OWNERS
file now; maybe it got removed in the interim).

[Matt Giuca, 2015-06-25 01:09:31]

mgiuca@chromium.org changed reviewers:
- tim@chromium.org

[Matt Giuca, 2015-06-25 01:09:48]

The CQ bit was checked by mgiuca@chromium.org

[Matt Giuca, 2015-06-25 01:09:49]

The patchset sent to the CQ was uploaded after l-g-t-m from tapted@chromium.org,
jochen@chromium.org, toyoshim@chromium.org, rsesek@chromium.org,
vabr@chromium.org, agoode@chromium.org
Link to the patchset: https://codereview.chromium.org/1096983002/#ps470001
(title: "Rebase off 1194373002 (move into base:: namespace).")

[commit-bot: I haz the power, 2015-06-25 01:10:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1096983002/470001

[commit-bot: I haz the power, 2015-06-25 01:14:50]

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

[commit-bot: I haz the power, 2015-06-25 01:14:55]

Exceeded global retry quota

[Matt Giuca, 2015-06-25 05:00:46]

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

[Matt Giuca, 2015-06-25 05:00:46]

The patchset sent to the CQ was uploaded after l-g-t-m from tapted@chromium.org,
stevenjb@chromium.org, jochen@chromium.org, toyoshim@chromium.org,
maniscalco@chromium.org, rsesek@chromium.org, vabr@chromium.org,
agoode@chromium.org
Link to the patchset: https://codereview.chromium.org/1096983002/#ps490001
(title: "Rebase.")

[commit-bot: I haz the power, 2015-06-25 05:00:51]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1096983002/490001

[Matt Giuca, 2015-06-25 05:58:39]

The CQ bit was unchecked by mgiuca@chromium.org

[Matt Giuca, 2015-06-25 05:58:50]

The CQ bit was checked by mgiuca@chromium.org

[commit-bot: I haz the power, 2015-06-25 09:01:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1096983002/490001

[commit-bot: I haz the power, 2015-06-25 09:12:12]

Committed patchset #22 (id:490001)

[commit-bot: I haz the power, 2015-06-25 09:13:10]

Patchset 22 (id:??) landed as
https://crrev.com/f59c7a8db11a9c5f5abbba3b0fb6f42059e4ce1d
Cr-Commit-Position: refs/heads/master@{#336096}