Last Comment Bug 732936 - Feeding ArrayBuffer to js-ctypes
: Feeding ArrayBuffer to js-ctypes
Status: RESOLVED FIXED
: dev-doc-needed
Product: Core
Classification: Components
Component: js-ctypes (show other bugs)
: unspecified
: All All
: -- enhancement (vote)
: mozilla15
Assigned To: David Teller [:Yoric] (please use "needinfo")
:
: Jason Orendorff [:jorendorff]
Mentors:
: 710830 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-05 06:01 PST by David Teller [:Yoric] (please use "needinfo")
Modified: 2015-05-05 08:25 PDT (History)
6 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
First prototype (9.66 KB, patch)
2012-05-06 23:33 PDT, David Teller [:Yoric] (please use "needinfo")
jorendorff: feedback+
Details | Diff | Splinter Review
mplicitConvert ArrayBuffer to array or pointer (8.83 KB, patch)
2012-05-15 06:00 PDT, David Teller [:Yoric] (please use "needinfo")
jorendorff: review+
Details | Diff | Splinter Review
mplicitConvert ArrayBuffer to array or pointer (7.89 KB, patch)
2012-05-21 13:42 PDT, David Teller [:Yoric] (please use "needinfo")
no flags Details | Diff | Splinter Review
ImplicitConvert ArrayBuffer to array or pointer (7.90 KB, patch)
2012-05-22 03:16 PDT, David Teller [:Yoric] (please use "needinfo")
no flags Details | Diff | Splinter Review

Description David Teller [:Yoric] (please use "needinfo") 2012-03-05 06:01:10 PST
It would be nice to be able to use a TypedArray as a C array/pointer when performing a js-ctypes call. I believe it would be rather simple to adapt ImplicitConvert for that purpose.
Comment 1 David Teller [:Yoric] (please use "needinfo") 2012-05-06 23:33:26 PDT
Created attachment 621509 [details] [diff] [review]
First prototype
Comment 2 Jason Orendorff [:jorendorff] 2012-05-14 14:13:18 PDT
Comment on attachment 621509 [details] [diff] [review]
First prototype

Hmm. I like everything here except that nothing is holding on to the TypedArray, so it would be easy for this pointer to be taken, implicitly, and then for the buffer to be GC'd, leaving a dangling ctypes pointer.

One way around that is to move this code to ExplicitConvert. Another is to create utility functions:
  ctypes.bufferPtr(buffer or typedarray) 
  ctypes.addressOfElement(typedarray, index)
Another is to create a property on the pointer CData object that refers back to the TypedArray (but that's lame).

Another possibility is to allow this conversion only if isArgument is true, similar to what we do for strings. For arguments, the conversion is safe at least for the duration of the C function call, since all arguments are rooted on the JS stack.

Or we can leave it like it is and be really careful. It's up to you, you're the main ctypes user right now.
Comment 3 Jason Orendorff [:jorendorff] 2012-05-14 15:10:14 PDT
Actually, this is just the same thing we already do for js-ctypes arrays. It should be totally fine.
Comment 4 Jason Orendorff [:jorendorff] 2012-05-14 15:11:35 PDT
Comment on attachment 621509 [details] [diff] [review]
First prototype

no changes needed, post an updated patch for review

One note, there are a few bits in here that seem to belong to a different patch: the change to dom/system/OSFileConstants.cpp, and the one to toolkit/components/osfile/osfile_unix_front.jsm.
Comment 5 David Teller [:Yoric] (please use "needinfo") 2012-05-15 06:00:07 PDT
Created attachment 624009 [details] [diff] [review]
mplicitConvert ArrayBuffer to array or pointer

Same patch, with a few more comments and without parasite code.
Comment 6 Jason Orendorff [:jorendorff] 2012-05-21 08:32:00 PDT
Comment on attachment 624009 [details] [diff] [review]
mplicitConvert ArrayBuffer to array or pointer

Review of attachment 624009 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/ctypes/tests/unit/test_jsctypes.js.in
@@ +61,1 @@
>      }

Did you want these new print calls checked in? They don't hurt anything, afaik, so it's fine with me.

@@ +1910,5 @@
>    }
>  }
>  
>  function run_ArrayType_tests() {
> +  do_print("run_ArrayType_tests");

Here's another do_print(), just in case you meant to remove them.

@@ +2051,5 @@
> +    c.value = c_arraybuffer;
> +  } catch(x) {
> +    exn = x;
> +  }
> +  do_check_true(!!exn);

Use do_check_throws.
Comment 7 David Teller [:Yoric] (please use "needinfo") 2012-05-21 13:42:55 PDT
Created attachment 625748 [details] [diff] [review]
mplicitConvert ArrayBuffer to array or pointer
Comment 8 David Teller [:Yoric] (please use "needinfo") 2012-05-22 03:16:11 PDT
Created attachment 625956 [details] [diff] [review]
ImplicitConvert ArrayBuffer to array or pointer
Comment 9 Ryan VanderMeulen [:RyanVM] 2012-05-22 17:38:17 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/9320ebf5821d
Comment 10 Ed Morley [:emorley] 2012-05-23 04:53:21 PDT
https://hg.mozilla.org/mozilla-central/rev/9320ebf5821d
Comment 11 David Teller [:Yoric] (please use "needinfo") 2012-06-06 02:59:26 PDT
*** Bug 710830 has been marked as a duplicate of this bug. ***
Comment 12 David Teller [:Yoric] (please use "needinfo") 2012-09-28 15:49:08 PDT
Title is actually a misnomer.
Comment 13 noitidart 2015-05-05 08:25:11 PDT
Documentation started here: https://developer.mozilla.org/en-US/docs/Mozilla/js-ctypes/Using_js-ctypes/Working_with_ArrayBuffers

Probalby needs some serious technical review.

Note You need to log in before you can comment on or make changes to this bug.