Oilpan: Change persistent handles in the Event hierarchy to members

Oilpan: Change persistent handles in the Event hierarchy to members

Persistent handles in on-heap objects are not allowed (because it's likely to cause memory leaks). This CL removes them from the Event hierarchy, which was moved to oilpan's heap in r169941.

BUG=340522
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170022

[haraken, 2014-03-26 04:48:50]

PTAL.

Ian: I noticed that I forgot to change some Members in the Event hierarchy back
to Persistents when I reverted Event back to RefCounted. (e.g.,
DeviceMotionEvent.h already has Members.) This means that we've had Members in
off-heap objects. It would be great if the plugin catch this kind of error (=we
never have Members in off-heap objects).

[kouhei (in TOK), 2014-03-26 04:50:27]

lgtm

[haraken, 2014-03-26 04:57:58]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-03-26 04:58:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/haraken@chromium.org/208743011/1

[commit-bot: I haz the power, 2014-03-26 06:02:14]

Change committed as 170022

[Mads Ager (chromium), 2014-03-26 06:36:03]

LGTM

https://codereview.chromium.org/208743011/diff/1/Source/core/storage/StorageE...
File Source/core/storage/StorageEvent.h (right):

https://codereview.chromium.org/208743011/diff/1/Source/core/storage/StorageE...
Source/core/storage/StorageEvent.h:1: 
Accidental edit?

[haraken, 2014-03-26 06:42:05]

On 2014/03/26 06:36:03, Mads Ager (chromium) wrote:
> LGTM
> 
>
https://codereview.chromium.org/208743011/diff/1/Source/core/storage/StorageE...
> File Source/core/storage/StorageEvent.h (right):
> 
>
https://codereview.chromium.org/208743011/diff/1/Source/core/storage/StorageE...
> Source/core/storage/StorageEvent.h:1: 
> Accidental edit?

Sorry, fixed in https://codereview.chromium.org/212243002/.

[zerny-chromium, 2014-03-26 09:37:32]

On 2014/03/26 04:48:50, haraken wrote:
> PTAL.
> 
> Ian: I noticed that I forgot to change some Members in the Event hierarchy
back
> to Persistents when I reverted Event back to RefCounted. (e.g.,
> DeviceMotionEvent.h already has Members.) This means that we've had Members in
> off-heap objects. It would be great if the plugin catch this kind of error
(=we
> never have Members in off-heap objects).

Thanks for reporting this issue! In what CL did you replace them by
Persistent(s) again, or was the oilpan build "broken" up until you moved Event
back on the managed heap?

Regarding reporting such errors, we need to allow members in classes that are
stack and part allocated. I'll look at strengthening our checks so that we take
the DISALLOW_ALLOCATION macro into account when validating the occurrence of a
Member in a class.

[haraken, 2014-03-26 09:45:10]

On 2014/03/26 09:37:32, zerny-chromium wrote:
> On 2014/03/26 04:48:50, haraken wrote:
> > PTAL.
> > 
> > Ian: I noticed that I forgot to change some Members in the Event hierarchy
> back
> > to Persistents when I reverted Event back to RefCounted. (e.g.,
> > DeviceMotionEvent.h already has Members.) This means that we've had Members
in
> > off-heap objects. It would be great if the plugin catch this kind of error
> (=we
> > never have Members in off-heap objects).
> 
> Thanks for reporting this issue! In what CL did you replace them by
> Persistent(s) again, or was the oilpan build "broken" up until you moved Event
> back on the managed heap?
> 
> Regarding reporting such errors, we need to allow members in classes that are
> stack and part allocated. I'll look at strengthening our checks so that we
take
> the DISALLOW_ALLOCATION macro into account when validating the occurrence of a
> Member in a class.

(1) I moved Event to RefCountedGarbageCollected three weeks ago
(https://codereview.chromium.org/181153003/). Then I made Persistents in the
Event hierarchy to Members.
(2) Because it broke a lot of tests, I reverted the RefCountedGarbageCollected
to RefCounted (https://codereview.chromium.org/185463005/). However, I forgot to
revert Members to Persistents.
(3) Today I noticed the issue.

Between (2) and (3), Event was off the heap but some Events had Members in them.