Remove stl_util's deletion function use from chrome/.

Remove stl_util's deletion function use from chrome/.

BUG=555865
Committed: https://crrev.com/d2c61e49116e9936be53e6f326cf702838ca73fa
Cr-Commit-Position: refs/heads/master@{#427193}

[Avi (use Gerrit), 2016-10-22 02:16:08]

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

[commit-bot: I haz the power, 2016-10-22 02:16:35]

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

[Avi (use Gerrit), 2016-10-22 02:30:34]

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

[commit-bot: I haz the power, 2016-10-22 02:30:42]

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, 2016-10-22 03:05:34]

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

[commit-bot: I haz the power, 2016-10-22 03:05:35]

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

[Avi (use Gerrit), 2016-10-22 19:29:59]

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

[commit-bot: I haz the power, 2016-10-22 19:30:08]

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, 2016-10-22 19:47:16]

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

[commit-bot: I haz the power, 2016-10-22 19:47:17]

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

[Avi (use Gerrit), 2016-10-22 20:03:57]

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

[commit-bot: I haz the power, 2016-10-22 20:04:04]

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, 2016-10-22 20:22:55]

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

[commit-bot: I haz the power, 2016-10-22 20:22:56]

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

[Avi (use Gerrit), 2016-10-22 21:21:08]

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

[commit-bot: I haz the power, 2016-10-22 21:21:20]

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, 2016-10-22 21:59:14]

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

[commit-bot: I haz the power, 2016-10-22 21:59:15]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Avi (use Gerrit), 2016-10-22 23:14:04]

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

[commit-bot: I haz the power, 2016-10-22 23:14:11]

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

[Avi (use Gerrit), 2016-10-22 23:53:39]

avi@chromium.org changed reviewers:
+ sky@chromium.org

[commit-bot: I haz the power, 2016-10-22 23:55:24]

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

[commit-bot: I haz the power, 2016-10-22 23:55:25]

Dry run: This issue passed the CQ dry run.

[sky, 2016-10-24 15:58:42]

sky@chromium.org changed reviewers:
+ thakis@chromium.org
- sky@chromium.org

[sky, 2016-10-24 15:58:43]

Load balancing, sky->thakis

[Nico, 2016-10-24 18:41:02]

lgtm except change in first file, and i don't understand
chrome/browser/media_galleries/media_file_system_registry_unittest.cc

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/devtool...
File chrome/browser/devtools/device/port_forwarding_controller.cc (right):

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/devtool...
chrome/browser/devtools/device/port_forwarding_controller.cc:472: delete
entry.second;
I don't understand why this is better than the lhs

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/media_g...
File chrome/browser/media_galleries/media_file_system_registry_unittest.cc
(right):

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/media_g...
chrome/browser/media_galleries/media_file_system_registry_unittest.cc:455:
single_rph_ = rph_factory->ReleaseRPH(profile_.get()).release();
as far as i can tell nothing ever reads this variable.

How does the rph here get deleted?

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/plugins...
File chrome/browser/plugins/plugin_finder.cc (right):

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/plugins...
chrome/browser/plugins/plugin_finder.cc:325: for (auto it =
identifier_plugin_.begin(); it != identifier_plugin_.end();
nit: for-each loop?

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/plugins...
File chrome/browser/plugins/plugin_observer.cc (right):

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/plugins...
chrome/browser/plugins/plugin_observer.cc:384: #if
BUILDFLAG(ENABLE_PLUGIN_INSTALLATION)
this code is probably not covered by trybots or by a default local build (?)

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/printin...
File chrome/browser/printing/background_printing_manager.cc (right):

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/printin...
chrome/browser/printing/background_printing_manager.cc:124: i !=
printing_contents_map_.end(); ++i) {
nit: foreach?

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/process...
File chrome/browser/process_singleton_posix.cc (right):

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/process...
chrome/browser/process_singleton_posix.cc:584: std::unique_ptr<SocketReader>
reader =
nit: maybe no need for local var

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/safe_br...
File
chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc
(right):

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc:621:
for (auto scan = profiles_.begin(); scan != profiles_.end(); ++scan) {
nit: foreach?

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc:733:
for (auto scan = profiles_.begin(); scan != profiles_.end(); ++scan) {
ditto

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc:839:
for (auto scan = profiles_.begin(); scan != profiles_.end(); ++scan) {
ditto

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/safe_br...
File chrome/browser/safe_browsing/safe_browsing_service.cc (right):

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_service.cc:667: for (auto iter =
prefs_map_.begin(); iter != prefs_map_.end(); ++iter) {
nit: foreach?

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/ui/sear...
File chrome/browser/ui/search_engines/template_url_table_model.cc (right):

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/ui/sear...
chrome/browser/ui/search_engines/template_url_table_model.cc:145: for (auto i =
urls.begin(); i != urls.end(); ++i) {
nit: foreach

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/ui/sear...
chrome/browser/ui/search_engines/template_url_table_model.cc:306: for (auto i =
entries_.begin(); i != entries_.end(); ++i) {
nit: foreach?

[Lei Zhang, 2016-10-24 18:45:14]

thestig@chromium.org changed reviewers:
+ thestig@chromium.org

[Lei Zhang, 2016-10-24 18:45:15]

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/media_g...
File chrome/browser/media_galleries/media_file_system_registry_unittest.cc
(right):

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/media_g...
chrome/browser/media_galleries/media_file_system_registry_unittest.cc:455:
single_rph_ = rph_factory->ReleaseRPH(profile_.get()).release();
On 2016/10/24 18:41:02, Nico wrote:
> as far as i can tell nothing ever reads this variable.
> 
> How does the rph here get deleted?

The |single_rph_| declaration above has this comment:

// The RenderProcessHosts are freed when their respective WebContents /
// RenderViewHosts go away.

[Nico, 2016-10-24 18:49:01]

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/media_g...
File chrome/browser/media_galleries/media_file_system_registry_unittest.cc
(right):

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/media_g...
chrome/browser/media_galleries/media_file_system_registry_unittest.cc:455:
single_rph_ = rph_factory->ReleaseRPH(profile_.get()).release();
On 2016/10/24 18:45:14, Lei Zhang wrote:
> On 2016/10/24 18:41:02, Nico wrote:
> > as far as i can tell nothing ever reads this variable.
> > 
> > How does the rph here get deleted?
> 
> The |single_rph_| declaration above has this comment:
> 
> // The RenderProcessHosts are freed when their respective WebContents /
> // RenderViewHosts go away.

But why is this in an env var then? And if something else frees them, why is
this a container of unique_ptrs now? Maybe the comment should be here instead of
in the .h?

[Lei Zhang, 2016-10-24 18:58:18]

On 2016/10/24 18:49:01, Nico wrote:
> But why is this in an env var then? And if something else frees them, why is
> this a container of unique_ptrs now? Maybe the comment should be here instead
of
> in the .h?

It looks like the variables were in use in an earlier patch set in
https://chromiumcodereview.appspot.com/11027051/ but ~10 patch sets later, it
was no longer needed but did not get removed. If you'd like, just omit the
changes to media_file_system_registry_unittest.cc in this CL and I'll clean it
up separately.

[Avi (use Gerrit), 2016-10-24 19:14:08]

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

[commit-bot: I haz the power, 2016-10-24 19:15:03]

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

[Avi (use Gerrit), 2016-10-24 19:15:38]

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/devtool...
File chrome/browser/devtools/device/port_forwarding_controller.cc (right):

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/devtool...
chrome/browser/devtools/device/port_forwarding_controller.cc:472: delete
entry.second;
On 2016/10/24 18:41:01, Nico wrote:
> I don't understand why this is better than the lhs

The existence of STLDelete* functions encourages people to use them rather than
proper owning containers. They're an attractive nuisance.

I couldn't figure out how to turn this into an owning container, so I'm
switching it to what it actually does. I argue that this is a net positive;
using STLDeleteValues implies that this has an owning container but is just
old-school, whereas switching to the delete loop makes it clear that it's
playing ownership games.

In any case, even if you disagree with me and say that it's not necessarily
better than before, making this switch lets me remove the STLDelete* functions,
which makes Chrome better-off overall.

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/media_g...
File chrome/browser/media_galleries/media_file_system_registry_unittest.cc
(right):

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/media_g...
chrome/browser/media_galleries/media_file_system_registry_unittest.cc:455:
single_rph_ = rph_factory->ReleaseRPH(profile_.get()).release();
On 2016/10/24 18:49:00, Nico wrote:
> On 2016/10/24 18:45:14, Lei Zhang wrote:
> > On 2016/10/24 18:41:02, Nico wrote:
> > > as far as i can tell nothing ever reads this variable.
> > > 
> > > How does the rph here get deleted?
> > 
> > The |single_rph_| declaration above has this comment:
> > 
> > // The RenderProcessHosts are freed when their respective WebContents /
> > // RenderViewHosts go away.
> 
> But why is this in an env var then? And if something else frees them, why is
> this a container of unique_ptrs now? Maybe the comment should be here instead
of
> in the .h?

Let me remove this set of files from this CL and figure this out better.

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/plugins...
File chrome/browser/plugins/plugin_finder.cc (right):

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/plugins...
chrome/browser/plugins/plugin_finder.cc:325: for (auto it =
identifier_plugin_.begin(); it != identifier_plugin_.end();
On 2016/10/24 18:41:02, Nico wrote:
> nit: for-each loop?

Done.

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/plugins...
File chrome/browser/plugins/plugin_observer.cc (right):

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/plugins...
chrome/browser/plugins/plugin_observer.cc:384: #if
BUILDFLAG(ENABLE_PLUGIN_INSTALLATION)
On 2016/10/24 18:41:02, Nico wrote:
> this code is probably not covered by trybots or by a default local build (?)

Acknowledged.

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/printin...
File chrome/browser/printing/background_printing_manager.cc (right):

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/printin...
chrome/browser/printing/background_printing_manager.cc:124: i !=
printing_contents_map_.end(); ++i) {
On 2016/10/24 18:41:02, Nico wrote:
> nit: foreach?

Done.

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/process...
File chrome/browser/process_singleton_posix.cc (right):

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/process...
chrome/browser/process_singleton_posix.cc:584: std::unique_ptr<SocketReader>
reader =
On 2016/10/24 18:41:02, Nico wrote:
> nit: maybe no need for local var

Done.

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/safe_br...
File
chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc
(right):

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc:621:
for (auto scan = profiles_.begin(); scan != profiles_.end(); ++scan) {
On 2016/10/24 18:41:02, Nico wrote:
> nit: foreach?

Done.

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc:733:
for (auto scan = profiles_.begin(); scan != profiles_.end(); ++scan) {
On 2016/10/24 18:41:02, Nico wrote:
> ditto

Done.

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc:839:
for (auto scan = profiles_.begin(); scan != profiles_.end(); ++scan) {
On 2016/10/24 18:41:02, Nico wrote:
> ditto

Done.

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/safe_br...
File chrome/browser/safe_browsing/safe_browsing_service.cc (right):

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/safe_br...
chrome/browser/safe_browsing/safe_browsing_service.cc:667: for (auto iter =
prefs_map_.begin(); iter != prefs_map_.end(); ++iter) {
On 2016/10/24 18:41:02, Nico wrote:
> nit: foreach?

Done.

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/ui/sear...
File chrome/browser/ui/search_engines/template_url_table_model.cc (right):

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/ui/sear...
chrome/browser/ui/search_engines/template_url_table_model.cc:145: for (auto i =
urls.begin(); i != urls.end(); ++i) {
On 2016/10/24 18:41:02, Nico wrote:
> nit: foreach

Done.

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/ui/sear...
chrome/browser/ui/search_engines/template_url_table_model.cc:306: for (auto i =
entries_.begin(); i != entries_.end(); ++i) {
On 2016/10/24 18:41:02, Nico wrote:
> nit: foreach?

No, we need i on line 309.

[Nico, 2016-10-24 19:18:26]

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/devtool...
File chrome/browser/devtools/device/port_forwarding_controller.cc (right):

https://codereview.chromium.org/2442953002/diff/100001/chrome/browser/devtool...
chrome/browser/devtools/device/port_forwarding_controller.cc:472: delete
entry.second;
On 2016/10/24 19:15:37, Avi wrote:
> On 2016/10/24 18:41:01, Nico wrote:
> > I don't understand why this is better than the lhs
> 
> The existence of STLDelete* functions encourages people to use them rather
than
> proper owning containers. They're an attractive nuisance.
> 
> I couldn't figure out how to turn this into an owning container, so I'm
> switching it to what it actually does. I argue that this is a net positive;
> using STLDeleteValues implies that this has an owning container but is just
> old-school, whereas switching to the delete loop makes it clear that it's
> playing ownership games.
> 
> In any case, even if you disagree with me and say that it's not necessarily
> better than before, making this switch lets me remove the STLDelete*
functions,
> which makes Chrome better-off overall.

That's a good motivation, thanks.

[Avi (use Gerrit), 2016-10-24 19:43:43]

The CQ bit was unchecked by avi@chromium.org

[Avi (use Gerrit), 2016-10-24 19:43:48]

The CQ bit was checked by avi@chromium.org

[Avi (use Gerrit), 2016-10-24 19:43:49]

The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/2442953002/#ps120001
(title: "thakis")

[commit-bot: I haz the power, 2016-10-24 19:44:10]

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, 2016-10-24 19:47:01]

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

[commit-bot: I haz the power, 2016-10-24 19:47:03]

Try jobs failed on following builders:
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Avi (use Gerrit), 2016-10-24 20:42:23]

The CQ bit was checked by avi@chromium.org

[Avi (use Gerrit), 2016-10-24 20:42:23]

The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/2442953002/#ps140001
(title: "->")

[commit-bot: I haz the power, 2016-10-24 20:43: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, 2016-10-24 21:03:18]

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

[commit-bot: I haz the power, 2016-10-24 21:03:19]

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...)

[Avi (use Gerrit), 2016-10-24 21:40:01]

The CQ bit was checked by avi@chromium.org

[Avi (use Gerrit), 2016-10-24 21:40:02]

The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/2442953002/#ps150036
(title: "fix")

[commit-bot: I haz the power, 2016-10-24 21:40:38]

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, 2016-10-25 00:47:35]

Committed patchset #9 (id:150036)

[commit-bot: I haz the power, 2016-10-25 00:51:05]

Description was changed from

==========
Remove stl_util's deletion function use from chrome/.

BUG=555865
==========

to

==========
Remove stl_util's deletion function use from chrome/.

BUG=555865

Committed: https://crrev.com/d2c61e49116e9936be53e6f326cf702838ca73fa
Cr-Commit-Position: refs/heads/master@{#427193}
==========

[commit-bot: I haz the power, 2016-10-25 00:51:07]

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