Closed
Bug 559447
Opened 15 years ago
Closed 15 years ago
Workers: Make ChromeWorkers that have access to ctypes for browser and extensions
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla1.9.3a5
People
(Reporter: bent.mozilla, Assigned: bent.mozilla)
References
Details
(Keywords: dev-doc-complete)
Attachments
(1 file, 3 obsolete files)
49.66 KB,
patch
|
jst
:
review+
|
Details | Diff | Splinter Review |
ChromeOnlyWorker is a new constructor only available in chrome pages. It allows you to create a normal worker that has access to jsctypes for interacting with outside libraries. Should be useful for extensions (weave, for instance). To use it: var worker = new ChromeOnlyWorker(<scriptfile.js>); That scriptfile.js may use ctypes by simply accessing the 'ctypes' property on the global. It may also create a sub-ChromeOnlyWorker. Content script may not see the constructor, and all ChromeOnlyWorker objects should be auto-wrapped in a SOW to keep them from being shared with content. Includes an adaptor to run all the current jsctypes tests in a ChromeOnlyWorker and another test to make sure that web pages can't use them. Depends on fixing bug 559442 to keep GC from crashing ;)
Attachment #439103 -
Flags: review?(mrbkap)
Attachment #439103 -
Flags: review?(jst)
Attachment #439103 -
Flags: review?(dwitte)
Comment 1•15 years ago
|
||
Comment on attachment 439103 [details] [diff] [review] Patch, v1 >diff --git a/dom/src/threads/nsDOMWorker.cpp b/dom/src/threads/nsDOMWorker.cpp > class nsDOMWorkerFunctions > { > public: >+ typedef nsDOMWorker::WorkerPrivilegeModel WorkerPrivilegeModel; >+ > // Same as window.dump(). >- static JSBool Dump(JSContext* aCx, JSObject* aObj, uintN aArgc, >- jsval* aArgv, jsval* aRval); >+ static JSBool >+ Dump(JSContext* aCx, JSObject* aObj, uintN aArgc, jsval* aArgv, jsval* aRval); FWIW in ctypes I used a namespace for this; having a container class full of static functions felt a little bit weird. >+JSBool >+nsDOMWorkerFunctions::CTypesLazyGetter(JSContext* aCx, >+ JSObject* aObj, >+ jsval aId, >+ jsval* aVp) >+{ >+ if (worker->mPrivilegeModel != nsDOMWorker::CHROME) { >+ NS_ERROR("This shouldn't be possible!"); >+ JS_ReportError(aCx, "Not allowed to call this!"); >+ return JS_FALSE; >+ } I don't really think this needs to be a release-build check, since the only way this test can fail is statically, by coder error -- the property is only added for nsDOMWorker::CHROME. r=dwitte on the ctypes & testing bits. Very nice. :)
Attachment #439103 -
Flags: review?(dwitte) → review+
Assignee | ||
Comment 2•15 years ago
|
||
Ok, shaver wanted two things: 1. s/ChromeOnlyWorker/ChromeWorker/ 2. JSVERSION_LATEST for ChromeWorkers. Both now fixed, plus I made dwitte's suggested fix about asserting rather than runtime checking privileged status in the ctypes getter.
Attachment #439103 -
Attachment is obsolete: true
Attachment #439122 -
Flags: review?(jst)
Attachment #439103 -
Flags: review?(mrbkap)
Attachment #439103 -
Flags: review?(jst)
Assignee | ||
Updated•15 years ago
|
Summary: Make ChromeOnlyWorkers that have access to ctypes for browser and extensions → Make ChromeWorkers that have access to ctypes for browser and extensions
Assignee | ||
Updated•15 years ago
|
OS: Mac OS X → All
Assignee | ||
Comment 3•15 years ago
|
||
Comment on attachment 439122 [details] [diff] [review] Patch, v1.1 Oh, and carrying r=dwitte
Attachment #439122 -
Flags: review+
Assignee | ||
Comment 4•15 years ago
|
||
Oops, missed one ChromeOnlyWorker...
Attachment #439122 -
Attachment is obsolete: true
Attachment #439125 -
Flags: review?(jst)
Attachment #439122 -
Flags: review?(jst)
Comment 5•15 years ago
|
||
Comment on attachment 439125 [details] [diff] [review] Patch, v1.2 +#define NS_DEFINE_PRIVILEGED_CLASSINFO_DATA_WITH_NAME(_class, _name, _helper, \ + _flags) \ How about NS_DEFINE_CHROME_ONLY_CLASSINFO_DATA_WITH_NAME()? This isn't as much about privilege or not as it is about where we want things to show up IMO. - In nsWindowSH::GlobalResolve(): + // Don't expose privileged constructors to non-privileged windows. + if (name_struct->mPrivilegedOnly) { + nsIScriptSecurityManager* ssm = nsContentUtils::GetSecurityManager(); + NS_ENSURE_STATE(ssm); + + PRBool enabled; + rv = ssm->IsCapabilityEnabled("UniversalXPConnect", &enabled); + NS_ENSURE_SUCCESS(rv, rv); I think this check should simply be whether the window on which this constructor is to be defined on is a chrome window or not rather than checking how we're called. If - In dom/base/nsDOMClassInfo.h: + PRBool mPrivileged; Name that mChromeOnly IMO. - In dom/base/nsScriptNameSpaceManager.{cpp,h} nsresult RegisterClassName(const char *aClassName, PRInt32 aDOMClassInfoID, + PRBool aPrivileged, const PRUnichar **aResult); Rename aPrivileged to aChromeOnly. - In dom/src/threads/nsDOMWorker.h etc: + PRBool IsPrivileged() { + return mPrivilegeModel == CHROME; + } IsChromeWorker()? r=jst with that.
Attachment #439125 -
Flags: review?(jst) → review+
Assignee | ||
Comment 6•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/b0a280657e23
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 7•15 years ago
|
||
Apparently this broke builds that use --disable-ctypes. Duh. Followup filed as bug 560572.
Summary: Make ChromeWorkers that have access to ctypes for browser and extensions → Workers: Make ChromeWorkers that have access to ctypes for browser and extensions
Updated•15 years ago
|
Keywords: dev-doc-needed
Updated•15 years ago
|
Target Milestone: --- → mozilla1.9.3a5
Comment 8•15 years ago
|
||
I think this may also have broken mozilla-central xulrunner builds on win32: http://tinderbox.mozilla.org/showlog.cgi?log=XULRunner/1272016920.1272021822.23288.gz Regression window: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5f229488969c&tochange=cdc8bf25220e
This was filed as bug 561408.
That's actually a dup. Both should have been fixed by http://hg.mozilla.org/mozilla-central/rev/1475dbaf9ed5.
Never mind, I qimported the wrong patch.
This kind of a band-aid, dodge-the-bullet kind of patch, but I don't repro the original bug with it applied. If anything tried to re-enter sendSyncMessage (a CPOW, say), the content process would blow up.
Comment on attachment 441123 [details] [diff] [review] Make the sendSyncMessage use 'sync' semantics Yeah ... oops.
Attachment #441123 -
Attachment is obsolete: true
Comment 16•14 years ago
|
||
Documented here: https://developer.mozilla.org/en/DOM/ChromeWorker Also mentioned with links on the main Worker page, as well as the "Using web workers" page here: https://developer.mozilla.org/En/Using_web_workers
Keywords: dev-doc-needed → dev-doc-complete
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
•