[worklets] Introduce ThreadedWorkletMessagingProxy and AnimationWorkletMessagaingProxy.

[worklets] Introduce ThreadedWorkletMessagingProxy and AnimationWorkletMessagaingProxy.

This patch does the plumbing from the animationworklet to the globalscope to import scripts etc.

BUG=567358
Committed: https://crrev.com/4bd1a1b4e1d321526cfe0c10adc15f507a661bb8
Committed: https://crrev.com/a49dbd327c02c506b07bc9c024e2492b34a4c779
Cr-Original-Commit-Position: refs/heads/master@{#418765}
Cr-Commit-Position: refs/heads/master@{#422192}

[ikilpatrick, 2016-09-07 23:30:34]

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

[commit-bot: I haz the power, 2016-09-07 23:31:17]

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-09-07 23:36:45]

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

[commit-bot: I haz the power, 2016-09-07 23:36:46]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[ikilpatrick, 2016-09-08 22:55:58]

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

[commit-bot: I haz the power, 2016-09-08 22:56:44]

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-09-08 23:01:15]

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

[commit-bot: I haz the power, 2016-09-08 23:01:16]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_chromium_asan_rel_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_...)

[ikilpatrick, 2016-09-08 23:10:07]

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

[ikilpatrick, 2016-09-08 23:10:34]

Patchset #3 (id:40001) has been deleted

[commit-bot: I haz the power, 2016-09-08 23:10:41]

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

[ikilpatrick, 2016-09-08 23:10:44]

Patchset #2 (id:20001) has been deleted

[commit-bot: I haz the power, 2016-09-08 23:19:13]

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

[commit-bot: I haz the power, 2016-09-08 23:19:14]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_compile_dbg_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_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[ikilpatrick, 2016-09-09 00:14:11]

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

[commit-bot: I haz the power, 2016-09-09 00:14:35]

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-09-09 00:19:14]

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

[commit-bot: I haz the power, 2016-09-09 00:19:15]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  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_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[ikilpatrick, 2016-09-09 03:55:19]

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

[commit-bot: I haz the power, 2016-09-09 03:55:31]

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

[ikilpatrick, 2016-09-09 03:57:04]

Description was changed from

==========
animation messaging proxy.

BUG=
==========

to

==========
[worklets] Introduce ThreadedWorkletMessagingProxy and
AnimationWorkletMessagaingProxy.

This patch does the plumbing from the animationworklet to the globalscope to
import scripts etc.

BUG=567358
==========

[ikilpatrick, 2016-09-09 03:57:05]

ikilpatrick@chromium.org changed reviewers:
+ hongchan@chromium.org, nhiroki@chromium.org, yhirano@chromium.org

[commit-bot: I haz the power, 2016-09-09 05:22:21]

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

[commit-bot: I haz the power, 2016-09-09 05:22:22]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[hongchan, 2016-09-09 17:45:30]

https://codereview.chromium.org/2312493002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioWorklet.cpp (left):

https://codereview.chromium.org/2312493002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorklet.cpp:9: #include
"core/workers/ThreadedWorkletGlobalScopeProxy.h"
I am not sure what this change means. Can you elaborate? So
AudioWorkletGlobalScope does not need to a proxy? Why?

[ikilpatrick, 2016-09-09 22:16:59]

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

[commit-bot: I haz the power, 2016-09-09 22:17:28]

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-09-09 22:19:44]

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

[commit-bot: I haz the power, 2016-09-09 22:19:45]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[ikilpatrick, 2016-09-09 23:24:14]

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

[commit-bot: I haz the power, 2016-09-09 23:24:41]

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-09-10 04:13:04]

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

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

Dry run: This issue passed the CQ dry run.

[nhiroki, 2016-09-12 03:37:06]

lgtm!

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/http/tests/worklet/resources/import-tests.js
(right):

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/worklet/resources/import-tests.js:1:
// Runs a series of tests related to importing scripts on a worklet.
Nice abstraction :)

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/workers/ThreadedWorkletMessagingProxy.h
(right):

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/ThreadedWorkletMessagingProxy.h:26:
ThreadedWorkletMessagingProxy(ExecutionContext*);
explicit

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/ThreadedWorkletMessagingProxy.h:29:
private:
nit: Can you add blank lines before "protected:" and "private"?

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/modules/compositorworker/AnimationWorkletMessagingProxy.h
(right):

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/compositorworker/AnimationWorkletMessagingProxy.h:20:
AnimationWorkletMessagingProxy(ExecutionContext*);
explicit

[yhirano, 2016-09-12 09:02:59]

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/workers/ThreadedWorkletMessagingProxy.h
(right):

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/ThreadedWorkletMessagingProxy.h:24: void
initialize();
+blank line

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/ThreadedWorkletMessagingProxy.h:28:
ThreadedWorkletObjectProxy& workletObjectProxy() { return
*m_workletObjectProxy.get(); }
No get is needed.

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/workers/ThreadedWorkletObjectProxy.cpp
(right):

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/ThreadedWorkletObjectProxy.cpp:39: if
(context->isDocument())
Is there any chance to get a non-Document ExecutionContext here?

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/compositorworker/AnimationWorklet.cpp
(right):

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/compositorworker/AnimationWorklet.cpp:33:
DCHECK(m_workletMessagingProxy);
Is it good to move this check to the constructor and make m_workletMessageProxy
a const pointer (i.e., ThreadedWorkletMessagingProxy* const
m_workletMessagingProxy)?

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/compositorworker/AnimationWorklet.h
(right):

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/compositorworker/AnimationWorklet.h:31:
ThreadedWorkletMessagingProxy* m_workletMessagingProxy;
Can you add comments describing why having this raw ptr is safe?

[ikilpatrick, 2016-09-12 17:27:11]

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

[commit-bot: I haz the power, 2016-09-12 17:27:44]

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

[ikilpatrick, 2016-09-12 17:29:07]

https://codereview.chromium.org/2312493002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioWorklet.cpp (left):

https://codereview.chromium.org/2312493002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioWorklet.cpp:9: #include
"core/workers/ThreadedWorkletGlobalScopeProxy.h"
On 2016/09/09 17:45:30, hoch wrote:
> I am not sure what this change means. Can you elaborate? So
> AudioWorkletGlobalScope does not need to a proxy? Why?

I've reverted these changes. Basically it needs a proxy; but I was going to
remove ThreadedWorkletGlobalScopeProxy.h in this patch.

I've now added a TODO at the top of ThreadedWorkletGlobalScopeProxy to remove
that file :).

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/workers/ThreadedWorkletMessagingProxy.h
(right):

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/ThreadedWorkletMessagingProxy.h:24: void
initialize();
On 2016/09/12 09:02:58, yhirano (slow) wrote:
> +blank line

Done.

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/ThreadedWorkletMessagingProxy.h:26:
ThreadedWorkletMessagingProxy(ExecutionContext*);
On 2016/09/12 03:37:06, nhiroki wrote:
> explicit

Done.

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/ThreadedWorkletMessagingProxy.h:28:
ThreadedWorkletObjectProxy& workletObjectProxy() { return
*m_workletObjectProxy.get(); }
On 2016/09/12 09:02:58, yhirano (slow) wrote:
> No get is needed.

Done.

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/ThreadedWorkletMessagingProxy.h:29:
private:
On 2016/09/12 03:37:06, nhiroki wrote:
> nit: Can you add blank lines before "protected:" and "private"?

Done.

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/workers/ThreadedWorkletObjectProxy.cpp
(right):

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/ThreadedWorkletObjectProxy.cpp:39: if
(context->isDocument())
On 2016/09/12 09:02:58, yhirano (slow) wrote:
> Is there any chance to get a non-Document ExecutionContext here?

No, removed.

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/compositorworker/AnimationWorklet.cpp
(right):

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/compositorworker/AnimationWorklet.cpp:33:
DCHECK(m_workletMessagingProxy);
On 2016/09/12 09:02:58, yhirano (slow) wrote:
> Is it good to move this check to the constructor and make
m_workletMessageProxy
> a const pointer (i.e., ThreadedWorkletMessagingProxy* const
> m_workletMessagingProxy)?

sgtm, I've just removed the DCHECK now that it's a const pointer (as gets
created & used right above).

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/modules/compositorworker/AnimationWorkletMessagingProxy.h
(right):

https://codereview.chromium.org/2312493002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/compositorworker/AnimationWorkletMessagingProxy.h:20:
AnimationWorkletMessagingProxy(ExecutionContext*);
On 2016/09/12 03:37:06, nhiroki wrote:
> explicit

Done.

[ikilpatrick, 2016-09-12 17:29:25]

ikilpatrick@chromium.org changed reviewers:
+ majidvp@chromium.org

[ikilpatrick, 2016-09-12 17:29:36]

+majid FYI

[commit-bot: I haz the power, 2016-09-12 19:33:26]

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

[commit-bot: I haz the power, 2016-09-12 19:33:27]

Dry run: This issue passed the CQ dry run.

[ikilpatrick, 2016-09-14 18:49:40]

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

[commit-bot: I haz the power, 2016-09-14 18:50:30]

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

[majidvp, 2016-09-14 20:06:31]

lgtm with nits

https://codereview.chromium.org/2312493002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/workers/ThreadedWorkletObjectProxy.cpp
(right):

https://codereview.chromium.org/2312493002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/ThreadedWorkletObjectProxy.cpp:38:
toDocument(getExecutionContext())->postInspectorTask(BLINK_FROM_HERE,
createCrossThreadTask(&::blink::ThreadedWorkletMessagingProxy::postMessageToPageInspector,
crossThreadUnretained(m_messagingProxy), message));
nit: Are we confident this will always be a Document? Perhaps worth a DCHECK on
|getExecutionContext()->isDocument()| to document this.

https://codereview.chromium.org/2312493002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/workers/ThreadedWorkletObjectProxy.h
(right):

https://codereview.chromium.org/2312493002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/ThreadedWorkletObjectProxy.h:14: #include
"wtf/PassRefPtr.h"
nit: Timer.h, Handle.h and PassRefPtr.h are unused. Please remove.

https://codereview.chromium.org/2312493002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/ThreadedWorkletObjectProxy.h:22: class
WorkerOrWorkletGlobalScope;
nit: unused.

[ikilpatrick, 2016-09-14 20:52:58]

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

[commit-bot: I haz the power, 2016-09-14 20:53:23]

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

[ikilpatrick, 2016-09-14 21:54:52]

https://codereview.chromium.org/2312493002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/workers/ThreadedWorkletObjectProxy.cpp
(right):

https://codereview.chromium.org/2312493002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/ThreadedWorkletObjectProxy.cpp:38:
toDocument(getExecutionContext())->postInspectorTask(BLINK_FROM_HERE,
createCrossThreadTask(&::blink::ThreadedWorkletMessagingProxy::postMessageToPageInspector,
crossThreadUnretained(m_messagingProxy), message));
On 2016/09/14 20:06:31, majidvp wrote:
> nit: Are we confident this will always be a Document? Perhaps worth a DCHECK
on
> |getExecutionContext()->isDocument()| to document this.

Done.

https://codereview.chromium.org/2312493002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/workers/ThreadedWorkletObjectProxy.h
(right):

https://codereview.chromium.org/2312493002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/ThreadedWorkletObjectProxy.h:14: #include
"wtf/PassRefPtr.h"
On 2016/09/14 20:06:31, majidvp wrote:
> nit: Timer.h, Handle.h and PassRefPtr.h are unused. Please remove.

Done.

https://codereview.chromium.org/2312493002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/ThreadedWorkletObjectProxy.h:22: class
WorkerOrWorkletGlobalScope;
On 2016/09/14 20:06:31, majidvp wrote:
> nit: unused.

Done.

[commit-bot: I haz the power, 2016-09-15 00:59:27]

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

[commit-bot: I haz the power, 2016-09-15 00:59:28]

Dry run: 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_...)

[yhirano, 2016-09-15 01:03:25]

lgtm

[ikilpatrick, 2016-09-15 01:15:48]

The CQ bit was checked by ikilpatrick@chromium.org

[ikilpatrick, 2016-09-15 01:15:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
nhiroki@chromium.org, majidvp@chromium.org
Link to the patchset: https://codereview.chromium.org/2312493002/#ps180001
(title: "address comments.")

[commit-bot: I haz the power, 2016-09-15 01:16:19]

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-09-15 02:34:59]

Description was changed from

==========
[worklets] Introduce ThreadedWorkletMessagingProxy and
AnimationWorkletMessagaingProxy.

This patch does the plumbing from the animationworklet to the globalscope to
import scripts etc.

BUG=567358
==========

to

==========
[worklets] Introduce ThreadedWorkletMessagingProxy and
AnimationWorkletMessagaingProxy.

This patch does the plumbing from the animationworklet to the globalscope to
import scripts etc.

BUG=567358
==========

[commit-bot: I haz the power, 2016-09-15 02:35:01]

Committed patchset #8 (id:180001)

[commit-bot: I haz the power, 2016-09-15 02:37:53]

Description was changed from

==========
[worklets] Introduce ThreadedWorkletMessagingProxy and
AnimationWorkletMessagaingProxy.

This patch does the plumbing from the animationworklet to the globalscope to
import scripts etc.

BUG=567358
==========

to

==========
[worklets] Introduce ThreadedWorkletMessagingProxy and
AnimationWorkletMessagaingProxy.

This patch does the plumbing from the animationworklet to the globalscope to
import scripts etc.

BUG=567358

Committed: https://crrev.com/4bd1a1b4e1d321526cfe0c10adc15f507a661bb8
Cr-Commit-Position: refs/heads/master@{#418765}
==========

[commit-bot: I haz the power, 2016-09-15 02:37:55]

Patchset 8 (id:??) landed as
https://crrev.com/4bd1a1b4e1d321526cfe0c10adc15f507a661bb8
Cr-Commit-Position: refs/heads/master@{#418765}

[Yuta Kitamura, 2016-09-15 05:55:12]

Hey ikilpatrick & reviewers,

I've determined this commit has caused leaks in 18 layout tests:

Regressions: Unexpected leaks (18)
 
fast/dom/Window/property-access-on-cached-properties-after-frame-navigated.html
[ Leak ]
 
fast/dom/Window/property-access-on-cached-properties-after-frame-removed-and-gced.html
[ Leak ]
  fast/dom/Window/property-access-on-cached-properties-after-frame-removed.html
[ Leak ]
  fast/dom/Window/property-access-on-cached-window-after-frame-navigated.html [
Leak ]
 
fast/dom/Window/property-access-on-cached-window-after-frame-removed-and-gced.html
[ Leak ]
  fast/dom/Window/property-access-on-cached-window-after-frame-removed.html [
Leak ]
  fast/dom/constructed-objects-prototypes.html [ Leak ]
  fast/dom/everything-to-string.html [ Leak ]
  fast/js/global-constructors.html [ Leak ]
  http/tests/security/isolatedWorld/all-window-properties.html [ Leak ]
  http/tests/security/isolatedWorld/all-window-prototypes.html [ Leak ]
  http/tests/worklet/animation-worklet-import.html [ Leak ]
  inspector/console/console-dir-deprecated.html [ Leak ]
  inspector/console/console-dir-global.html [ Leak Leak Leak Timeout ]
  inspector/sources/debugger-ui/function-details.html [ Leak ]
  inspector/sources/debugger/debugger-completions-on-call-frame.html [ Leak ]
  virtual/mojo-loading/webexposed/global-interface-listing.html [ Leak ]
  webexposed/global-interface-listing.html [ Leak ]

I think we should revert this for now, as the list of failed tests
looks a little bit scary. WDYT?

[nhiroki, 2016-09-15 06:10:28]

On 2016/09/15 05:55:12, Yuta Kitamura wrote:
> Hey ikilpatrick & reviewers,
> 
> I've determined this commit has caused leaks in 18 layout tests:
> 
> Regressions: Unexpected leaks (18)
>  
>
fast/dom/Window/property-access-on-cached-properties-after-frame-navigated.html
> [ Leak ]
>  
>
fast/dom/Window/property-access-on-cached-properties-after-frame-removed-and-gced.html
> [ Leak ]
>  
fast/dom/Window/property-access-on-cached-properties-after-frame-removed.html
> [ Leak ]
>   fast/dom/Window/property-access-on-cached-window-after-frame-navigated.html
[
> Leak ]
>  
>
fast/dom/Window/property-access-on-cached-window-after-frame-removed-and-gced.html
> [ Leak ]
>   fast/dom/Window/property-access-on-cached-window-after-frame-removed.html [
> Leak ]
>   fast/dom/constructed-objects-prototypes.html [ Leak ]
>   fast/dom/everything-to-string.html [ Leak ]
>   fast/js/global-constructors.html [ Leak ]
>   http/tests/security/isolatedWorld/all-window-properties.html [ Leak ]
>   http/tests/security/isolatedWorld/all-window-prototypes.html [ Leak ]
>   http/tests/worklet/animation-worklet-import.html [ Leak ]
>   inspector/console/console-dir-deprecated.html [ Leak ]
>   inspector/console/console-dir-global.html [ Leak Leak Leak Timeout ]
>   inspector/sources/debugger-ui/function-details.html [ Leak ]
>   inspector/sources/debugger/debugger-completions-on-call-frame.html [ Leak ]
>   virtual/mojo-loading/webexposed/global-interface-listing.html [ Leak ]
>   webexposed/global-interface-listing.html [ Leak ]
> 
> I think we should revert this for now, as the list of failed tests
> looks a little bit scary. WDYT?

I'm fine with reverting this.

[Yuta Kitamura, 2016-09-15 07:38:42]

On 2016/09/15 06:10:28, nhiroki wrote:
> On 2016/09/15 05:55:12, Yuta Kitamura wrote:
> > Hey ikilpatrick & reviewers,
> > 
> > I've determined this commit has caused leaks in 18 layout tests:
> > 
> > Regressions: Unexpected leaks (18)
> >  
> >
>
fast/dom/Window/property-access-on-cached-properties-after-frame-navigated.html
> > [ Leak ]
> >  
> >
>
fast/dom/Window/property-access-on-cached-properties-after-frame-removed-and-gced.html
> > [ Leak ]
> >  
> fast/dom/Window/property-access-on-cached-properties-after-frame-removed.html
> > [ Leak ]
> >  
fast/dom/Window/property-access-on-cached-window-after-frame-navigated.html
> [
> > Leak ]
> >  
> >
>
fast/dom/Window/property-access-on-cached-window-after-frame-removed-and-gced.html
> > [ Leak ]
> >   fast/dom/Window/property-access-on-cached-window-after-frame-removed.html
[
> > Leak ]
> >   fast/dom/constructed-objects-prototypes.html [ Leak ]
> >   fast/dom/everything-to-string.html [ Leak ]
> >   fast/js/global-constructors.html [ Leak ]
> >   http/tests/security/isolatedWorld/all-window-properties.html [ Leak ]
> >   http/tests/security/isolatedWorld/all-window-prototypes.html [ Leak ]
> >   http/tests/worklet/animation-worklet-import.html [ Leak ]
> >   inspector/console/console-dir-deprecated.html [ Leak ]
> >   inspector/console/console-dir-global.html [ Leak Leak Leak Timeout ]
> >   inspector/sources/debugger-ui/function-details.html [ Leak ]
> >   inspector/sources/debugger/debugger-completions-on-call-frame.html [ Leak
]
> >   virtual/mojo-loading/webexposed/global-interface-listing.html [ Leak ]
> >   webexposed/global-interface-listing.html [ Leak ]
> > 
> > I think we should revert this for now, as the list of failed tests
> > looks a little bit scary. WDYT?
> 
> I'm fine with reverting this.

Alright, I'm going to revert this.

[Yuta Kitamura, 2016-09-15 07:39:39]

A revert of this CL (patchset #8 id:180001) has been created in
https://codereview.chromium.org/2342013002/ by yutak@chromium.org.

The reason for reverting is: Caused leaks in a number of layout tests.

https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Leak/b....

[Yuta Kitamura, 2016-09-15 08:41:11]

On 2016/09/15 07:39:39, Yuta Kitamura wrote:
> A revert of this CL (patchset #8 id:180001) has been created in
> https://codereview.chromium.org/2342013002/ by mailto:yutak@chromium.org.
> 
> The reason for reverting is: Caused leaks in a number of layout tests.
> 
>
https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Leak/b....

Also this commit is suspected to be causing layout test flakiness
in a few inspector tests:

https://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=in...

I'm not 100% sure yet because there's not much data after the revert, but
at least the flakiness of timeouts and leaks started at almost the same
time of this commit.

[Yuta Kitamura, 2016-09-15 10:17:13]

On 2016/09/15 08:41:11, Yuta Kitamura wrote:
> Also this commit is suspected to be causing layout test flakiness
> in a few inspector tests:
> 
>
https://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=in...
> 
> I'm not 100% sure yet because there's not much data after the revert, but
> at least the flakiness of timeouts and leaks started at almost the same
> time of this commit.

OK, this commit was actually the culprit for inspector tests
failure.

Please check leaks AND inspector tests flakiness when you
reland this.

[ikilpatrick, 2016-09-15 15:45:49]

On 2016/09/15 10:17:13, Yuta Kitamura wrote:
> On 2016/09/15 08:41:11, Yuta Kitamura wrote:
> > Also this commit is suspected to be causing layout test flakiness
> > in a few inspector tests:
> > 
> >
>
https://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=in...
> > 
> > I'm not 100% sure yet because there's not much data after the revert, but
> > at least the flakiness of timeouts and leaks started at almost the same
> > time of this commit.
> 
> OK, this commit was actually the culprit for inspector tests
> failure.
> 
> Please check leaks AND inspector tests flakiness when you
> reland this.

Thanks for the revert, will check both of those when we reland.

[ikilpatrick, 2016-09-29 23:32:06]

Description was changed from

==========
[worklets] Introduce ThreadedWorkletMessagingProxy and
AnimationWorkletMessagaingProxy.

This patch does the plumbing from the animationworklet to the globalscope to
import scripts etc.

BUG=567358

Committed: https://crrev.com/4bd1a1b4e1d321526cfe0c10adc15f507a661bb8
Cr-Commit-Position: refs/heads/master@{#418765}
==========

to

==========
[worklets] Introduce ThreadedWorkletMessagingProxy and
AnimationWorkletMessagaingProxy.

This patch does the plumbing from the animationworklet to the globalscope to
import scripts etc.

BUG=567358

Committed: https://crrev.com/4bd1a1b4e1d321526cfe0c10adc15f507a661bb8
Cr-Commit-Position: refs/heads/master@{#418765}
==========

[ikilpatrick, 2016-09-29 23:44:45]

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

[commit-bot: I haz the power, 2016-09-29 23:45:25]

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

[ikilpatrick, 2016-09-29 23:47:51]

nhiroki / majidvp PTAL

The leak(s) was due to not running with --enable-threaded-compositing like the
rest of the CompositorWorker tests.

As we were creating the thread when we touched window.animationWorklet, I've
moved it to be created when we first have an import call.

The crashes were fixed separately in:
codereview.chromium.org/2340693003

[ikilpatrick, 2016-09-29 23:55:28]

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

[commit-bot: I haz the power, 2016-09-29 23:56:19]

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-09-30 02:04:10]

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

[commit-bot: I haz the power, 2016-09-30 02:04:11]

Dry run: This issue passed the CQ dry run.

[nhiroki, 2016-09-30 10:50:36]

lgtm

[ikilpatrick, 2016-09-30 16:21:01]

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

[commit-bot: I haz the power, 2016-09-30 16:21:15]

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

[majidvp, 2016-09-30 16:55:48]

On 2016/09/30 10:50:36, nhiroki (slow) wrote:
> lgtm

lgtm. We should eventually figure out why not having the flag
(--enable-threaded-compositing)
 causes a leak but this looks fine on its own.

[commit-bot: I haz the power, 2016-09-30 18:42:32]

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

[commit-bot: I haz the power, 2016-09-30 18:42:34]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[ikilpatrick, 2016-09-30 18:46:03]

The CQ bit was checked by ikilpatrick@chromium.org

[ikilpatrick, 2016-09-30 18:46:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
yhirano@chromium.org, nhiroki@chromium.org
Link to the patchset: https://codereview.chromium.org/2312493002/#ps280001
(title: "rebase.")

[commit-bot: I haz the power, 2016-09-30 18:46:37]

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-09-30 19:50:40]

Description was changed from

==========
[worklets] Introduce ThreadedWorkletMessagingProxy and
AnimationWorkletMessagaingProxy.

This patch does the plumbing from the animationworklet to the globalscope to
import scripts etc.

BUG=567358

Committed: https://crrev.com/4bd1a1b4e1d321526cfe0c10adc15f507a661bb8
Cr-Commit-Position: refs/heads/master@{#418765}
==========

to

==========
[worklets] Introduce ThreadedWorkletMessagingProxy and
AnimationWorkletMessagaingProxy.

This patch does the plumbing from the animationworklet to the globalscope to
import scripts etc.

BUG=567358

Committed: https://crrev.com/4bd1a1b4e1d321526cfe0c10adc15f507a661bb8
Cr-Commit-Position: refs/heads/master@{#418765}
==========

[commit-bot: I haz the power, 2016-09-30 19:50:42]

Committed patchset #13 (id:280001)

[commit-bot: I haz the power, 2016-09-30 19:53:12]

Description was changed from

==========
[worklets] Introduce ThreadedWorkletMessagingProxy and
AnimationWorkletMessagaingProxy.

This patch does the plumbing from the animationworklet to the globalscope to
import scripts etc.

BUG=567358

Committed: https://crrev.com/4bd1a1b4e1d321526cfe0c10adc15f507a661bb8
Cr-Commit-Position: refs/heads/master@{#418765}
==========

to

==========
[worklets] Introduce ThreadedWorkletMessagingProxy and
AnimationWorkletMessagaingProxy.

This patch does the plumbing from the animationworklet to the globalscope to
import scripts etc.

BUG=567358

Committed: https://crrev.com/4bd1a1b4e1d321526cfe0c10adc15f507a661bb8
Committed: https://crrev.com/a49dbd327c02c506b07bc9c024e2492b34a4c779
Cr-Original-Commit-Position: refs/heads/master@{#418765}
Cr-Commit-Position: refs/heads/master@{#422192}
==========

[commit-bot: I haz the power, 2016-09-30 19:53:14]

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