[Oilpan] Fix wrong usage of HashSet<WebGLSharedObject*>

Fix wrong usage of HashSet<WebGLSharedObject*>

BUG=515524
Committed: https://crrev.com/9d2fb25681793c32e9cde5dc4ddf961ddb9a4a37
Cr-Commit-Position: refs/heads/master@{#355498}

[peria, 2015-10-22 02:16:11]

Description was changed from

==========
Fix wrong usage of HashSet<WebGL*>

BUG=515524
==========

to

==========
Fix wrong usage of HashSet<WebGLSharedObject*>

BUG=515524
==========

[peria, 2015-10-22 02:16:17]

Patchset #1 (id:1) has been deleted

[peria, 2015-10-22 02:16:37]

peria@chromium.org changed reviewers:
+ haraken@chromium.org, oilpan-reviews@chromium.org

[peria, 2015-10-22 02:16:38]

PTL

[haraken, 2015-10-22 02:27:05]

https://codereview.chromium.org/1418503005/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webgl/WebGLContextGroup.h (right):

https://codereview.chromium.org/1418503005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webgl/WebGLContextGroup.h:75:
WillBeHeapHashSet<RawPtrWillBeUntracedMember<WebGLSharedObject>> m_groupObjects;

This must be
WillBePersistentHeapHashSet<RawPtrWillBeUntracedMember<WebGLSharedObject>>
because WebGLContextGroup is an off-heap object.

yutak@: It would be great if the plugin can detect the error.

Not related to this CL, I noticed one thing.

  class X : public GarbageCollected<X> {
    DEFINE_INLINE_TRACE() { }
    HeapVector<UntracedMember<Y>> m_vec;
  };

I was thinking that the above code is correct but isn't. We must trace the
m_vec. Otherwise, the backing storage of the HeapVector is gone. In other words,
the correct code would be:

  class X : public GarbageCollected<X> {
    DEFINE_INLINE_TRACE() {
      visitor->trace(m_vec);  // This traces the backing storage of the
HeapVector but doesn't trace the untraced members in the vector.
    }
    HeapVector<UntracedMember<Y>> m_vec;
  };

peria@: Can we add more test cases about HeapVector<UntracedMember> and
HeapHashSet<UntracedMember> to HeapTest.cpp?

[peria, 2015-10-22 02:38:10]

https://codereview.chromium.org/1418503005/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webgl/WebGLContextGroup.h (right):

https://codereview.chromium.org/1418503005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webgl/WebGLContextGroup.h:75:
WillBeHeapHashSet<RawPtrWillBeUntracedMember<WebGLSharedObject>> m_groupObjects;
Oops, this was my mistake, but I think this should be
  HashSet<RawPtrWillBeUntracedMember<WebGLSharedObject>>
so, yes, the situation you noticed was what I made this CL wrong.
I think HeapVector<UntracedMember<Y>> should not be allowed.

[haraken, 2015-10-22 03:55:42]

LGTM

https://codereview.chromium.org/1418503005/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webgl/WebGLContextGroup.h (right):

https://codereview.chromium.org/1418503005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webgl/WebGLContextGroup.h:75:
WillBeHeapHashSet<RawPtrWillBeUntracedMember<WebGLSharedObject>> m_groupObjects;
On 2015/10/22 02:38:10, peria wrote:
> Oops, this was my mistake, but I think this should be
>   HashSet<RawPtrWillBeUntracedMember<WebGLSharedObject>>
> so, yes, the situation you noticed was what I made this CL wrong.
> I think HeapVector<UntracedMember<Y>> should not be allowed.

OK. Either way, it's better to have more tests in HeapTest.cpp.

[peria, 2015-10-22 04:49:00]

The CQ bit was checked by peria@chromium.org

[commit-bot: I haz the power, 2015-10-22 04:49:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1418503005/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1418503005/40001

[commit-bot: I haz the power, 2015-10-22 06:24:05]

Committed patchset #2 (id:40001)

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

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