1492014 - introduce WorkletImpl as an object to identify a worklet from parent and execution threads

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Karl Tomlinson (:karlt)]

When more than one worklet executes on the same worklet thread, each worklet
has its own global.

It is useful to have an object to represent the worklet and link its
associated objects on parent and execution threads.  Currently, WorkletThread
is the nearest thing to such an object, but this does not support multiple
worklets executing on the same thread.

WorkletLoadInfo can move from Worklet and WorkletThread to the new object,
which is a good point to subclass for AudioWorklet-specific storage and
behavior.

Separating this object from the parent-thread Worklet object provides a
useful notification when no owner-document references remain, and makes
multi-thread ownership simpler.

Comment 1

[Karl Tomlinson (:karlt)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d9072d5c005956b70c8d12490c4fba71e10b10c5

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: Bug 1492014 remove declaration for non-existant WorkletThread::GetWorkerPrincipal() r?baku

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: Bug 1492014 remove unused aWorkletLoadInfo parameter to ExecutionRunnable() r?baku

Depends on D6099

Comment 4

[Karl Tomlinson (:karlt)]

Attachment: Bug 1492014 introduce WorkletImpl as an object to identify a worklet from parent and execution threads r?baku

Depends on D6100

Comment 5

[Karl Tomlinson (:karlt)]

Attachment: Bug 1492014 document WorkletLoadInfo thread safety and relax destructor thread assertion r?baku

Depends on D6101

Comment 6

[Karl Tomlinson (:karlt)]

Attachment: Bug 1492014 reference WorkletImpl from WorkletGlobalScope r?baku

Depends on D6102

Comment 7

[Karl Tomlinson (:karlt)]

Attachment: Bug 1492014 get WorkletLoadInfo from global rather than thread in WorkletGlobalScope r?baku

We'll need to support multiple worklets sharng a single execution thread for
AudioWorklet.

Depends on D6103

Comment 8

[Karl Tomlinson (:karlt)]

Attachment: Bug 1492014 remove unused Console::GetParentObject() r?baku

WebIDL bindings do not need this method because console is a namespace.
All methods are static.

Depends on D6104

Comment 9

[Karl Tomlinson (:karlt)]

Attachment: Bug 1492014 store global as nsIGlobalObject on Console so that this may be used for Worklets also r?baku

Depends on D6105

Comment 10

[Karl Tomlinson (:karlt)]

Attachment: Bug 1492014 get WorkletLoadInfo from global instead of thread in Console r?baku

Depends on D6106

Comment 11

[Karl Tomlinson (:karlt)]

Attachment: Bug 1492014 remove now-unused WorkletThread::mWorkletLoadInfo r?baku

Depends on D6107

Comment 12

[Karl Tomlinson (:karlt)]

Attachment: Bug 1492014 add AudioWorkletImpl/PaintWorkletImpl subclasses r?baku

This will be useful for AudioWorklet-specific storage and behavior.

PaintWorkletImpl is in layout/style, because it will be referenced
from CSS.cpp in the same directory.

Depends on D6108

Comment 13

[Andrea Marchesini [:baku]]

Comment on attachment 9009817 [details]
Bug 1492014 add AudioWorkletImpl/PaintWorkletImpl subclasses r?baku

Andrea Marchesini [:baku] has approved the revision.

Comment 14

[Andrea Marchesini [:baku]]

Comment on attachment 9009816 [details]
Bug 1492014 remove now-unused WorkletThread::mWorkletLoadInfo r?baku

Andrea Marchesini [:baku] has approved the revision.

Comment 15

[Andrea Marchesini [:baku]]

Comment on attachment 9009815 [details]
Bug 1492014 get WorkletLoadInfo from global instead of thread in Console r?baku

Andrea Marchesini [:baku] has approved the revision.

Comment 16

[Andrea Marchesini [:baku]]

Comment on attachment 9009814 [details]
Bug 1492014 store global as nsIGlobalObject on Console so that this may be used for Worklets also r?baku

Andrea Marchesini [:baku] has approved the revision.

Comment 17

[Andrea Marchesini [:baku]]

Comment on attachment 9009813 [details]
Bug 1492014 remove unused Console::GetParentObject() r?baku

Andrea Marchesini [:baku] has approved the revision.

Comment 18

[Andrea Marchesini [:baku]]

Comment on attachment 9009812 [details]
Bug 1492014 get WorkletLoadInfo from global rather than thread in WorkletGlobalScope r?baku

Andrea Marchesini [:baku] has approved the revision.

Comment 19

[Andrea Marchesini [:baku]]

Comment on attachment 9009811 [details]
Bug 1492014 reference WorkletImpl from WorkletGlobalScope r?baku

Andrea Marchesini [:baku] has approved the revision.

Comment 20

[Andrea Marchesini [:baku]]

Comment on attachment 9009810 [details]
Bug 1492014 document WorkletLoadInfo thread safety and relax destructor thread assertion r?baku

Andrea Marchesini [:baku] has approved the revision.

Comment 21

[Andrea Marchesini [:baku]]

Comment on attachment 9009809 [details]
Bug 1492014 introduce WorkletImpl as an object to identify a worklet from parent and execution threads r?baku

Andrea Marchesini [:baku] has approved the revision.

Comment 22

[Andrea Marchesini [:baku]]

Comment on attachment 9009808 [details]
Bug 1492014 remove unused aWorkletLoadInfo parameter to ExecutionRunnable() r?baku

Andrea Marchesini [:baku] has approved the revision.

Comment 23

[Andrea Marchesini [:baku]]

Comment on attachment 9009807 [details]
Bug 1492014 remove declaration for non-existant WorkletThread::GetWorkerPrincipal() r?baku

Andrea Marchesini [:baku] has approved the revision.

Comment 24

[Karl Tomlinson (:karlt)]

Attachment: Bug 1492014 test Console::mInnerID instead of mGlobal when needing a window id r?baku

No behavior change intended.

Comment 25

[Karl Tomlinson (:karlt)]

Attachment: Bug 1492014 replace WorkletLoadInfo::mDumpEnabled with StaticPrefs r?baku

Depends on D6382

Comment 26

[Karl Tomlinson (:karlt)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=40e19cbf83a4f04c6e6c2149db6ebedc53d2b67e

Comment 27

[Andrea Marchesini [:baku]]

Comment on attachment 9010604 [details]
Bug 1492014 replace WorkletLoadInfo::mDumpEnabled with StaticPrefs r?baku

Andrea Marchesini [:baku] has approved the revision.

Comment 28

[Andrea Marchesini [:baku]]

Comment on attachment 9010603 [details]
Bug 1492014 test Console::mInnerID instead of mGlobal when needing a window id r?baku

Andrea Marchesini [:baku] has approved the revision.

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d77330a1cbfa
remove declaration for non-existant WorkletThread::GetWorkerPrincipal() r=baku
https://hg.mozilla.org/integration/autoland/rev/b0f2fafdba8c
remove unused aWorkletLoadInfo parameter to ExecutionRunnable() r=baku
https://hg.mozilla.org/integration/autoland/rev/3e142ea44140
introduce WorkletImpl as an object to identify a worklet from parent and execution threads r=baku
https://hg.mozilla.org/integration/autoland/rev/e34938a11fab
document WorkletLoadInfo thread safety and relax destructor thread assertion r=baku
https://hg.mozilla.org/integration/autoland/rev/4d1ba44e30ab
reference WorkletImpl from WorkletGlobalScope r=baku
https://hg.mozilla.org/integration/autoland/rev/ac095c2fdd8b
get WorkletLoadInfo from global rather than thread in WorkletGlobalScope r=baku
https://hg.mozilla.org/integration/autoland/rev/5a3967f37d3f
remove unused Console::GetParentObject() r=baku
https://hg.mozilla.org/integration/autoland/rev/d1654231ffb8
store global as nsIGlobalObject on Console so that this may be used for Worklets also r=baku
https://hg.mozilla.org/integration/autoland/rev/0c6b469c9618
get WorkletLoadInfo from global instead of thread in Console r=baku
https://hg.mozilla.org/integration/autoland/rev/8818f5ae0d13
remove now-unused WorkletThread::mWorkletLoadInfo r=baku
https://hg.mozilla.org/integration/autoland/rev/2798792623cf
add AudioWorkletImpl/PaintWorkletImpl subclasses r=baku
https://hg.mozilla.org/integration/autoland/rev/0a8510efbba3
test Console::mInnerID instead of mGlobal when needing a window id r=baku
https://hg.mozilla.org/integration/autoland/rev/f629b66a235f
replace WorkletLoadInfo::mDumpEnabled with StaticPrefs r=baku

Comment 30

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/d77330a1cbfa
https://hg.mozilla.org/mozilla-central/rev/b0f2fafdba8c
https://hg.mozilla.org/mozilla-central/rev/3e142ea44140
https://hg.mozilla.org/mozilla-central/rev/e34938a11fab
https://hg.mozilla.org/mozilla-central/rev/4d1ba44e30ab
https://hg.mozilla.org/mozilla-central/rev/ac095c2fdd8b
https://hg.mozilla.org/mozilla-central/rev/5a3967f37d3f
https://hg.mozilla.org/mozilla-central/rev/d1654231ffb8
https://hg.mozilla.org/mozilla-central/rev/0c6b469c9618
https://hg.mozilla.org/mozilla-central/rev/8818f5ae0d13
https://hg.mozilla.org/mozilla-central/rev/2798792623cf
https://hg.mozilla.org/mozilla-central/rev/0a8510efbba3
https://hg.mozilla.org/mozilla-central/rev/f629b66a235f

Comment 31

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/d03b538b6b41
Backed out 13 changesets for multiple failures. a=backout

Comment 32

[Ryan VanderMeulen [:RyanVM]]

Narcis, can you please link to the failures in question? Also, bugs backed out from m-c need to be reopened.

Comment 33

[Narcis Beleuzu [:NarcisB]]

Ryan, one of the failures that we think that there are caused by, is 
https://treeherder.mozilla.org/logviewer.html#?job_id=201400860&repo=mozilla-central&lineNumber=14440

I wanted to check if those failures were fixed by the backout and I`ll add them after

Comment 34

[Narcis Beleuzu [:NarcisB]]

:RyanVM, also https://treeherder.mozilla.org/logviewer.html#?job_id=201397970&repo=mozilla-central&lineNumber=2147

Comment 35

[Narcis Beleuzu [:NarcisB]]

After the backout, it seems that this push didn`t cause the https://treeherder.mozilla.org/logviewer.html#?job_id=201400860&repo=mozilla-central&lineNumber=14440 but causes :
1. Xpcshell failure - https://treeherder.mozilla.org/logviewer.html#?job_id=201397970&repo=mozilla-central&lineNumber=2147
2. Web platform failures:
   - https://treeherder.mozilla.org/logviewer.html#?job_id=201397883&repo=mozilla-central&lineNumber=3313
   - https://treeherder.mozilla.org/logviewer.html#?job_id=201397979&repo=mozilla-central&lineNumber=20595
   - https://treeherder.mozilla.org/logviewer.html#?job_id=201397925&repo=mozilla-central&lineNumber=23779
   - https://treeherder.mozilla.org/logviewer.html#?job_id=201397874&repo=mozilla-central&lineNumber=30425
   - https://treeherder.mozilla.org/logviewer.html#?job_id=201397905&repo=mozilla-central&lineNumber=13550
   - https://treeherder.mozilla.org/logviewer.html#?job_id=201397935&repo=mozilla-central&lineNumber=17057

Karl, can you please take a look?

Comment 36

[Narcis Beleuzu [:NarcisB]]

There were also Mochitests crashes on  [@ mozilla::dom::Console::Console(JSContext*, nsIGlobalObject*, unsigned long, unsigned long)]
Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=201397994&repo=mozilla-central&lineNumber=164462

Comment 37

[Karl Tomlinson (:karlt)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=db26948e2b76d614d2b1b1e1c81c172f0b850b9e

Comment 38

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc2b839f4e68
remove declaration for non-existant WorkletThread::GetWorkerPrincipal() r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/c6bb9f8bbaa3
remove unused aWorkletLoadInfo parameter to ExecutionRunnable() r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/8bfd0578ebaf
introduce WorkletImpl as an object to identify a worklet from parent and execution threads r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/19c7245a11e0
document WorkletLoadInfo thread safety and relax destructor thread assertion r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/7576b6eca5d1
reference WorkletImpl from WorkletGlobalScope r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb62e7dad63f
get WorkletLoadInfo from global rather than thread in WorkletGlobalScope r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/4d3a8fe2d2c7
remove unused Console::GetParentObject() r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/270ae583f0ef
store global as nsIGlobalObject on Console so that this may be used for Worklets also r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/d952654b9cc2
get WorkletLoadInfo from global instead of thread in Console r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/66eb950c3f82
remove now-unused WorkletThread::mWorkletLoadInfo r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/e545854fd235
add AudioWorkletImpl/PaintWorkletImpl subclasses r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb3fa704e175
test Console::mInnerID instead of mGlobal when needing a window id r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/17f44e6e8f49
replace WorkletLoadInfo::mDumpEnabled with StaticPrefs r=baku

Comment 39

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/dc2b839f4e68
https://hg.mozilla.org/mozilla-central/rev/c6bb9f8bbaa3
https://hg.mozilla.org/mozilla-central/rev/8bfd0578ebaf
https://hg.mozilla.org/mozilla-central/rev/19c7245a11e0
https://hg.mozilla.org/mozilla-central/rev/7576b6eca5d1
https://hg.mozilla.org/mozilla-central/rev/bb62e7dad63f
https://hg.mozilla.org/mozilla-central/rev/4d3a8fe2d2c7
https://hg.mozilla.org/mozilla-central/rev/270ae583f0ef
https://hg.mozilla.org/mozilla-central/rev/d952654b9cc2
https://hg.mozilla.org/mozilla-central/rev/66eb950c3f82
https://hg.mozilla.org/mozilla-central/rev/e545854fd235
https://hg.mozilla.org/mozilla-central/rev/cb3fa704e175
https://hg.mozilla.org/mozilla-central/rev/17f44e6e8f49