Oilpan: Move core/clipboard/ to oilpan's heap

Oilpan: Move core/clipboard/ to oilpan's heap

This CL moves Clipboard, DataObject, DataObjectItem, DataTransferItem, DataTransferItemList and DragState to oilpan's heap.

The tricky part is the way WebDragData keeps the DataObject alive. In the non-oilpan world, WebDragData keeps the DataObject alive by having WebDragDataPrivate call ref()/deref(). On the other hand, in the oilpan world, WebDragData keeps the DataObject alive by having WebDragDataPrivate retain a persistent handle to the DataObject.

Since this difference leads to a substantial amount of code difference, I used the oilpan flag and split the code between the oilpan world and the non-oilpan world. See WebDragData.cpp for more details.

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

[haraken, 2014-02-17 07:35:45]

PTAL

This is a first CL that introduces a persistent handle to WebXXXPrivate. See
WebDragData.cpp for more details.

[wibling-chromium, 2014-02-17 10:15:34]

lgtm

https://codereview.chromium.org/169323002/diff/1/Source/core/events/Event.h
File Source/core/events/Event.h (right):

https://codereview.chromium.org/169323002/diff/1/Source/core/events/Event.h#n...
Source/core/events/Event.h:30: #include "heap/Handle.h"
Why this include?

[haraken, 2014-02-17 10:26:53]

I think I should wait for sof's WebPrivateGarbageCollectedPtr change to land.

sof: Once your CL lands, we can dramatically simplify WebDragData.cpp, right?

https://codereview.chromium.org/169323002/diff/1/Source/core/events/Event.h
File Source/core/events/Event.h (right):

https://codereview.chromium.org/169323002/diff/1/Source/core/events/Event.h#n...
Source/core/events/Event.h:30: #include "heap/Handle.h"
On 2014/02/17 10:15:35, wibling-chromium wrote:
> Why this include?

To avoid including heap/Handle.h in a lot of files that inherit Event. I'm not
sure if this is a good thing or not.

[sof, 2014-02-17 10:33:42]

On 2014/02/17 10:26:53, haraken wrote:
> I think I should wait for sof's WebPrivateGarbageCollectedPtr change to land.
> 
> sof: Once your CL lands, we can dramatically simplify WebDragData.cpp, right?
> 

I haven't tried, but it does look like the WebDataPrivate field could turn into
a WebPrivateGarbageCollectedPtr<DataObject> instead. Hopefully.

[zerny-chromium, 2014-02-17 10:43:12]

There is a subtle issue with this change because the public interface uses the
value of m_private in isNull:

      bool isNull() const { return !m_private; }

That needs to use the contents of the persistent inside m_private (I had this
issue when moving over WebNode on the branch).

With that fixed, lgtm

https://codereview.chromium.org/169323002/diff/1/Source/core/clipboard/DataTr...
File Source/core/clipboard/DataTransferItem.h (right):

https://codereview.chromium.org/169323002/diff/1/Source/core/clipboard/DataTr...
Source/core/clipboard/DataTransferItem.h:49: class DataTransferItem : public
RefCountedWillBeGarbageCollected<DataTransferItem>, public ScriptWrappable {
+Finalized

https://codereview.chromium.org/169323002/diff/1/Source/core/clipboard/DataTr...
File Source/core/clipboard/DataTransferItemList.h (right):

https://codereview.chromium.org/169323002/diff/1/Source/core/clipboard/DataTr...
Source/core/clipboard/DataTransferItemList.h:49: class DataTransferItemList :
public RefCountedWillBeGarbageCollected<DataTransferItemList>, public
ScriptWrappable {
+Finalized

[haraken, 2014-02-19 06:28:50]

sof: Can I now use the WebPrivatePtr in this clipboard CL?

[sof, 2014-02-19 06:46:56]

On 2014/02/19 06:28:50, haraken wrote:
> sof: Can I now use the WebPrivatePtr in this clipboard CL?

Yes, might it work to have a WebPrivatePtr<DataObject> on WebDragData directly?

[sof, 2014-02-19 07:11:18]

https://codereview.chromium.org/169323002/diff/1/Source/core/clipboard/DataOb...
File Source/core/clipboard/DataObject.cpp (right):

https://codereview.chromium.org/169323002/diff/1/Source/core/clipboard/DataOb...
Source/core/clipboard/DataObject.cpp:270: void DataObject::trace(Visitor*
visitor)
This is now a Supplementable that's also garbage collected. Do you need to
delegate to the Supplementable trace() method also? (Or provide a FIXME if for
later.)

[haraken, 2014-02-19 08:06:15]

Updated the CL using the new WebPrivatePtr.

Would you double-check the change to WebDragData.{h,cpp} if I'm using the
awesome WebPrivatePtr correctly?

https://codereview.chromium.org/169323002/diff/1/Source/core/clipboard/DataOb...
File Source/core/clipboard/DataObject.cpp (right):

https://codereview.chromium.org/169323002/diff/1/Source/core/clipboard/DataOb...
Source/core/clipboard/DataObject.cpp:270: void DataObject::trace(Visitor*
visitor)
On 2014/02/19 07:11:18, sof wrote:
> This is now a Supplementable that's also garbage collected. Do you need to
> delegate to the Supplementable trace() method also? (Or provide a FIXME if for
> later.)

Good point.

However, I cannot find any class that is supplementing DataObject. Probably can
we drop Supplementable from DataObject?

[sof, 2014-02-19 08:10:06]

https://codereview.chromium.org/169323002/diff/1/Source/core/clipboard/DataOb...
File Source/core/clipboard/DataObject.cpp (right):

https://codereview.chromium.org/169323002/diff/1/Source/core/clipboard/DataOb...
Source/core/clipboard/DataObject.cpp:270: void DataObject::trace(Visitor*
visitor)
On 2014/02/19 08:06:16, haraken wrote:
> On 2014/02/19 07:11:18, sof wrote:
> > This is now a Supplementable that's also garbage collected. Do you need to
> > delegate to the Supplementable trace() method also? (Or provide a FIXME if
for
> > later.)
> 
> Good point.
> 
> However, I cannot find any class that is supplementing DataObject. Probably
can
> we drop Supplementable from DataObject?

DraggedIsolatedFileSystem is the only one I could find. filesystem/ hasn't been
given the Oilpan treatment yet though.

[haraken, 2014-02-19 08:15:27]

On 2014/02/19 08:10:06, sof wrote:
>
https://codereview.chromium.org/169323002/diff/1/Source/core/clipboard/DataOb...
> File Source/core/clipboard/DataObject.cpp (right):
> 
>
https://codereview.chromium.org/169323002/diff/1/Source/core/clipboard/DataOb...
> Source/core/clipboard/DataObject.cpp:270: void DataObject::trace(Visitor*
> visitor)
> On 2014/02/19 08:06:16, haraken wrote:
> > On 2014/02/19 07:11:18, sof wrote:
> > > This is now a Supplementable that's also garbage collected. Do you need to
> > > delegate to the Supplementable trace() method also? (Or provide a FIXME if
> for
> > > later.)
> > 
> > Good point.
> > 
> > However, I cannot find any class that is supplementing DataObject. Probably
> can
> > we drop Supplementable from DataObject?
> 
> DraggedIsolatedFileSystem is the only one I could find. filesystem/ hasn't
been
> given the Oilpan treatment yet though.

Nice catch. I'll address the Supplementable tracing in a follow-up CL. I added a
FIXME at the moment. PTAL.

What we need to do is to make DataObject trace Supplementable and then make the
Supplementable trace all supplementing objects (e.g.,
DraggedIsolatedFileSystem), right?

[sof, 2014-02-19 08:30:24]

looks good to me, the WebDragData simplifications came out well.

https://codereview.chromium.org/169323002/diff/330002/Source/web/WebDragData.cpp
File Source/web/WebDragData.cpp (right):

https://codereview.chromium.org/169323002/diff/330002/Source/web/WebDragData....
Source/web/WebDragData.cpp:97: DataObject* WebDragData::getValue() const
I'm not sure getValue() is worth having in this setting; what do you think?

[sof, 2014-02-19 08:35:04]

On 2014/02/19 08:15:27, haraken wrote:
> On 2014/02/19 08:10:06, sof wrote:
> >
>
https://codereview.chromium.org/169323002/diff/1/Source/core/clipboard/DataOb...
> > File Source/core/clipboard/DataObject.cpp (right):
> > 
> >
>
https://codereview.chromium.org/169323002/diff/1/Source/core/clipboard/DataOb...
> > Source/core/clipboard/DataObject.cpp:270: void DataObject::trace(Visitor*
> > visitor)
> > On 2014/02/19 08:06:16, haraken wrote:
> > > On 2014/02/19 07:11:18, sof wrote:
> > > > This is now a Supplementable that's also garbage collected. Do you need
to
> > > > delegate to the Supplementable trace() method also? (Or provide a FIXME
if
> > for
> > > > later.)
> > > 
> > > Good point.
> > > 
> > > However, I cannot find any class that is supplementing DataObject.
Probably
> > can
> > > we drop Supplementable from DataObject?
> > 
> > DraggedIsolatedFileSystem is the only one I could find. filesystem/ hasn't
> been
> > given the Oilpan treatment yet though.
> 
> Nice catch. I'll address the Supplementable tracing in a follow-up CL. I added
a
> FIXME at the moment. PTAL.
> 

Makes good sense to me.

> What we need to do is to make DataObject trace Supplementable and then make
the
> Supplementable trace all supplementing objects (e.g.,
> DraggedIsolatedFileSystem), right?

Yes, perhaps delay that until most Supplements are ready? (i.e., would be a pity
to have to run around adding dummy trace()s to all at the same time.)

Remembering to add calls to the Supplementable trace() from the parent will be
something to watch out for. Would be great if tool support would catch that.. :)

[haraken, 2014-02-19 08:37:11]

https://codereview.chromium.org/169323002/diff/330002/Source/web/WebDragData.cpp
File Source/web/WebDragData.cpp (right):

https://codereview.chromium.org/169323002/diff/330002/Source/web/WebDragData....
Source/web/WebDragData.cpp:97: DataObject* WebDragData::getValue() const
On 2014/02/19 08:30:24, sof wrote:
> I'm not sure getValue() is worth having in this setting; what do you think?

Yeah, this might be unnecessary.

In the experimental branch, 'operator WTF::PassRefPtr<WebCore::DataObject>()'
caused some issue, so we introduced getValue() to remove the implicit
conversion. If I can remove the implicit conversion without introducing
getValue(), I'm happy to remove getValue(). Let me try.

[haraken, 2014-02-19 08:40:43]

> Yes, perhaps delay that until most Supplements are ready? (i.e., would be a
pity
> to have to run around adding dummy trace()s to all at the same time.)

Sounds like a plan. At the moment we can just use persistent handles in
supplementing objects. Once most supplementing objects are ready to get on the
heap, we can start dispatching trace() from the supplementable object.

[haraken, 2014-02-19 08:52:29]

On 2014/02/19 08:37:11, haraken wrote:
>
https://codereview.chromium.org/169323002/diff/330002/Source/web/WebDragData.cpp
> File Source/web/WebDragData.cpp (right):
> 
>
https://codereview.chromium.org/169323002/diff/330002/Source/web/WebDragData....
> Source/web/WebDragData.cpp:97: DataObject* WebDragData::getValue() const
> On 2014/02/19 08:30:24, sof wrote:
> > I'm not sure getValue() is worth having in this setting; what do you think?
> 
> Yeah, this might be unnecessary.
> 
> In the experimental branch, 'operator WTF::PassRefPtr<WebCore::DataObject>()'
> caused some issue, so we introduced getValue() to remove the implicit
> conversion. If I can remove the implicit conversion without introducing
> getValue(), I'm happy to remove getValue(). Let me try.

The conversion is necessary in WebFrameView.cpp. So I left getValue() and used
it in WebFrameView.cpp, but replaced all getValue()s in WebDragData.cpp with
m_private.

[sof, 2014-02-19 08:58:49]

On 2014/02/19 08:52:29, haraken wrote:
> On 2014/02/19 08:37:11, haraken wrote:
> >
>
https://codereview.chromium.org/169323002/diff/330002/Source/web/WebDragData.cpp
> > File Source/web/WebDragData.cpp (right):
> > 
> >
>
https://codereview.chromium.org/169323002/diff/330002/Source/web/WebDragData....
> > Source/web/WebDragData.cpp:97: DataObject* WebDragData::getValue() const
> > On 2014/02/19 08:30:24, sof wrote:
> > > I'm not sure getValue() is worth having in this setting; what do you
think?
> > 
> > Yeah, this might be unnecessary.
> > 
> > In the experimental branch, 'operator
WTF::PassRefPtr<WebCore::DataObject>()'
> > caused some issue, so we introduced getValue() to remove the implicit
> > conversion. If I can remove the implicit conversion without introducing
> > getValue(), I'm happy to remove getValue(). Let me try.
> 
> The conversion is necessary in WebFrameView.cpp. So I left getValue() and used
> it in WebFrameView.cpp, but replaced all getValue()s in WebDragData.cpp with
> m_private.

lgtm from here. That works - an alternative is to provide a conversion to
PassRefPtrWillBeRawPtr<DataObject>, I suppose.

[haraken, 2014-02-19 09:38:17]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-02-19 09:38:31]

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

[Mads Ager (chromium), 2014-02-19 09:54:37]

https://codereview.chromium.org/169323002/diff/370002/Source/core/clipboard/D...
File Source/core/clipboard/DataObject.h (right):

https://codereview.chromium.org/169323002/diff/370002/Source/core/clipboard/D...
Source/core/clipboard/DataObject.h:53: class DataObject : public
RefCountedWillBeGarbageCollected<DataObject>, public Supplementable<DataObject>
{
This needs to be finalized. Supplementable has a suplement map that has to be
destructed.

https://codereview.chromium.org/169323002/diff/370002/Source/core/clipboard/D...
File Source/core/clipboard/DataTransferItem.h (right):

https://codereview.chromium.org/169323002/diff/370002/Source/core/clipboard/D...
Source/core/clipboard/DataTransferItem.h:49: class DataTransferItem : public
RefCountedWillBeGarbageCollected<DataTransferItem>, public ScriptWrappable {
Finalized because of ScriptWrappable.

https://codereview.chromium.org/169323002/diff/370002/Source/core/clipboard/D...
File Source/core/clipboard/DataTransferItemList.h (right):

https://codereview.chromium.org/169323002/diff/370002/Source/core/clipboard/D...
Source/core/clipboard/DataTransferItemList.h:49: class DataTransferItemList :
public RefCountedWillBeGarbageCollected<DataTransferItemList>, public
ScriptWrappable {
Finalized because of ScriptWrappable?

[tkent, 2014-02-19 09:54:43]

lgtm

https://codereview.chromium.org/169323002/diff/370002/public/platform/WebDrag...
File public/platform/WebDragData.h (left):

https://codereview.chromium.org/169323002/diff/370002/public/platform/WebDrag...
public/platform/WebDragData.h:39: #if BLINK_IMPLEMENTATION
Please do not remove the #if.  It helps to avoid using WebCore classes in public
API.

[tkent, 2014-02-19 09:55:42]

The CQ bit was unchecked by tkent@chromium.org

[haraken, 2014-02-19 10:37:23]

Thanks for review!

https://codereview.chromium.org/169323002/diff/370002/Source/core/clipboard/D...
File Source/core/clipboard/DataObject.h (right):

https://codereview.chromium.org/169323002/diff/370002/Source/core/clipboard/D...
Source/core/clipboard/DataObject.h:53: class DataObject : public
RefCountedWillBeGarbageCollected<DataObject>, public Supplementable<DataObject>
{
On 2014/02/19 09:54:37, Mads Ager (chromium) wrote:
> This needs to be finalized. Supplementable has a suplement map that has to be
> destructed.

Done.

https://codereview.chromium.org/169323002/diff/370002/Source/core/clipboard/D...
File Source/core/clipboard/DataTransferItem.h (right):

https://codereview.chromium.org/169323002/diff/370002/Source/core/clipboard/D...
Source/core/clipboard/DataTransferItem.h:49: class DataTransferItem : public
RefCountedWillBeGarbageCollected<DataTransferItem>, public ScriptWrappable {
On 2014/02/19 09:54:37, Mads Ager (chromium) wrote:
> Finalized because of ScriptWrappable.

Done.

https://codereview.chromium.org/169323002/diff/370002/Source/core/clipboard/D...
File Source/core/clipboard/DataTransferItemList.h (right):

https://codereview.chromium.org/169323002/diff/370002/Source/core/clipboard/D...
Source/core/clipboard/DataTransferItemList.h:49: class DataTransferItemList :
public RefCountedWillBeGarbageCollected<DataTransferItemList>, public
ScriptWrappable {
On 2014/02/19 09:54:37, Mads Ager (chromium) wrote:
> Finalized because of ScriptWrappable?

Done.

https://codereview.chromium.org/169323002/diff/370002/public/platform/WebDrag...
File public/platform/WebDragData.h (left):

https://codereview.chromium.org/169323002/diff/370002/public/platform/WebDrag...
public/platform/WebDragData.h:39: #if BLINK_IMPLEMENTATION
On 2014/02/19 09:54:44, tkent wrote:
> Please do not remove the #if.  It helps to avoid using WebCore classes in
public
> API.

Done. I reverted this #if and added #if to the line 109.

[haraken, 2014-02-19 10:37:31]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-02-19 10:37:48]

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

[Mads Ager (chromium), 2014-02-19 10:47:12]

LGTM modulo a question on the use of WebPrivatePtr

https://codereview.chromium.org/169323002/diff/550001/public/platform/WebDrag...
File public/platform/WebDragData.h (right):

https://codereview.chromium.org/169323002/diff/550001/public/platform/WebDrag...
public/platform/WebDragData.h:112: WebPrivatePtr<WebCore::DataObject> m_private;
I don't think this will work. Isn't m_private needed for isNull for example?

It looks like the other uses of WebPrivatePtr have the forward declaration and
the m_private declaration outside of the INSIDE_BLINK #ifdef.

[haraken, 2014-02-19 10:51:14]

https://codereview.chromium.org/169323002/diff/550001/public/platform/WebDrag...
File public/platform/WebDragData.h (right):

https://codereview.chromium.org/169323002/diff/550001/public/platform/WebDrag...
public/platform/WebDragData.h:112: WebPrivatePtr<WebCore::DataObject> m_private;
On 2014/02/19 10:47:13, Mads Ager (chromium) wrote:
> I don't think this will work. Isn't m_private needed for isNull for example?
> 
> It looks like the other uses of WebPrivatePtr have the forward declaration and
> the m_private declaration outside of the INSIDE_BLINK #ifdef.

You're right. This CL compiles by accident probably because the code is not
used.

I'll remove the #if from here and the forward declaration.

[haraken, 2014-02-19 10:52:09]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-02-19 10:52:17]

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

[commit-bot: I haz the power, 2014-02-19 11:32:03]

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

[commit-bot: I haz the power, 2014-02-19 11:32:03]

Retried try job too often on linux_blink_rel for step(s) webkit_unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[haraken, 2014-02-19 11:39:04]

hmm, WebFrameTest.DisambiguationPopup in webkit_unittests is failing. Looking.

[haraken, 2014-02-19 12:05:41]

https://codereview.chromium.org/169323002/diff/790001/Source/core/page/EventH...
File Source/core/page/EventHandler.cpp (right):

https://codereview.chromium.org/169323002/diff/790001/Source/core/page/EventH...
Source/core/page/EventHandler.cpp:313:
DEFINE_STATIC_LOCAL(Persistent<DragState>, state, (new DragState()));

This was a bug. I had to add 'new DragState()'.

[haraken, 2014-02-19 12:05:49]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-02-19 12:06:05]

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

[sof, 2014-02-19 12:43:39]

Will complete a core/fileapi/ CL once this lands.

[haraken, 2014-02-19 12:48:13]

On 2014/02/19 12:43:39, sof wrote:
> Will complete a core/fileapi/ CL once this lands.

BTW, what's a relationship between core/fileapi/ and modules/filesystem?

[commit-bot: I haz the power, 2014-02-19 13:06:58]

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

[commit-bot: I haz the power, 2014-02-19 13:06:59]

Retried try job too often on mac_layout for step(s) webkit_lint
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_layout...

[sof, 2014-02-19 13:09:01]

On 2014/02/19 12:48:13, haraken wrote:
> On 2014/02/19 12:43:39, sof wrote:
> > Will complete a core/fileapi/ CL once this lands.
> 
> BTW, what's a relationship between core/fileapi/ and modules/filesystem?

File access API vs (virtual) file system API, is one (approximate) way to
characterize it, I think.

I can have a go at filesystem/ after that.

[sof, 2014-02-19 14:13:58]

https://codereview.chromium.org/169323002/diff/790001/Source/core/page/DragSt...
File Source/core/page/DragState.h (right):

https://codereview.chromium.org/169323002/diff/790001/Source/core/page/DragSt...
Source/core/page/DragState.h:38: class DragState : public
RefCountedWillBeGarbageCollectedFinalized<DragState> {
Could this be NoBaseWillBeGarbageCollectedFinalized instead?

[Mads Ager (chromium), 2014-02-19 14:43:33]

https://codereview.chromium.org/169323002/diff/790001/Source/core/page/DragSt...
File Source/core/page/DragState.h (right):

https://codereview.chromium.org/169323002/diff/790001/Source/core/page/DragSt...
Source/core/page/DragState.h:38: class DragState : public
RefCountedWillBeGarbageCollectedFinalized<DragState> {
On 2014/02/19 14:13:59, sof wrote:
> Could this be NoBaseWillBeGarbageCollectedFinalized instead? 

Good catch, yes, it should be!

[haraken, 2014-02-19 14:54:42]

On 2014/02/19 14:43:33, Mads Ager (chromium) wrote:
>
https://codereview.chromium.org/169323002/diff/790001/Source/core/page/DragSt...
> File Source/core/page/DragState.h (right):
> 
>
https://codereview.chromium.org/169323002/diff/790001/Source/core/page/DragSt...
> Source/core/page/DragState.h:38: class DragState : public
> RefCountedWillBeGarbageCollectedFinalized<DragState> {
> On 2014/02/19 14:13:59, sof wrote:
> > Could this be NoBaseWillBeGarbageCollectedFinalized instead? 
> 
> Good catch, yes, it should be!

Nice catch. Looks like several tests are failing. I'll take a look tomorrow
morning.

[haraken, 2014-02-21 06:00:56]

>
https://codereview.chromium.org/169323002/diff/790001/Source/core/page/DragSt...
> > Source/core/page/DragState.h:38: class DragState : public
> > RefCountedWillBeGarbageCollectedFinalized<DragState> {
> > On 2014/02/19 14:13:59, sof wrote:
> > > Could this be NoBaseWillBeGarbageCollectedFinalized instead? 

Done.

Also I think I've fixed all test failures. Landing.

[haraken, 2014-02-21 06:01:05]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-02-21 06:01:19]

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

[commit-bot: I haz the power, 2014-02-21 07:14:04]

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

[commit-bot: I haz the power, 2014-02-21 07:14:05]

Retried try job too often on mac_layout for step(s) webkit_lint
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_layout...

[haraken, 2014-02-21 09:27:07]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-02-21 09:27:22]

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

[commit-bot: I haz the power, 2014-02-21 09:27:30]

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

[commit-bot: I haz the power, 2014-02-21 09:27:31]

Failed to apply patch for Source/core/clipboard/DataObject.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/core/clipboard/DataObject.cpp
  Hunk #3 succeeded at 74 with fuzz 1.
  Hunk #4 FAILED at 93.
  Hunk #8 succeeded at 249 with fuzz 1.
  1 out of 9 hunks FAILED -- saving rejects to file
Source/core/clipboard/DataObject.cpp.rej

Patch:       Source/core/clipboard/DataObject.cpp
Index: Source/core/clipboard/DataObject.cpp
diff --git a/Source/core/clipboard/DataObject.cpp
b/Source/core/clipboard/DataObject.cpp
index
e3fb8a2addec70656c6c1c5048caf3b240ed4dd9..d1c1a895a7e9057008708c4bedc35d705e596c74
100644
--- a/Source/core/clipboard/DataObject.cpp
+++ b/Source/core/clipboard/DataObject.cpp
@@ -39,9 +39,11 @@
 
 namespace WebCore {
 
-PassRefPtr<DataObject> DataObject::createFromPasteboard(PasteMode pasteMode)
+DEFINE_GC_INFO(DataObject);
+
+PassRefPtrWillBeRawPtr<DataObject> DataObject::createFromPasteboard(PasteMode
pasteMode)
 {
-    RefPtr<DataObject> dataObject = create();
+    RefPtrWillBeRawPtr<DataObject> dataObject = create();
     blink::WebClipboard::Buffer buffer =
Pasteboard::generalPasteboard()->buffer();
     uint64_t sequenceNumber =
blink::Platform::current()->clipboard()->sequenceNumber(buffer);
     bool ignored;
@@ -57,14 +59,14 @@ PassRefPtr<DataObject>
DataObject::createFromPasteboard(PasteMode pasteMode)
     return dataObject.release();
 }
 
-PassRefPtr<DataObject> DataObject::create()
+PassRefPtrWillBeRawPtr<DataObject> DataObject::create()
 {
-    return adoptRef(new DataObject());
+    return adoptRefWillBeNoop(new DataObject());
 }
 
-PassRefPtr<DataObject> DataObject::copy() const
+PassRefPtrWillBeRawPtr<DataObject> DataObject::copy() const
 {
-    return adoptRef(new DataObject(*this));
+    return adoptRefWillBeNoop(new DataObject(*this));
 }
 
 size_t DataObject::length() const
@@ -72,7 +74,7 @@ size_t DataObject::length() const
     return m_itemList.size();
 }
 
-PassRefPtr<DataObjectItem> DataObject::item(unsigned long index)
+PassRefPtrWillBeRawPtr<DataObjectItem> DataObject::item(unsigned long index)
 {
     if (index >= length())
         return 0;
@@ -91,20 +93,20 @@ void DataObject::clearAll()
     m_itemList.clear();
 }
 
-PassRefPtr<DataObjectItem> DataObject::add(const String& data, const String&
type)
+PassRefPtrWillBeRawPtr<DataObjectItem> DataObject::add(const String& data,
const String& type)
 {
-    RefPtr<DataObjectItem> item = DataObjectItem::createFromString(type, data);
+    RefPtrWillBeRawPtr<DataObjectItem> item =
DataObjectItem::createFromString(type, data);
     if (!internalAddStringItem(item))
         return 0;
     return item;
 }
 
-PassRefPtr<DataObjectItem> DataObject::add(PassRefPtr<File> file)
+PassRefPtrWillBeRawPtr<DataObjectItem> DataObject::add(PassRefPtr<File> file)
 {
     if (!file)
         return 0;
 
-    RefPtr<DataObjectItem> item = DataObjectItem::createFromFile(file);
+    RefPtrWillBeRawPtr<DataObjectItem> item =
DataObjectItem::createFromFile(file);
     m_itemList.append(item);
     return item;
 }
@@ -169,7 +171,7 @@ bool DataObject::setData(const String& type, const String&
data)
 
 void DataObject::urlAndTitle(String& url, String* title) const
 {
-    RefPtr<DataObjectItem> item = findStringItem(mimeTypeTextURIList);
+    RefPtrWillBeRawPtr<DataObjectItem> item =
findStringItem(mimeTypeTextURIList);
     if (!item)
         return;
     url = convertURIListToURL(item->getAsString());
@@ -185,7 +187,7 @@ void DataObject::setURLAndTitle(const String& url, const
String& title)
 
 void DataObject::htmlAndBaseURL(String& html, KURL& baseURL) const
 {
-    RefPtr<DataObjectItem> item = findStringItem(mimeTypeTextHTML);
+    RefPtrWillBeRawPtr<DataObjectItem> item = findStringItem(mimeTypeTextHTML);
     if (!item)
         return;
     html = item->getAsString();
@@ -233,13 +235,12 @@ DataObject::DataObject()
 }
 
 DataObject::DataObject(const DataObject& other)
-    : RefCounted<DataObject>()
-    , m_itemList(other.m_itemList)
+    : m_itemList(other.m_itemList)
     , m_modifierKeyState(0)
 {
 }
 
-PassRefPtr<DataObjectItem> DataObject::findStringItem(const String& type) const
+PassRefPtrWillBeRawPtr<DataObjectItem> DataObject::findStringItem(const String&
type) const
 {
     for (size_t i = 0; i < m_itemList.size(); ++i) {
         if (m_itemList[i]->kind() == DataObjectItem::StringKind &&
m_itemList[i]->type() == type)
@@ -248,7 +249,7 @@ PassRefPtr<DataObjectItem> DataObject::findStringItem(const
String& type) const
     return 0;
 }
 
-bool DataObject::internalAddStringItem(PassRefPtr<DataObjectItem> item)
+bool DataObject::internalAddStringItem(PassRefPtrWillBeRawPtr<DataObjectItem>
item)
 {
     ASSERT(item->kind() == DataObjectItem::StringKind);
     for (size_t i = 0; i < m_itemList.size(); ++i) {
@@ -260,10 +261,15 @@ bool
DataObject::internalAddStringItem(PassRefPtr<DataObjectItem> item)
     return true;
 }
 
-void DataObject::internalAddFileItem(PassRefPtr<DataObjectItem> item)
+void DataObject::internalAddFileItem(PassRefPtrWillBeRawPtr<DataObjectItem>
item)
 {
     ASSERT(item->kind() == DataObjectItem::FileKind);
     m_itemList.append(item);
 }
 
+void DataObject::trace(Visitor* visitor)
+{
+    visitor->trace(m_itemList);
+}
+
 } // namespace WebCore

[haraken, 2014-02-21 09:42:00]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-02-21 09:42:15]

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

[commit-bot: I haz the power, 2014-02-21 10:41:49]

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

[commit-bot: I haz the power, 2014-02-21 10:41:50]

Retried try job too often on mac_layout for step(s) webkit_lint
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_layout...

[haraken, 2014-02-21 11:44:13]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-02-21 11:44:33]

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

[commit-bot: I haz the power, 2014-02-21 12:36:53]

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

[commit-bot: I haz the power, 2014-02-21 12:36:54]

Retried try job too often on mac_layout for step(s) webkit_lint
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_layout...

[haraken, 2014-02-21 13:02:48]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-02-21 13:02:57]

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

[commit-bot: I haz the power, 2014-02-21 13:56:59]

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

[commit-bot: I haz the power, 2014-02-21 13:57:00]

Retried try job too often on mac_layout for step(s) webkit_lint
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_layout...

[haraken, 2014-02-24 00:53:34]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-02-24 00:53:52]

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

[commit-bot: I haz the power, 2014-02-24 10:44:04]

Change committed as 167676