Pass DOMWinow to PerformanceLongTaskTiming for attribution

Pass DOMWinow to PerformanceLongTaskTiming for attribution
Handle cross-origin attribution.

BUG=635596
Committed: https://crrev.com/d4b3a10ae0b35dc3f40c966a139f30127c29c1b9
Cr-Commit-Position: refs/heads/master@{#423271}

[panicker, 2016-10-04 00:01:46]

panicker@chromium.org changed reviewers:
+ caseq@chromium.org, dgozman@chromium.org

[panicker, 2016-10-04 00:01:47]

PTAL

[panicker, 2016-10-04 00:02:29]

Description was changed from

==========
Pass DOMWinow to PerformanceLongTaskTiming for attribution

BUG=635596
==========

to

==========
Pass DOMWinow to PerformanceLongTaskTiming for attribution
Handle cross-origin attribution.

BUG=635596
==========

[caseq, 2016-10-05 00:22:18]

Mostly looks good -- let's just fix the JSON generation.

https://codereview.chromium.org/2386403002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp (right):

https://codereview.chromium.org/2386403002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp:23: static
const char unknownAttribution[] = "unknown";
kUnknownAttribution[] and ditto below.

https://codereview.chromium.org/2386403002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/timing/PerformanceLongTaskTiming.cpp
(right):

https://codereview.chromium.org/2386403002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/timing/PerformanceLongTaskTiming.cpp:27:
builder.add("culpritWindow", culpritWindow());
This looks suspicious to me -- we can't really serialize the window as a JSON.
Let's just remove this?

[panicker, 2016-10-05 01:02:18]

PTAL.
Dima, could you review for OWNERS ?

https://codereview.chromium.org/2386403002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp (right):

https://codereview.chromium.org/2386403002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp:23: static
const char unknownAttribution[] = "unknown";
On 2016/10/05 00:22:18, caseq wrote:
> kUnknownAttribution[] and ditto below.

Done.

https://codereview.chromium.org/2386403002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/timing/PerformanceLongTaskTiming.cpp
(right):

https://codereview.chromium.org/2386403002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/timing/PerformanceLongTaskTiming.cpp:27:
builder.add("culpritWindow", culpritWindow());
On 2016/10/05 00:22:18, caseq wrote:
> This looks suspicious to me -- we can't really serialize the window as a JSON.
> Let's just remove this?

Done.

[caseq, 2016-10-05 01:05:14]

lgtm

https://codereview.chromium.org/2386403002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/timing/PerformanceLongTaskTiming.cpp
(right):

https://codereview.chromium.org/2386403002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/timing/PerformanceLongTaskTiming.cpp:7: #include
"bindings/core/v8/V8ObjectBuilder.h"
unused now?

[dgozman, 2016-10-05 01:21:39]

lgtm

https://codereview.chromium.org/2386403002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/timing/PerformanceLongTaskTiming.h (right):

https://codereview.chromium.org/2386403002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/timing/PerformanceLongTaskTiming.h:41:
Member<DOMWindow> m_culpritWindow;
Are we concerned about this entry retaining DOMWindow? How long does it live? Do
other entries retain something heavy?

[panicker, 2016-10-05 04:11:31]

https://codereview.chromium.org/2386403002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/timing/PerformanceLongTaskTiming.cpp
(right):

https://codereview.chromium.org/2386403002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/timing/PerformanceLongTaskTiming.cpp:7: #include
"bindings/core/v8/V8ObjectBuilder.h"
On 2016/10/05 01:05:14, caseq wrote:
> unused now?

Done.

https://codereview.chromium.org/2386403002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/timing/PerformanceLongTaskTiming.h (right):

https://codereview.chromium.org/2386403002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/timing/PerformanceLongTaskTiming.h:41:
Member<DOMWindow> m_culpritWindow;
On 2016/10/05 01:21:39, dgozman wrote:
> Are we concerned about this entry retaining DOMWindow? How long does it live?
Do
> other entries retain something heavy?

This is a good question, and we did discuss it a fair bit.
Currently the performance implication is not significant, as there is no buffer
being maintained (see https://codereview.chromium.org/2395653002) )we deliver
the entries - right away, triggered by short periodic timer.
(Other Performance Entries like Resource Timing do maintain buffers, but they
don't typically retain anything heavy)

Secondly, this is likely not the final shape of the API, but we wanted to make
the DOMWindow available in the Origin Trial to 
give developers the best opportunity to identify the culprit frame.

Going forward (beyond Origin Trial limited to 0.03% of chrome page loads) we
should definitely measure and determine whether there is a non-trivial
performance implication, if there is interest in adding this to the spec.

[panicker, 2016-10-05 04:13:20]

The CQ bit was checked by panicker@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-05 04:13:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-05 04:16:31]

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

[commit-bot: I haz the power, 2016-10-05 04:16:32]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[panicker, 2016-10-05 04:24:54]

The CQ bit was checked by panicker@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-05 04:25:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-05 05:48:59]

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

[commit-bot: I haz the power, 2016-10-05 05:49:01]

Dry run: This issue passed the CQ dry run.

[panicker, 2016-10-05 14:58:24]

The CQ bit was checked by panicker@chromium.org

[panicker, 2016-10-05 14:58:24]

The patchset sent to the CQ was uploaded after l-g-t-m from caseq@chromium.org,
dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/2386403002/#ps100001
(title: "sync and rebase")

[commit-bot: I haz the power, 2016-10-05 14:58:52]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-05 17:37:58]

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

[commit-bot: I haz the power, 2016-10-05 17:38:00]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[panicker, 2016-10-05 18:42:31]

The CQ bit was checked by panicker@chromium.org

[commit-bot: I haz the power, 2016-10-05 18:42:56]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-05 20:19:02]

Description was changed from

==========
Pass DOMWinow to PerformanceLongTaskTiming for attribution
Handle cross-origin attribution.

BUG=635596
==========

to

==========
Pass DOMWinow to PerformanceLongTaskTiming for attribution
Handle cross-origin attribution.

BUG=635596
==========

[commit-bot: I haz the power, 2016-10-05 20:19:04]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-10-05 20:20:50]

Description was changed from

==========
Pass DOMWinow to PerformanceLongTaskTiming for attribution
Handle cross-origin attribution.

BUG=635596
==========

to

==========
Pass DOMWinow to PerformanceLongTaskTiming for attribution
Handle cross-origin attribution.

BUG=635596

Committed: https://crrev.com/d4b3a10ae0b35dc3f40c966a139f30127c29c1b9
Cr-Commit-Position: refs/heads/master@{#423271}
==========

[commit-bot: I haz the power, 2016-10-05 20:20:52]

Patchset 6 (id:??) landed as
https://crrev.com/d4b3a10ae0b35dc3f40c966a139f30127c29c1b9
Cr-Commit-Position: refs/heads/master@{#423271}