Summary: | [V8] We should set a class id for a NPObject wrapper | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kentaro Hara <haraken> | ||||||
Component: | WebCore JavaScript | Assignee: | Kentaro Hara <haraken> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, japhet, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Kentaro Hara
2013-01-18 02:39:39 PST
Created attachment 183414 [details]
Patch
Comment on attachment 183414 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183414&action=review What effect does setting the class ID have? I think it causes us to assume the object has native info, which is true of these objects. I think this change is probably ok, but I'll need to study it more. > Source/WebCore/bindings/v8/V8NPObject.cpp:434 > + // FIXME: Move staticNPObjectMap() to DOMDataStore. There's something weird about weakNPObjectCallback, which is why I didn't do this before. Our code might be good enough for us to do it now though. > Source/WebCore/bindings/v8/V8NPObject.cpp:446 > + v8::Persistent<v8::Object> wrapperHandle = v8::Persistent<v8::Object>::New(value); > + V8DOMWrapper::setWrapperClass(object, wrapperHandle); > + staticNPObjectMap().set(object, wrapperHandle); Is it ok that we've re-ordered this operations relative to _NPN_RetainObject and _NPN_RegisterObject ? Comment on attachment 183414 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183414&action=review > Source/WebCore/bindings/v8/V8NPObject.cpp:445 > + V8DOMWrapper::setWrapperClass(object, wrapperHandle); I've checked, and this should be fine. The main thin we use the wrapper class for is make sure that the object has native info set. The main thing I'm worried about in this patch is the re-ordering of these calls and the _NPN_RetainObject and _NPN_RegisterObject calls. Is that necessary? Created attachment 183709 [details]
Patch
(In reply to comment #3) > The main thing I'm worried about in this patch is the re-ordering of these calls and the _NPN_RetainObject and _NPN_RegisterObject calls. Is that necessary? Fixed the re-order issue. Thanks! Comment on attachment 183709 [details]
Patch
Thanks.
Comment on attachment 183709 [details] Patch Rejecting attachment 183709 [details] from commit-queue. New failing tests: inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html Full output: http://queues.webkit.org/results/16010311 Comment on attachment 183709 [details] Patch Clearing flags on attachment: 183709 Committed r140302: <http://trac.webkit.org/changeset/140302> All reviewed patches have been landed. Closing bug. |