Closed
Bug 450449
Opened 16 years ago
Closed 16 years ago
Implement 'importScripts' for worker threads
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: bent.mozilla, Assigned: bent.mozilla)
References
Details
Attachments
(3 files)
(deleted),
patch
|
mrbkap
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jst
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
We need a way to load and compile external scripts on worker threads.
Assignee | ||
Comment 1•16 years ago
|
||
This patch implements the 'createWorkerFromURL' function on the pool as well as the 'loadScripts' function from within the worker.
nsIDOMWorkerThread createWorkerFromURL(in AString aURL);
void loadScripts(in AString aURL1, in AString aURL2, ...)
Current behavior is to download and compile scripts as they become available (off the main thread) but wait to execute until all scripts are compiled. Scripts are executed in the order they're passed in to the loadScripts function.
Attachment #335106 -
Flags: superreview?(jst)
Attachment #335106 -
Flags: review?(mrbkap)
Assignee | ||
Comment 3•16 years ago
|
||
Talked with mrbkap today and he suggested a few changes. This patch applies on top of attachment 335106 [details] [diff] [review] and changes:
1. Runtime rooting everywhere, no more using the context.
2. Suspending requests around locks
3. Removed some !!
4. Using NS_NOTREACHED instead of NS_ASSERTION(PR_FALSE)
Attachment #335669 -
Flags: review?(mrbkap)
Assignee | ||
Comment 4•16 years ago
|
||
Comment on attachment 335669 [details] [diff] [review]
Additional changes, v1
Oh, and I fixed an assertion that would fire if you reload a page super quickly.
Updated•16 years ago
|
Attachment #335106 -
Flags: review?(mrbkap) → review+
Comment 5•16 years ago
|
||
Comment on attachment 335669 [details] [diff] [review]
Additional changes, v1
r=mrbkap with the changes jst pointed out in person.
Attachment #335669 -
Flags: review?(mrbkap) → review+
Assignee | ||
Comment 6•16 years ago
|
||
Here are the additional changes requested by mrbkap/jst. It applies on top of attachment 335669 [details] [diff] [review] and attachment 335106 [details] [diff] [review]. Changes include:
1. Using volatile PRBools when appropriate.
2. Fixed comments on the general flow of the script loading process.
3. Renamed nsAutoRootedJSObject to nsAutoJSObjectHolder.
4. Replaced an atomic set with an ordinary set.
5. Added an absolute cap on the number of threads the service will ever create in the face of multiple malicious script loads.
Attachment #336076 -
Flags: superreview?(jst)
Attachment #336076 -
Flags: review?(jst)
Assignee | ||
Updated•16 years ago
|
Attachment #336076 -
Flags: superreview?(jst)
Attachment #336076 -
Flags: superreview?(jonas)
Attachment #336076 -
Flags: review?(jst)
Attachment #336076 -
Flags: review?(jonas)
Assignee | ||
Comment 7•16 years ago
|
||
Comment on attachment 336076 [details] [diff] [review]
Additional changes, v2
Doh, jst is out today.
Assignee | ||
Updated•16 years ago
|
Attachment #335106 -
Flags: superreview?(jst) → superreview?(jonas)
Assignee | ||
Updated•16 years ago
|
Attachment #336076 -
Flags: review?(jonas) → review?(mrbkap)
Updated•16 years ago
|
Attachment #336076 -
Flags: review?(mrbkap)
Comment 8•16 years ago
|
||
Comment on attachment 336076 [details] [diff] [review]
Additional changes, v2
As I understand it, the 'volatile' declaration only need to be on the original boolean.
Smaug pointed out on IRC that we should push a sane context onto the main thread's context stack when calling the error handler, and he's right. I think that because we cancel the threads on page shift, we *should* be able to just use the context associated with the window that owns the threads.
Holding off on a final + until then.
Updated•16 years ago
|
Attachment #336076 -
Flags: superreview?(jonas)
Attachment #336076 -
Flags: superreview+
Attachment #336076 -
Flags: review+
Comment 9•16 years ago
|
||
Comment on attachment 336076 [details] [diff] [review]
Additional changes, v2
sr=jst (and r=mrbkap).
Updated•16 years ago
|
Attachment #335106 -
Flags: superreview?(jonas) → superreview+
Comment 10•16 years ago
|
||
c++ -o nsDOMWorkerThread.o -c -I../../../dist/include/system_wrappers -include ../../../config/gcc_hidden.h -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_COM_OBSOLETE -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES -DZLIB_INTERNAL -DOSTYPE=\"Linux2.6\" -DOSARCH=Linux -I../../../dom/src/base -I. -I. -I../../../dist/include/caps -I../../../dist/include/content -I../../../dist/include/js -I../../../dist/include/layout -I../../../dist/include/necko -I../../../dist/include/pref -I../../../dist/include/string -I../../../dist/include/widget -I../../../dist/include/xpcom -I../../../dist/include/xpconnect -I../../../dist/include -I../../../dist/include/dom -I../../../dist/include/nspr -I../../../dist/sdk/include -fPIC -fno-rtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-long-long -pedantic -fno-strict-aliasing -fshort-wchar -pthread -pipe -DNDEBUG -DTRIMMED -O2 -DMOZILLA_CLIENT -include ../../../mozilla-config.h -Wp,-MD,.deps/nsDOMWorkerThread.pp nsDOMWorkerThread.cpp
nsDOMWorkerThread.cpp:245: error: expected initializer before ‘nsDOMWorkerFunctions’
make[5]: *** [nsDOMWorkerThread.o] Error 1
make[5]: Leaving directory `/home/mdew/ff/mozilla-central/dom/src/threads'
make[4]: *** [libs] Error 2
make[4]: Leaving directory `/home/mdew/ff/mozilla-central/dom/src'
make[3]: *** [libs] Error 2
make[3]: Leaving directory `/home/mdew/ff/mozilla-central/dom'
make[2]: *** [libs_tier_gecko] Error 2
Assignee | ||
Comment 11•16 years ago
|
||
Pushed changeset 56a46aed3502 to mozilla-central.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 12•16 years ago
|
||
(In reply to comment #11)
> Pushed changeset 56a46aed3502 to mozilla-central.
Er, changeset f3df01ee0118. Oops.
Updated•12 years ago
|
Component: DOM: Mozilla Extensions → DOM
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•