Oilpan: Allocate all EventTarget derived types on the manged heap.

Oilpan: Allocate all EventTarget derived types on the manged heap.

This is the first step of three in moving over EventTarget. The next step is to update the IDL of EventTarget to be WillBeGarbageCollected, and then to replace all RefPtr uses. Once all Node RefPtr uses have also been replaced, we can remove the ref-count from Node.

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

[zerny-chromium, 2014-05-26 14:59:12]

Sorry for the large'ish CL. Most changes are very simple. A few required
replacing OwnPtr+back-ref with a traceable object that has its own keep-alive
ref via RefCountedGarbageCollected. Those cases will go away again once all
RefPtr<EventTarget> uses are removed by follow-up CLs. PTAL

[haraken, 2014-05-27 01:39:19]

Mostly looks good!

https://codereview.chromium.org/307433003/diff/1/Source/core/events/EventTarg...
File Source/core/events/EventTarget.h (right):

https://codereview.chromium.org/307433003/diff/1/Source/core/events/EventTarg...
Source/core/events/EventTarget.h:232: #endif

Can we write this:

#define REFCOUNTED_EVENT_TARGET(className)
DEFINE_EVENT_TARGET_REFCOUNTING(RefCountedWillBeRefCountedGarbageCollected<className>)

?

https://codereview.chromium.org/307433003/diff/1/Source/core/html/ime/InputMe...
File Source/core/html/ime/InputMethodContext.h (right):

https://codereview.chromium.org/307433003/diff/1/Source/core/html/ime/InputMe...
Source/core/html/ime/InputMethodContext.h:61: #endif

Can you avoid writing this by using REFCOUNTED_EVENT_TARGET(InputMethodContext)
?

https://codereview.chromium.org/307433003/diff/1/Source/core/xml/XMLHttpReque...
File Source/core/xml/XMLHttpRequestUpload.h (right):

https://codereview.chromium.org/307433003/diff/1/Source/core/xml/XMLHttpReque...
Source/core/xml/XMLHttpRequestUpload.h:60: #endif

Can we avoid writing this by using DEFINE_EVENT_TARGET(XMLHttpRequestUpload) ?

https://codereview.chromium.org/307433003/diff/1/Source/modules/mediastream/M...
File Source/modules/mediastream/MediaStreamTrack.h (right):

https://codereview.chromium.org/307433003/diff/1/Source/modules/mediastream/M...
Source/modules/mediastream/MediaStreamTrack.h:102: Vector<Observer*>
m_observers;

Probably should we make this a HeapHashSet of a weak members?

[zerny-chromium, 2014-05-27 07:34:32]

Thanks for the review!

https://codereview.chromium.org/307433003/diff/1/Source/core/events/EventTarg...
File Source/core/events/EventTarget.h (right):

https://codereview.chromium.org/307433003/diff/1/Source/core/events/EventTarg...
Source/core/events/EventTarget.h:232: #endif
On 2014/05/27 01:39:19, haraken wrote:
> 
> Can we write this:
> 
> #define REFCOUNTED_EVENT_TARGET(className)
>
DEFINE_EVENT_TARGET_REFCOUNTING(RefCountedWillBeRefCountedGarbageCollected<className>)
> 
> ?

Yes, that is simpler :-) Done.

https://codereview.chromium.org/307433003/diff/1/Source/core/html/ime/InputMe...
File Source/core/html/ime/InputMethodContext.h (right):

https://codereview.chromium.org/307433003/diff/1/Source/core/html/ime/InputMe...
Source/core/html/ime/InputMethodContext.h:61: #endif
On 2014/05/27 01:39:19, haraken wrote:
> 
> Can you avoid writing this by using
REFCOUNTED_EVENT_TARGET(InputMethodContext)
> ?

Not without some #if hacking since InputMethodContext is not RefCounted in the
non-oilpan build.

https://codereview.chromium.org/307433003/diff/1/Source/core/xml/XMLHttpReque...
File Source/core/xml/XMLHttpRequestUpload.h (right):

https://codereview.chromium.org/307433003/diff/1/Source/core/xml/XMLHttpReque...
Source/core/xml/XMLHttpRequestUpload.h:60: #endif
On 2014/05/27 01:39:19, haraken wrote:
> 
> Can we avoid writing this by using DEFINE_EVENT_TARGET(XMLHttpRequestUpload) ?

Ditto here.

https://codereview.chromium.org/307433003/diff/1/Source/modules/mediastream/M...
File Source/modules/mediastream/MediaStreamTrack.h (right):

https://codereview.chromium.org/307433003/diff/1/Source/modules/mediastream/M...
Source/modules/mediastream/MediaStreamTrack.h:102: Vector<Observer*>
m_observers;
On 2014/05/27 01:39:19, haraken wrote:
> 
> Probably should we make this a HeapHashSet of a weak members?

I'll defer that to a follow-up CL.

[haraken, 2014-05-27 07:56:25]

LGTM.

https://codereview.chromium.org/307433003/diff/1/Source/modules/mediastream/M...
File Source/modules/mediastream/MediaStreamTrack.h (right):

https://codereview.chromium.org/307433003/diff/1/Source/modules/mediastream/M...
Source/modules/mediastream/MediaStreamTrack.h:102: Vector<Observer*>
m_observers;
On 2014/05/27 07:34:33, zerny-chromium wrote:
> On 2014/05/27 01:39:19, haraken wrote:
> > 
> > Probably should we make this a HeapHashSet of a weak members?
> 
> I'll defer that to a follow-up CL.

keishi@ just landed a CL that moves modules/mediastream to oilpan's heap. You'll
need to rebase with it, but the rebase will resolve this issue.

[Mads Ager (chromium), 2014-05-27 08:09:09]

LGTM2

[zerny-chromium, 2014-05-27 13:18:28]

The CQ bit was checked by zerny@chromium.org

[commit-bot: I haz the power, 2014-05-27 13:18:34]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/zerny@chromium.org/307433003/40001

[commit-bot: I haz the power, 2014-05-27 14:40:33]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/9261)

[commit-bot: I haz the power, 2014-05-27 15:31:17]

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

[commit-bot: I haz the power, 2014-05-27 15:31:18]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/9275)

[zerny-chromium, 2014-05-28 09:13:12]

The CQ bit was checked by zerny@chromium.org

[commit-bot: I haz the power, 2014-05-28 09:13:24]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/zerny@chromium.org/307433003/60001

[commit-bot: I haz the power, 2014-05-28 09:13:49]

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

[commit-bot: I haz the power, 2014-05-28 09:13:50]

Failed to apply patch for Source/core/animation/AnimationPlayer.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/core/animation/AnimationPlayer.h
  Hunk #1 FAILED at 41.
  Hunk #2 succeeded at 153 with fuzz 1 (offset 6 lines).
  1 out of 2 hunks FAILED -- saving rejects to file
Source/core/animation/AnimationPlayer.h.rej

Patch:       Source/core/animation/AnimationPlayer.h
Index: Source/core/animation/AnimationPlayer.h
diff --git a/Source/core/animation/AnimationPlayer.h
b/Source/core/animation/AnimationPlayer.h
index
be2c92f59ffd6f64c7b27b9b939e3c3a0dca7fc3..30be306094274073a30cbf54651371774177e8d3
100644
--- a/Source/core/animation/AnimationPlayer.h
+++ b/Source/core/animation/AnimationPlayer.h
@@ -41,7 +41,8 @@ class AnimationTimeline;
 class ExceptionState;
 
 class AnimationPlayer FINAL : public
RefCountedWillBeRefCountedGarbageCollected<AnimationPlayer>, public
EventTargetWithInlineData {
-   
DEFINE_EVENT_TARGET_REFCOUNTING(RefCountedWillBeRefCountedGarbageCollected<AnimationPlayer>);
+    REFCOUNTED_EVENT_TARGET(AnimationPlayer);
+    WILL_BE_USING_GARBAGE_COLLECTED_MIXIN(AnimationPlayer);
 public:
 
     ~AnimationPlayer();
@@ -147,7 +148,7 @@ public:
 
     virtual bool addEventListener(const AtomicString& eventType,
PassRefPtr<EventListener>, bool useCapture = false) OVERRIDE;
 
-    void trace(Visitor*);
+    virtual void trace(Visitor*) OVERRIDE;
 
 private:
     AnimationPlayer(AnimationTimeline&, AnimationNode*);

[zerny-chromium, 2014-05-28 09:23:01]

The CQ bit was checked by zerny@chromium.org

[commit-bot: I haz the power, 2014-05-28 09:23:19]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/zerny@chromium.org/307433003/80001

[commit-bot: I haz the power, 2014-05-28 10:44:02]

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

[commit-bot: I haz the power, 2014-05-28 10:44:03]

Failed to apply patch for Source/core/animation/AnimationPlayer.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/core/animation/AnimationPlayer.cpp
  Hunk #1 FAILED at 451.
  1 out of 1 hunk FAILED -- saving rejects to file
Source/core/animation/AnimationPlayer.cpp.rej

Patch:       Source/core/animation/AnimationPlayer.cpp
Index: Source/core/animation/AnimationPlayer.cpp
diff --git a/Source/core/animation/AnimationPlayer.cpp
b/Source/core/animation/AnimationPlayer.cpp
index
99e1ec0c103297112771994177de51d78e9d27d4..87b9583e5f9e2bd8de76738e861f2c605ea4ec94
100644
--- a/Source/core/animation/AnimationPlayer.cpp
+++ b/Source/core/animation/AnimationPlayer.cpp
@@ -451,6 +451,7 @@ void AnimationPlayer::trace(Visitor* visitor)
 {
     visitor->trace(m_content);
     visitor->trace(m_timeline);
+    EventTargetWithInlineData::trace(visitor);
 }
 
 } // namespace

[zerny-chromium, 2014-05-28 12:36:01]

The CQ bit was checked by zerny@chromium.org

[commit-bot: I haz the power, 2014-05-28 12:36:33]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/zerny@chromium.org/307433003/100001

[commit-bot: I haz the power, 2014-05-28 13:58:04]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/9454)

[commit-bot: I haz the power, 2014-05-28 14:46:48]

Change committed as 174973