Remove stl_util's deletion function use from blimp/.

Remove stl_util's deletion function use from blimp/.

This fixes memory ownership; the objects in the live_unlocked_chunks_ collection are not BlimpDiscardableMemoryAllocator's to destroy.

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

[Avi (use Gerrit), 2016-10-26 22:52:51]

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

[commit-bot: I haz the power, 2016-10-26 22:53:23]

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-26 23:03:34]

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

[commit-bot: I haz the power, 2016-10-26 23:03:35]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Avi (use Gerrit), 2016-10-26 23:06:21]

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

[commit-bot: I haz the power, 2016-10-26 23:07:00]

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-26 23:28:03]

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

[commit-bot: I haz the power, 2016-10-26 23:28:04]

Dry run: This issue passed the CQ dry run.

[Avi (use Gerrit), 2016-10-26 23:39:50]

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

[Avi (use Gerrit), 2016-10-26 23:39:50]

I am removing the STLDelete* functions from Chromium, and I found this use. The
use of STLDeleteElements in this way is incorrect; I am removing it.

BlimpDiscardableMemoryAllocator has a method named
AllocateLockedDiscardableMemory(). AllocateLockedDiscardableMemory returns a
unique_ptr. What that means is that BlimpDiscardableMemoryAllocator is
transferring ownership of the shared memory instance to the caller, and it is
the responsibility of the caller to delete it (likely via the destructor of the
unique_ptr).

Now, all instances of the DiscardableMemory maintain a pointer to the
BlimpDiscardableMemoryAllocator, and insert and remove themselves from that
list. However, the fact that they insert and remove themselves from that list
does not change the ownership promise that AllocateLockedDiscardableMemory made.

Therefore, ~BlimpDiscardableMemoryAllocator cannot delete the objects using
STLDeleteElements. Every element in the live_unlocked_chunks_ was returned by
AllocateLockedDiscardableMemory, and is managed by a unique_ptr. If
STLDeleteElements is used, then we will crash when the unique_ptr tries to
exercise its ownership and delete its owned object, only to find it deleted out
from under it.

We can insist in ~BlimpDiscardableMemoryAllocator that the live_unlocked_chunks_
list is empty, and in fact, it's probably a good idea to do so, otherwise we
have dangling pointers to the allocator, and I made that change.

But we can't delete pointers out from underneath unique_ptrs. That is a
violation of the contract with the caller.

[Wez, 2016-10-26 23:54:40]

The CQ bit was checked by wez@chromium.org

[Wez, 2016-10-26 23:54:40]

lgtm

Thanks for spotting that avi@!

That code was based on components/html_viewer/discardable_memory_manager.cc,
which has since gone, I think, so hopefully there aren't other impls which
inherited that bug.

[commit-bot: I haz the power, 2016-10-26 23:55:20]

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-27 00:01:18]

On 2016/10/26 23:54:40, Wez wrote:
> lgtm
> 
> Thanks for spotting that avi@!
> 
> That code was based on components/html_viewer/discardable_memory_manager.cc,
> which has since gone, I think, so hopefully there aren't other impls which
> inherited that bug.

Yikes!

There appear to be three implementations:
https://cs.chromium.org/search/?q="public+base::DiscardableMemoryAllocator"

It looks like the other two are fine.

[commit-bot: I haz the power, 2016-10-27 00:01:26]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-27 00:03:22]

Description was changed from

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

This fixes memory ownership; the objects in the live_unlocked_chunks_ collection
are not BlimpDiscardableMemoryAllocator's to destroy.

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

to

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

This fixes memory ownership; the objects in the live_unlocked_chunks_ collection
are not BlimpDiscardableMemoryAllocator's to destroy.

BUG=555865

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

[commit-bot: I haz the power, 2016-10-27 00:03:23]

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