Open
Bug 767875
Opened 12 years ago
Updated 2 years ago
xpcshell does not allow to create a worker from scripts loaded from a local URL without the protocol scheme.
Categories
(Core :: DOM: Workers, defect, P5)
Core
DOM: Workers
Tracking
()
NEW
People
(Reporter: sinker, Unassigned)
References
Details
Attachments
(1 file, 1 obsolete file)
25.27 KB,
patch
|
bent.mozilla
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Comment 1•12 years ago
|
||
We usually load a script with a local native path (an invalid URI), but WorkerPrivate always check for if it is valid URL. So, we need a patch to make local native path as an URI automatically.
Reporter | ||
Comment 2•12 years ago
|
||
Try to treat an native file path as a local file URI.
Updated•12 years ago
|
Component: DOM → DOM: Workers
QA Contact: general → workers
Reporter | ||
Updated•12 years ago
|
Attachment #636242 -
Flags: review?(bent.mozilla)
Reporter | ||
Comment 3•12 years ago
|
||
ping for review
Jonas, what do you think about this behavior? Is this something we should allow?
This sounds like a good idea, but we should use a different constructor than just "Worker". Maybe something like "new LocalWorker"?
Reporter | ||
Comment 6•12 years ago
|
||
(In reply to Jonas Sicking (:sicking) from comment #5) > This sounds like a good idea, but we should use a different constructor than > just "Worker". Maybe something like "new LocalWorker"? Agree!
Comment on attachment 636242 [details] [diff] [review] handle local file path as script URIs of workers Review of attachment 636242 [details] [diff] [review]: ----------------------------------------------------------------- Clearing review until a new patch shows up.
Attachment #636242 -
Flags: review?(bent.mozilla)
Reporter | ||
Comment 8•12 years ago
|
||
Add LocalWorker handling local file pathes as script locations.
Attachment #636242 -
Attachment is obsolete: true
Attachment #659274 -
Flags: review?(bent.mozilla)
Comment on attachment 659274 [details] [diff] [review] Handle local file pathes as script URIs of workers, v2 Review of attachment 659274 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for being patient here. r=me with these changes: ::: dom/workers/RuntimeService.cpp @@ +434,5 @@ > if (isChrome && !chromeworker::InitClass(aCx, aObj, worker, true)) { > return false; > } > > + if (isChrome && !localworker::InitClass(aCx, aObj, worker, true)) { Nit: Just combine this with the other isChrome check: if (isChrome && (!chromeWorker::InitClass(...) || !localWorker::InitClass(...))) ::: dom/workers/ScriptLoader.cpp @@ +270,5 @@ > nsCOMPtr<nsIURI> uri; > rv = nsContentUtils::NewURIWithDocumentCharset(getter_AddRefs(uri), > loadInfo.mURL, parentDoc, > baseURI); > + if (NS_FAILED(rv) && mWorkerPrivate->IsLocalWorker()) { This shouldn't be needed, right? You already might have prepended this in the worker constructor. I would expect this would make "file://file://foo.js" ::: dom/workers/Worker.cpp @@ +400,5 @@ > + return ConstructInternal(aCx, > + aArgc, > + aVp, > + WorkerType::WT_CHROMEWORKER, > + Class()); Nit: Please don't do one arg per line like this, here and below. @@ +452,5 @@ > +/** > + * Workers created from a local URL (without a scheme). > + * > + * It is avaiable only for chrome, but worker itself is not with > + * chrome privilege. Nit: "This type of worker may only be created from chrome but the resultant worker does not have chrome privileges." @@ +545,5 @@ > +}; > + > +// When this DOMJSClass is removed and it's the last consumer of > +// sNativePropertyHooks then sNativePropertyHooks should be removed too. > +DOMJSClass LocalWorker::sClass = { Make sure everything that you copy/pasted here is still correct. There have been some JS engine changes since you wrote this the first time. ::: dom/workers/WorkerPrivate.h @@ +164,5 @@ > > + enum WorkerType { > + WT_WORKER = 0, > + WT_LOCALWORKER, > + WT_CHROMEWORKER How about just 'Normal, Local, Chrome'? All caps make it look awful. ::: dom/workers/test/localWorker_worker.js @@ +1,5 @@ > +/** > + * Any copyright is dedicated to the Public Domain. > + * http://creativecommons.org/publicdomain/zero/1.0/ > + */ > +onmessage = function(event) { Can you make sure that 'LocalWorker' and 'ChromeWorker' and something like 'ctypes' is not available here?
Attachment #659274 -
Flags: review?(bent.mozilla) → review+
Updated•6 years ago
|
Priority: -- → P5
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•