Summary: | [V8] Fix WebGL bindings for subarrays | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mads Ager <ager> | ||||||||
Component: | New Bugs | Assignee: | Mads Ager <ager> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | ||||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Mads Ager
2011-06-17 07:51:29 PDT
Created attachment 97601 [details]
Patch
Comment on attachment 97601 [details]
Patch
Nice and neat! LGTM
Comment on attachment 97601 [details]
Patch
One question though: do this objects go to some maps? To phrase differently: is toV8Independent equal to toV8 modulo MarkIndependent? And if yes, can we express it in more concise manner?
Created attachment 97688 [details]
Patch
(In reply to comment #3) > (From update of attachment 97601 [details]) > One question though: do this objects go to some maps? To phrase differently: is toV8Independent equal to toV8 modulo MarkIndependent? And if yes, can we express it in more concise manner? Yeah, toV8Independent is the same as toV8 modulo the MarkIndependent call. Refactored. Looking forward to getting this in. Reduced pause times in WebGL demoes from nearly one second to 30ms. :-) Comment on attachment 97688 [details]
Patch
Nice! LGTM
Comment on attachment 97688 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=97688&action=review > Source/WebCore/bindings/v8/V8Proxy.h:423 > + v8::Persistent<v8::Object> handle = v8::Persistent<v8::Object>::New(holder); Where is the Dispose that balances this New ? Created attachment 97766 [details]
Patch
(In reply to comment #7) > (From update of attachment 97688 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=97688&action=review > > > Source/WebCore/bindings/v8/V8Proxy.h:423 > > + v8::Persistent<v8::Object> handle = v8::Persistent<v8::Object>::New(holder); > > Where is the Dispose that balances this New ? Urgh. You are right, I introduced that bug when going from patch 1 to patch 2. Thank you Adam! Comment on attachment 97766 [details]
Patch
Aside from the fact that this patch adds code to V8Proxy (which needs to die), this looks good. (I don't fully understand what MarkIndependent does, but I assume you do.)
Committed r89236: <http://trac.webkit.org/changeset/89236> |