Move performance module/ to oilpan

Move performance module/ to oilpan

Nothing complicated. This CL just moves performance module/ to oilpan
using transition types.

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

[haraken, 2014-02-07 04:47:33]

PTAL

[tkent, 2014-02-07 05:08:11]

lgtm

[haraken, 2014-02-07 05:45:46]

The CQ bit was checked by haraken@chromium.org

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

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

[haraken, 2014-02-07 05:49:46]

https://codereview.chromium.org/157213002/diff/40001/Source/modules/performan...
File Source/modules/performance/WorkerPerformance.h (right):

https://codereview.chromium.org/157213002/diff/40001/Source/modules/performan...
Source/modules/performance/WorkerPerformance.h:44: class WorkerPerformance :
public ScriptWrappable, public
RefCountedWillBeGarbageCollectedFinalized<WorkerPerformance> {

Mads: It's a shame that we have to use GarbageCollected"Finalized" for objects
that inherit from ScriptWrappable, because most DOM objects inherit from
ScriptWrappable.

ScriptWrappable's destructor does nothing in terms of object lifetime, but the
destructor flips a bit of a ScriptWrappable's member for preventing security
exploit. (See
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...)

Do you have any idea to remove ScriptWrappable's destructor?

[haraken, 2014-02-07 05:56:50]

https://codereview.chromium.org/157213002/diff/40001/Source/modules/performan...
File Source/modules/performance/WorkerPerformance.h (right):

https://codereview.chromium.org/157213002/diff/40001/Source/modules/performan...
Source/modules/performance/WorkerPerformance.h:44: class WorkerPerformance :
public ScriptWrappable, public
RefCountedWillBeGarbageCollectedFinalized<WorkerPerformance> {
On 2014/02/07 05:49:46, haraken wrote:
> 
> Mads: It's a shame that we have to use GarbageCollected"Finalized" for objects
> that inherit from ScriptWrappable, because most DOM objects inherit from
> ScriptWrappable.
> 
> ScriptWrappable's destructor does nothing in terms of object lifetime, but the
> destructor flips a bit of a ScriptWrappable's member for preventing security
> exploit. (See
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...)
> 
> Do you have any idea to remove ScriptWrappable's destructor?

Also I noticed that RefCountedWillBeGarbageCollectedFinalized must be the
left-most class of WorkerPerformance. I'd be happy if Ian's plugin will catch
this kind of error :)

[haraken, 2014-02-07 05:57:32]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-02-07 05:57:41]

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

[zerny-chromium, 2014-02-07 05:58:04]

lgtm with a nit/request

https://codereview.chromium.org/157213002/diff/40001/Source/modules/performan...
File Source/modules/performance/WorkerPerformance.h (right):

https://codereview.chromium.org/157213002/diff/40001/Source/modules/performan...
Source/modules/performance/WorkerPerformance.h:44: class WorkerPerformance :
public ScriptWrappable, public
RefCountedWillBeGarbageCollectedFinalized<WorkerPerformance> {
On 2014/02/07 05:49:46, haraken wrote:
> 
> Mads: It's a shame that we have to use GarbageCollected"Finalized" for objects
> that inherit from ScriptWrappable, because most DOM objects inherit from
> ScriptWrappable.
> 
> ScriptWrappable's destructor does nothing in terms of object lifetime, but the
> destructor flips a bit of a ScriptWrappable's member for preventing security
> exploit. (See
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...)
> 
> Do you have any idea to remove ScriptWrappable's destructor?

I'll let Mads answer that, but on the branch we actually use GarbageCollected<T>
for a few classes that don't have any destruction code besides that of
ScriptWrappable (eg, NamedNodeMap, WebKitCSSMatrix, etc.).

On another note, I'd swap the order of ScriptWrappable and
RefCountedGarbageCollected. Once it becomes GarbageCollected it should be
left-most.

[zerny-chromium, 2014-02-07 05:59:43]

On 2014/02/07 05:56:50, haraken wrote:
>
https://codereview.chromium.org/157213002/diff/40001/Source/modules/performan...
> File Source/modules/performance/WorkerPerformance.h (right):
> 
>
https://codereview.chromium.org/157213002/diff/40001/Source/modules/performan...
> Source/modules/performance/WorkerPerformance.h:44: class WorkerPerformance :
> public ScriptWrappable, public
> RefCountedWillBeGarbageCollectedFinalized<WorkerPerformance> {
> On 2014/02/07 05:49:46, haraken wrote:
> > 
> > Mads: It's a shame that we have to use GarbageCollected"Finalized" for
objects
> > that inherit from ScriptWrappable, because most DOM objects inherit from
> > ScriptWrappable.
> > 
> > ScriptWrappable's destructor does nothing in terms of object lifetime, but
the
> > destructor flips a bit of a ScriptWrappable's member for preventing security
> > exploit. (See
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...)
> > 
> > Do you have any idea to remove ScriptWrappable's destructor?
> 
> Also I noticed that RefCountedWillBeGarbageCollectedFinalized must be the
> left-most class of WorkerPerformance. I'd be happy if Ian's plugin will catch
> this kind of error :)

Yes. That is on the todo list. We want to check that either that is the case or
that the class implements our start-of-object/adjust pattern.

[haraken, 2014-02-07 06:02:51]

> > Do you have any idea to remove ScriptWrappable's destructor?
> 
> I'll let Mads answer that, but on the branch we actually use
GarbageCollected<T>
> for a few classes that don't have any destruction code besides that of
> ScriptWrappable (eg, NamedNodeMap, WebKitCSSMatrix, etc.).

Yeah, not calling ScriptWrappable's destructor won't cause any practical issue.
It just weakens the security of the binding layer. (So we should call
ScriptWrappable's destructor.)

[tkent, 2014-02-07 06:07:55]

https://codereview.chromium.org/157213002/diff/40001/Source/modules/performan...
File Source/modules/performance/WorkerPerformance.h (right):

https://codereview.chromium.org/157213002/diff/40001/Source/modules/performan...
Source/modules/performance/WorkerPerformance.h:44: class WorkerPerformance :
public ScriptWrappable, public
RefCountedWillBeGarbageCollectedFinalized<WorkerPerformance> {
On 2014/02/07 05:56:50, haraken wrote:
> On 2014/02/07 05:49:46, haraken wrote:
> > 
> > Mads: It's a shame that we have to use GarbageCollected"Finalized" for
objects
> > that inherit from ScriptWrappable, because most DOM objects inherit from
> > ScriptWrappable.
> > 
> > ScriptWrappable's destructor does nothing in terms of object lifetime, but
the
> > destructor flips a bit of a ScriptWrappable's member for preventing security
> > exploit. (See
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...)
> > 
> > Do you have any idea to remove ScriptWrappable's destructor?
> 
> Also I noticed that RefCountedWillBeGarbageCollectedFinalized must be the
> left-most class of WorkerPerformance. I'd be happy if Ian's plugin will catch
> this kind of error :)

I didn't know this rule.  Do we have a documentation about it?

[haraken, 2014-02-07 06:18:55]

> > Also I noticed that RefCountedWillBeGarbageCollectedFinalized must be the
> > left-most class of WorkerPerformance. I'd be happy if Ian's plugin will
catch
> > this kind of error :)
> 
> I didn't know this rule.  Do we have a documentation about it?

No documentation yet but the situation is as follows: When GC scans the heap,
the GarbageCollected(Finalized) class treats the beginning address of the
GarbageCollected object as the head address of the object. Thus we have to make
sure that GarbageCollected is written as the left-most class.

Multiple inheritance makes the situation more complicated, because if we have
multiple inheritance, we might inherit multiple classes each of which inherits
GarbageCollected. Then we need to fix the head address of |this| pointer. See
the "adjustAndMark" method in HTMLFormControlElement in this CL
(https://codereview.chromium.org/106423003/).

[zerny-chromium, 2014-02-07 06:24:00]

> I didn't know this rule.  Do we have a documentation about it?

W have not written that out yet. For the time being, you can find an example in
the oilpan branchs FormAssociatedElement [1].

Here FormAssociatedElement is used as a mix-in to GC'ed objects so it is not in
the left-most position. We specialize TraceTrait to call the virtual
adjustAndMark (instead of just mark) which will adjust the this pointer to
properly point at the start of object (which is GC'ed). We need this in order to
find the mark bit which is stored just prior to the object itself.

[1]
http://src.chromium.org/viewvc/blink/branches/oilpan/Source/core/html/FormAss...

[commit-bot: I haz the power, 2014-02-07 08:07:24]

Change committed as 166691

[Mads Ager (chromium), 2014-02-07 09:28:33]

https://codereview.chromium.org/157213002/diff/40001/Source/modules/performan...
File Source/modules/performance/WorkerPerformance.h (right):

https://codereview.chromium.org/157213002/diff/40001/Source/modules/performan...
Source/modules/performance/WorkerPerformance.h:44: class WorkerPerformance :
public ScriptWrappable, public
RefCountedWillBeGarbageCollectedFinalized<WorkerPerformance> {
On 2014/02/07 05:49:46, haraken wrote:
> 
> Mads: It's a shame that we have to use GarbageCollected"Finalized" for objects
> that inherit from ScriptWrappable, because most DOM objects inherit from
> ScriptWrappable.
> 
> ScriptWrappable's destructor does nothing in terms of object lifetime, but the
> destructor flips a bit of a ScriptWrappable's member for preventing security
> exploit. (See
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...)
> 
> Do you have any idea to remove ScriptWrappable's destructor?

That is indeed a shame. We should see if we can come up with something that can
provide the same security guarantees but does not require us to run the
destructor. I don't see how to do that simply at this point. Let's keep it in
mind and see if we can come up with idea.