Support performance.now() in workers.

Support performance.now() in workers, behind a runtime flag.

performance.now() provides a high resolution monotonic clock. The time values start at an "origin time" instead of the typical UNIX epoch. For shared workers, the origin time is the time when the shared worker was created. For dedicated workers, the origin time is its document's navigationStart. That means performance.now() in the document and the dedicated worker should return values in the same range.

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

[abarth-chromium, 2013-06-12 23:11:41]

https://codereview.chromium.org/16434011/diff/2001/LayoutTests/http/tests/w3c...
File LayoutTests/http/tests/w3c/webperf/resources/worker.js (right):

https://codereview.chromium.org/16434011/diff/2001/LayoutTests/http/tests/w3c...
LayoutTests/http/tests/w3c/webperf/resources/worker.js:16:
postMessage(JSON.stringify(results));
You probably don't need to use JSON encoding here.  postMessage can structured
clone the objects for you.

https://codereview.chromium.org/16434011/diff/2001/Source/modules/performance...
File Source/modules/performance/WorkerContextPerformance.cpp (right):

https://codereview.chromium.org/16434011/diff/2001/Source/modules/performance...
Source/modules/performance/WorkerContextPerformance.cpp:42: : m_context(context)
Why do you need to hold m_context?

https://codereview.chromium.org/16434011/diff/2001/Source/modules/performance...
Source/modules/performance/WorkerContextPerformance.cpp:67: return
from(context)->performance();
You can just pass context in here.

https://codereview.chromium.org/16434011/diff/2001/Source/modules/performance...
File Source/modules/performance/WorkerContextPerformance.h (right):

https://codereview.chromium.org/16434011/diff/2001/Source/modules/performance...
Source/modules/performance/WorkerContextPerformance.h:49:
WorkerContextPerformance(ScriptExecutionContext*);
Please mark one-argument constructors "explicit"

https://codereview.chromium.org/16434011/diff/2001/Source/modules/performance...
File Source/modules/performance/WorkerPerformance.h (right):

https://codereview.chromium.org/16434011/diff/2001/Source/modules/performance...
Source/modules/performance/WorkerPerformance.h:48: using
RefCounted<WorkerPerformance>::deref;
These aren't needed.

https://codereview.chromium.org/16434011/diff/2001/Source/modules/performance...
Source/modules/performance/WorkerPerformance.h:55: WorkerContext* m_context;
Rather than holding a raw m_context pointer, this object should subclass
ContextDestructionObserver.  That way it won't have a pointer to a context that
has been freed.

https://codereview.chromium.org/16434011/diff/2001/Source/modules/performance...
File Source/modules/performance/WorkerPerformance.idl (right):

https://codereview.chromium.org/16434011/diff/2001/Source/modules/performance...
Source/modules/performance/WorkerPerformance.idl:31: interface WorkerPerformance
{
[
    NoInterfaceObject
]

I presume you don't want an interface object.

[abarth-chromium, 2013-06-12 23:13:46]

https://codereview.chromium.org/16434011/diff/2001/Source/modules/modules.gypi
File Source/modules/modules.gypi (right):

https://codereview.chromium.org/16434011/diff/2001/Source/modules/modules.gyp...
Source/modules/modules.gypi:406: 'performance/WorkerPerformance.h',
Will we be able to add the main-thread WebPerformance code to this module as
well?  My understanding is that at least some of it is tightly coupled with
FrameLoader and other core classes...

[James Simonsen, 2013-06-12 23:16:43]

https://codereview.chromium.org/16434011/diff/2001/Source/modules/modules.gypi
File Source/modules/modules.gypi (right):

https://codereview.chromium.org/16434011/diff/2001/Source/modules/modules.gyp...
Source/modules/modules.gypi:406: 'performance/WorkerPerformance.h',
On 2013/06/12 23:13:46, abarth wrote:
> Will we be able to add the main-thread WebPerformance code to this module as
> well?  My understanding is that at least some of it is tightly coupled with
> FrameLoader and other core classes...

Yes, that's the intent. The first thing I want to do is move all of the
page/Performance* files here.

Moving the stuff in loader/ will be harder as you say. I don't have a plan for
that right now.

[James Simonsen, 2013-06-13 00:01:36]

Thanks for taking the time to review this, Adam.

https://codereview.chromium.org/16434011/diff/2001/LayoutTests/http/tests/w3c...
File LayoutTests/http/tests/w3c/webperf/resources/worker.js (right):

https://codereview.chromium.org/16434011/diff/2001/LayoutTests/http/tests/w3c...
LayoutTests/http/tests/w3c/webperf/resources/worker.js:16:
postMessage(JSON.stringify(results));
On 2013/06/12 23:11:41, abarth wrote:
> You probably don't need to use JSON encoding here.  postMessage can structured
> clone the objects for you.

Oh nice. Thanks.

https://codereview.chromium.org/16434011/diff/2001/Source/modules/performance...
File Source/modules/performance/WorkerContextPerformance.cpp (right):

https://codereview.chromium.org/16434011/diff/2001/Source/modules/performance...
Source/modules/performance/WorkerContextPerformance.cpp:42: : m_context(context)
On 2013/06/12 23:11:41, abarth wrote:
> Why do you need to hold m_context?

Done.

https://codereview.chromium.org/16434011/diff/2001/Source/modules/performance...
Source/modules/performance/WorkerContextPerformance.cpp:67: return
from(context)->performance();
On 2013/06/12 23:11:41, abarth wrote:
> You can just pass context in here.

Yep. Thanks.

https://codereview.chromium.org/16434011/diff/2001/Source/modules/performance...
File Source/modules/performance/WorkerContextPerformance.h (right):

https://codereview.chromium.org/16434011/diff/2001/Source/modules/performance...
Source/modules/performance/WorkerContextPerformance.h:49:
WorkerContextPerformance(ScriptExecutionContext*);
On 2013/06/12 23:11:41, abarth wrote:
> Please mark one-argument constructors "explicit"

Done.

https://codereview.chromium.org/16434011/diff/2001/Source/modules/performance...
File Source/modules/performance/WorkerPerformance.h (right):

https://codereview.chromium.org/16434011/diff/2001/Source/modules/performance...
Source/modules/performance/WorkerPerformance.h:48: using
RefCounted<WorkerPerformance>::deref;
On 2013/06/12 23:11:41, abarth wrote:
> These aren't needed.

Done. Thanks.

https://codereview.chromium.org/16434011/diff/2001/Source/modules/performance...
Source/modules/performance/WorkerPerformance.h:55: WorkerContext* m_context;
On 2013/06/12 23:11:41, abarth wrote:
> Rather than holding a raw m_context pointer, this object should subclass
> ContextDestructionObserver.  That way it won't have a pointer to a context
that
> has been freed.

Done.

https://codereview.chromium.org/16434011/diff/2001/Source/modules/performance...
File Source/modules/performance/WorkerPerformance.idl (right):

https://codereview.chromium.org/16434011/diff/2001/Source/modules/performance...
Source/modules/performance/WorkerPerformance.idl:31: interface WorkerPerformance
{
On 2013/06/12 23:11:41, abarth wrote:
> [
>     NoInterfaceObject
> ]
> 
> I presume you don't want an interface object.

Done. Fixed the typo about this in the spec too.

[abarth-chromium, 2013-06-13 00:04:30]

I'm probably not the right person to review the worker-specific pieces of this
CL.  Do you have someone in mind for those?

[James Simonsen, 2013-06-13 00:09:37]

On 2013/06/13 00:04:30, abarth wrote:
> I'm probably not the right person to review the worker-specific pieces of this
> CL.  Do you have someone in mind for those?

Not really, I spent 10 minutes reading through the revision logs for
WebCore/workers and couldn't figure out who the owner is. atwilson owns the
Chromium side. Andrew, do you know who's a good reviewer for Blink worker
changes?

[Andrew T Wilson (Slow), 2013-06-13 08:12:16]

I made a pass through the worker changes and it LGTM. Other people who have
worked on workers in the past are levin@ and dimich@ (CC'd), although like me,
they haven't contributed to WK/Blink in quite some time.

[abarth-chromium, 2013-06-13 19:19:01]

LGTM

Feel free to land once you feel like you've gotten enough feedback from worker
experts.  :)

[levin, 2013-06-13 19:55:21]

Just a few comments.

https://codereview.chromium.org/16434011/diff/19001/Source/modules/performanc...
File Source/modules/performance/WorkerContextPerformance.cpp (right):

https://codereview.chromium.org/16434011/diff/19001/Source/modules/performanc...
Source/modules/performance/WorkerContextPerformance.cpp:41:
WorkerContextPerformance::WorkerContextPerformance()
How is it deleted?

https://codereview.chromium.org/16434011/diff/19001/Source/modules/performanc...
File Source/modules/performance/WorkerPerformance.h (right):

https://codereview.chromium.org/16434011/diff/19001/Source/modules/performanc...
Source/modules/performance/WorkerPerformance.h:43: class WorkerPerformance :
public RefCounted<WorkerPerformance>, public ContextDestructionObserver {
How does this use ContextDestructionObserver?

How is it deleted?

[abarth-chromium, 2013-06-13 20:18:19]

https://codereview.chromium.org/16434011/diff/19001/Source/modules/performanc...
File Source/modules/performance/WorkerContextPerformance.cpp (right):

https://codereview.chromium.org/16434011/diff/19001/Source/modules/performanc...
Source/modules/performance/WorkerContextPerformance.cpp:41:
WorkerContextPerformance::WorkerContextPerformance()
On 2013/06/13 19:55:22, levin wrote:
> How is it deleted?

class WorkerContextPerformance : public Supplement<ScriptExecutionContext>

That means the object is owned by the ScriptExecutionContext.  When the
ScriptExecutionContext destructs, it deletes this object.

https://codereview.chromium.org/16434011/diff/19001/Source/modules/performanc...
File Source/modules/performance/WorkerPerformance.h (right):

https://codereview.chromium.org/16434011/diff/19001/Source/modules/performanc...
Source/modules/performance/WorkerPerformance.h:43: class WorkerPerformance :
public RefCounted<WorkerPerformance>, public ContextDestructionObserver {
On 2013/06/13 19:55:22, levin wrote:
> How does this use ContextDestructionObserver?

The pointer to the ScriptExecutionContext is stored in the
ContextDestructionObserver.  That means the pointer is zeroed out automatically
when the ScriptExecutionContext dies.

> How is it deleted?

It's refcounted, which means it dies when the references from JavaScript and
from WorkerContextPerformance go away.

[levin, 2013-06-13 22:19:07]

lgtm

https://codereview.chromium.org/16434011/diff/19001/Source/modules/performanc...
File Source/modules/performance/WorkerContextPerformance.cpp (right):

https://codereview.chromium.org/16434011/diff/19001/Source/modules/performanc...
Source/modules/performance/WorkerContextPerformance.cpp:41:
WorkerContextPerformance::WorkerContextPerformance()
On 2013/06/13 20:18:19, abarth wrote:
> On 2013/06/13 19:55:22, levin wrote:
> > How is it deleted?
> 
> class WorkerContextPerformance : public Supplement<ScriptExecutionContext>
> 
> That means the object is owned by the ScriptExecutionContext.  When the
> ScriptExecutionContext destructs, it deletes this object.

I was looking for a destructor on Supplement and I missed that it has a HashMap
of OwnPtr.

https://codereview.chromium.org/16434011/diff/19001/Source/modules/performanc...
File Source/modules/performance/WorkerPerformance.h (right):

https://codereview.chromium.org/16434011/diff/19001/Source/modules/performanc...
Source/modules/performance/WorkerPerformance.h:43: class WorkerPerformance :
public RefCounted<WorkerPerformance>, public ContextDestructionObserver {
On 2013/06/13 20:18:19, abarth wrote:
> On 2013/06/13 19:55:22, levin wrote:
> > How does this use ContextDestructionObserver?
> 
> The pointer to the ScriptExecutionContext is stored in the
> ContextDestructionObserver.  That means the pointer is zeroed out
automatically
> when the ScriptExecutionContext dies.

ok.

Somehow I missed the usage of sec completely.... I feel like I missed a lot
here. But thanks for your patience in helping with the basics :).

[abarth-chromium, 2013-06-13 22:27:16]

https://codereview.chromium.org/16434011/diff/19001/Source/modules/performanc...
File Source/modules/performance/WorkerPerformance.cpp (right):

https://codereview.chromium.org/16434011/diff/19001/Source/modules/performanc...
Source/modules/performance/WorkerPerformance.cpp:55: return 1000.0 *
(monotonicallyIncreasingTime() - workerContext->timeOrigin());
I missed it the first time too, but it's here on this line.

[James Simonsen, 2013-06-13 22:38:18]

Thanks for the reviews everyone.

Dave raised some good points about how the time origin is inconsistent between
shared workers and dedicated workers. I think I'll land this as is (matching the
spec) and let devs play with it. Meanwhile, I'll start a thread on
public-webperf and see if others think we should make time origin always be from
worker creation time.

[commit-bot: I haz the power, 2013-06-13 22:38:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonjam@chromium.org/16434011/19001

[commit-bot: I haz the power, 2013-06-14 00:29:33]

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

[commit-bot: I haz the power, 2013-06-14 00:31:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonjam@chromium.org/16434011/19001

[commit-bot: I haz the power, 2013-06-14 01:40:15]

Change committed as 152396