Bug 62864 - [V8] Fix WebGL bindings for subarrays
Summary: [V8] Fix WebGL bindings for subarrays
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mads Ager
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-17 07:51 PDT by Mads Ager
Modified: 2011-06-20 02:36 PDT (History)
0 users

See Also:

Attachments
Patch (3.52 KB, patch)
2011-06-17 07:53 PDT, Mads Ager 		no flags Details | Formatted Diff | Diff
Patch (3.83 KB, patch)
2011-06-18 00:23 PDT, Mads Ager 		no flags Details | Formatted Diff | Diff
Patch (3.92 KB, patch)
2011-06-20 02:13 PDT, Mads Ager 		abarth: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Mads Ager 2011-06-17 07:51:29 PDT 
[V8] Fix WebGL bindings for subarrays
Comment 1 Mads Ager 2011-06-17 07:53:01 PDT 
Created attachment 97601 [details]
Patch
Comment 2 anton muhin 2011-06-17 09:08:12 PDT 
Comment on attachment 97601 [details]
Patch

Nice and neat!  LGTM
Comment 3 anton muhin 2011-06-17 09:09:49 PDT 
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?
Comment 4 Mads Ager 2011-06-18 00:23:41 PDT 
Created attachment 97688 [details]
Patch
Comment 5 Mads Ager 2011-06-18 00:26:26 PDT 
(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 6 anton muhin 2011-06-20 01:03:32 PDT 
Comment on attachment 97688 [details]
Patch

Nice!  LGTM
Comment 7 Adam Barth 2011-06-20 02:04:36 PDT 
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 ?
Comment 8 Mads Ager 2011-06-20 02:13:10 PDT 
Created attachment 97766 [details]
Patch
Comment 9 Mads Ager 2011-06-20 02:14:18 PDT 
(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 10 Adam Barth 2011-06-20 02:30:55 PDT 
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.)
Comment 11 Mads Ager 2011-06-20 02:36:45 PDT 
Committed r89236: <http://trac.webkit.org/changeset/89236>