Remove WTF HashMap and PassOwnPtr dependencies for CCRenderPass

Remove WTF HashMap and PassOwnPtr dependencies for CCRenderPass

We add a ScopedPtrHashMap and ScopedPtrVector which preserve the behaviour
of HashMap<T, OwnPtr<V> > and Vector<OwnPtr<T>> and use these for CCRenderPass
and any code that creates CCRenderPass objects.

R=enne,jamesr
BUG=152049
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=158729

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=158837

[danakj, 2012-09-25 01:26:46]

PTAL

[jamesr, 2012-09-25 03:53:50]

https://codereview.chromium.org/10979010/diff/2001/cc/CCLayerTreeHostImplTest...
File cc/CCLayerTreeHostImplTest.cpp (right):

https://codereview.chromium.org/10979010/diff/2001/cc/CCLayerTreeHostImplTest...
cc/CCLayerTreeHostImplTest.cpp:3940: if
(!testData.renderPassCache.get(renderPassId))
.contains(renderPassId) ?

https://codereview.chromium.org/10979010/diff/2001/cc/CCLayerTreeHostImplTest...
cc/CCLayerTreeHostImplTest.cpp:3943: scoped_ptr<CCRenderPass> renderPass =
testData.renderPassCache.take(renderPassId);
the old code would leave a null entry in place in the map, this actually removes
the entry. sure this is a safe change to make?

https://codereview.chromium.org/10979010/diff/2001/cc/scoped_ptr_hash_map.h
File cc/scoped_ptr_hash_map.h (right):

https://codereview.chromium.org/10979010/diff/2001/cc/scoped_ptr_hash_map.h#n...
cc/scoped_ptr_hash_map.h:64: for (iterator it = data_.find(k); it !=
data_.end(); it = data_.find(k)) {
do we need to handle multiple entries with the same key? that seems weird

https://codereview.chromium.org/10979010/diff/2001/cc/scoped_ptr_hash_map.h#n...
cc/scoped_ptr_hash_map.h:74: data_.erase(it);
scoped_ptr_vector::take() leaves a null entry in place so you can call .take()
when iterating over the container. do we do that with map?

[danakj, 2012-09-25 16:13:51]

Thanks, PTAL again.

https://codereview.chromium.org/10979010/diff/2001/cc/CCLayerTreeHostImplTest...
File cc/CCLayerTreeHostImplTest.cpp (right):

https://codereview.chromium.org/10979010/diff/2001/cc/CCLayerTreeHostImplTest...
cc/CCLayerTreeHostImplTest.cpp:3940: if
(!testData.renderPassCache.get(renderPassId))
On 2012/09/25 03:53:50, jamesr wrote:
> .contains(renderPassId) ?

oh absolutely.

https://codereview.chromium.org/10979010/diff/2001/cc/CCLayerTreeHostImplTest...
cc/CCLayerTreeHostImplTest.cpp:3943: scoped_ptr<CCRenderPass> renderPass =
testData.renderPassCache.take(renderPassId);
On 2012/09/25 03:53:50, jamesr wrote:
> the old code would leave a null entry in place in the map, this actually
removes
> the entry. sure this is a safe change to make?

i think it doesn't really matter which one we do, unless the same renderpass id
was in the test case twice (a bug), which there aren't. but i'll preserve the
old behaviour here (by changing take())

https://codereview.chromium.org/10979010/diff/2001/cc/scoped_ptr_hash_map.h
File cc/scoped_ptr_hash_map.h (right):

https://codereview.chromium.org/10979010/diff/2001/cc/scoped_ptr_hash_map.h#n...
cc/scoped_ptr_hash_map.h:64: for (iterator it = data_.find(k); it !=
data_.end(); it = data_.find(k)) {
On 2012/09/25 03:53:50, jamesr wrote:
> do we need to handle multiple entries with the same key? that seems weird

oh thanks. I was learning stl again.. and we don't.

https://codereview.chromium.org/10979010/diff/2001/cc/scoped_ptr_hash_map.h#n...
cc/scoped_ptr_hash_map.h:74: data_.erase(it);
On 2012/09/25 03:53:50, jamesr wrote:
> scoped_ptr_vector::take() leaves a null entry in place so you can call .take()
> when iterating over the container. do we do that with map?

Hm, iteration.. We can do that but then things like contains() will not be
affected by take() since that is checking a key not a value.

How about I do that, and add take_and_erase() as well?

[jamesr, 2012-09-25 21:37:42]

lgtm

https://codereview.chromium.org/10979010/diff/10004/cc/scoped_ptr_hash_map.h
File cc/scoped_ptr_hash_map.h (right):

https://codereview.chromium.org/10979010/diff/10004/cc/scoped_ptr_hash_map.h#...
cc/scoped_ptr_hash_map.h:78: data_.insert(std::pair<Key, Value*>(key, NULL));
hmm, can we mutate it->second instead of doing an erase/insert?

[danakj, 2012-09-25 21:42:58]

https://codereview.chromium.org/10979010/diff/10004/cc/scoped_ptr_hash_map.h
File cc/scoped_ptr_hash_map.h (right):

https://codereview.chromium.org/10979010/diff/10004/cc/scoped_ptr_hash_map.h#...
cc/scoped_ptr_hash_map.h:78: data_.insert(std::pair<Key, Value*>(key, NULL));
On 2012/09/25 21:37:42, jamesr wrote:
> hmm, can we mutate it->second instead of doing an erase/insert?

I don't think so. All I see in the hash_map interface is insert() which does not
do replacement. Unless you know something I don't.

[commit-bot: I haz the power, 2012-09-25 21:48:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/10979010/10004

[commit-bot: I haz the power, 2012-09-25 22:02:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/10979010/17001

[commit-bot: I haz the power, 2012-09-25 22:22:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/10979010/16003

[commit-bot: I haz the power, 2012-09-25 22:37:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/10979010/33

[commit-bot: I haz the power, 2012-09-25 23:10:36]

Sorry for I got bad news for ya.
Compile failed with a clobber build.
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-09-26 00:02:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/10979010/11009

[commit-bot: I haz the power, 2012-09-26 00:14:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/10979010/11009

[commit-bot: I haz the power, 2012-09-26 00:52:51]

Sorry for I got bad news for ya.
Compile failed with a clobber build.
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-09-26 00:52:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/10979010/6009

[commit-bot: I haz the power, 2012-09-26 02:53:57]

Change committed as 158729

[commit-bot: I haz the power, 2012-09-26 04:17:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/10979010/19009

[commit-bot: I haz the power, 2012-09-26 07:04:35]

Retried try job too often for step(s) browser_tests

[commit-bot: I haz the power, 2012-09-26 14:06:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/10979010/19009

[commit-bot: I haz the power, 2012-09-26 18:59:59]

Change committed as 158837