Enable Oilpan for core/timing/.

Enable Oilpan for core/timing/.

R=haraken
BUG=479467
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=194227

[sof, 2015-04-22 08:25:03]

sigbjornf@opera.com changed reviewers:
+ oilpan-reviews@chromium.org

[sof, 2015-04-22 08:25:04]

Please take a look.

[haraken, 2015-04-22 11:17:24]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org

[haraken, 2015-04-22 11:17:25]

LGTM

https://codereview.chromium.org/693483004/diff/20001/Source/core/timing/DOMWi...
File Source/core/timing/DOMWindowPerformance.h (right):

https://codereview.chromium.org/693483004/diff/20001/Source/core/timing/DOMWi...
Source/core/timing/DOMWindowPerformance.h:35: // TODO(sof): try to move this
direct reference and instead rely on frame().

Sounds reasonable!

https://codereview.chromium.org/693483004/diff/20001/Source/core/timing/Perfo...
File Source/core/timing/Performance.h (right):

https://codereview.chromium.org/693483004/diff/20001/Source/core/timing/Perfo...
Source/core/timing/Performance.h:58:
DEFINE_EVENT_TARGET_REFCOUNTING_WILL_BE_REMOVED(RefCountedGarbageCollected<Performance>);

Not related to this CL, this macro sounds a bit confusing. We can probably
invent a better named macro for
RefCountedGarbageCollectedEventTargetWithInlineData.

https://codereview.chromium.org/693483004/diff/20001/Source/core/timing/Perfo...
Source/core/timing/Performance.h:107: WeakMember<MemoryInfo> m_memoryInfo;

Why does this need to be weak? (c.f., You use Member<MemoryInfo> for
WorkerPerformance::m_memoryInfo.)

https://codereview.chromium.org/693483004/diff/20001/Source/core/timing/Perfo...
File Source/core/timing/PerformanceResourceTiming.cpp (right):

https://codereview.chromium.org/693483004/diff/20001/Source/core/timing/Perfo...
Source/core/timing/PerformanceResourceTiming.cpp:53: ,
m_requestingDocument(requestingDocument)

Is there any reason to move the member? (I'm ok with the change.)

[sof, 2015-04-22 11:41:36]

Thanks; this makes the following script-exposed objects be on the heap with
!ENABLE(OILPAN):

 - MemoryInfo
 - PerformanceEntry
 - PerformanceMark
 - PerformanceMeasure
 - PerformanceResourceTiming

There shouldn't be performance concerns with making that switch; agreed?

https://codereview.chromium.org/693483004/diff/20001/Source/core/timing/Perfo...
File Source/core/timing/Performance.h (right):

https://codereview.chromium.org/693483004/diff/20001/Source/core/timing/Perfo...
Source/core/timing/Performance.h:107: WeakMember<MemoryInfo> m_memoryInfo;
On 2015/04/22 11:17:25, haraken wrote:
> 
> Why does this need to be weak? (c.f., You use Member<MemoryInfo> for
> WorkerPerformance::m_memoryInfo.)

Yes, it should be consistent with that (and ConsoleMemory)'s; switched to
Member<> now.

However, I think using WeakMember<MemoryInfo> would be worth considering -- if
there are no other parties holding these MemoryInfo objects alive, letting go of
them sounds preferable.

https://codereview.chromium.org/693483004/diff/20001/Source/core/timing/Perfo...
File Source/core/timing/PerformanceResourceTiming.cpp (right):

https://codereview.chromium.org/693483004/diff/20001/Source/core/timing/Perfo...
Source/core/timing/PerformanceResourceTiming.cpp:53: ,
m_requestingDocument(requestingDocument)
On 2015/04/22 11:17:25, haraken wrote:
> 
> Is there any reason to move the member? (I'm ok with the change.)

No strong reason, but the class had a bunch of bool fields at the very end, so I
felt compelled not waste a few bytes.

[haraken, 2015-04-22 11:58:07]

On 2015/04/22 11:41:36, sof wrote:
> Thanks; this makes the following script-exposed objects be on the heap with
> !ENABLE(OILPAN):
> 
>  - MemoryInfo
>  - PerformanceEntry
>  - PerformanceMark
>  - PerformanceMeasure
>  - PerformanceResourceTiming
> 
> There shouldn't be performance concerns with making that switch; agreed?

Agreed; I don't think these objects are performance-sensitive.

[haraken, 2015-04-22 11:58:13]

https://codereview.chromium.org/693483004/diff/20001/Source/core/timing/Perfo...
File Source/core/timing/Performance.h (right):

https://codereview.chromium.org/693483004/diff/20001/Source/core/timing/Perfo...
Source/core/timing/Performance.h:107: WeakMember<MemoryInfo> m_memoryInfo;
On 2015/04/22 11:41:36, sof wrote:
> On 2015/04/22 11:17:25, haraken wrote:
> > 
> > Why does this need to be weak? (c.f., You use Member<MemoryInfo> for
> > WorkerPerformance::m_memoryInfo.)
> 
> Yes, it should be consistent with that (and ConsoleMemory)'s; switched to
> Member<> now.
> 
> However, I think using WeakMember<MemoryInfo> would be worth considering -- if
> there are no other parties holding these MemoryInfo objects alive, letting go
of
> them sounds preferable.

Either is fine with me :)

[sof, 2015-04-22 14:36:14]

If the Oilpan bots come through green also, proceed to land?

https://codereview.chromium.org/693483004/diff/20001/Source/core/timing/Perfo...
File Source/core/timing/Performance.h (right):

https://codereview.chromium.org/693483004/diff/20001/Source/core/timing/Perfo...
Source/core/timing/Performance.h:107: WeakMember<MemoryInfo> m_memoryInfo;
On 2015/04/22 11:58:13, haraken wrote:
> On 2015/04/22 11:41:36, sof wrote:
> > On 2015/04/22 11:17:25, haraken wrote:
> > > 
> > > Why does this need to be weak? (c.f., You use Member<MemoryInfo> for
> > > WorkerPerformance::m_memoryInfo.)
> > 
> > Yes, it should be consistent with that (and ConsoleMemory)'s; switched to
> > Member<> now.
> > 
> > However, I think using WeakMember<MemoryInfo> would be worth considering --
if
> > there are no other parties holding these MemoryInfo objects alive, letting
go
> of
> > them sounds preferable.
> 
> Either is fine with me :)

Created https://code.google.com/p/chromium/issues/detail?id=479740 for
weakifying.

[haraken, 2015-04-22 15:30:38]

> If the Oilpan bots come through green also, proceed to land?

I think it's ok.

[sof, 2015-04-22 17:58:09]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2015-04-22 17:58:09]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/693483004/#ps40001 (title:
"de-weakify Performance::m_memoryInfo")

[commit-bot: I haz the power, 2015-04-22 17:58:38]

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

[commit-bot: I haz the power, 2015-04-22 18:05:12]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=194227

[aboxhall, 2015-04-23 00:17:55]

Any chance this could have caused these test failures?
https://build.chromium.org/p/chromium.webkit/builders/Android%20Tests%20%28db...

[haraken, 2015-04-23 04:20:59]

On 2015/04/23 00:17:55, aboxhall wrote:
> Any chance this could have caused these test failures?
>
https://build.chromium.org/p/chromium.webkit/builders/Android%20Tests%20%28db...

I don't think it's related, but why do you think this CL is suspicios?