Last Comment Bug 664577 - Fix inclusion of jsobj.h in Content code due to ArrayBuffer optimizations
: Fix inclusion of jsobj.h in Content code due to ArrayBuffer optimizations
Status: RESOLVED FIXED
fixed-in-tracemonkey
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)
:
Mentors:
Depends on: 656519
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-15 15:21 PDT by Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)
Modified: 2011-06-20 17:02 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
API fix (23.97 KB, patch)
2011-06-15 15:23 PDT, Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)
mrbkap: review+
Details | Diff | Review
API fix (17.33 KB, patch)
2011-06-16 11:12 PDT, Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)
no flags Details | Diff | Review

Description Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?) 2011-06-15 15:21:41 PDT
User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0.1) Gecko/20100101 Firefox/4.0.1
Build Identifier: 

Since ArrayBuffer was collapsed into a JSObject by bug 656519, content and DOM code which used it required the inclusion of jsobj.h, which is frowned upon :)

This patch creates two versions of the public API for ArrayBuffer, JS_GetArrayBufferByteLength and JS_GetArrayBufferData which are not-inline and can be used treating JSObject as opaque, while js_GetArrayBufferByteLength and js_GetArrayBufferData are used by the engine.

Reproducible: Always
Comment 1 Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?) 2011-06-15 15:23:56 PDT
Created attachment 539672 [details] [diff] [review]
API fix
Comment 2 Blake Kaplan (:mrbkap) (please use needinfo!) 2011-06-16 10:29:15 PDT
Comment on attachment 539672 [details] [diff] [review]
API fix

My only thought is that you could let the inlined versions still be member functions on ArrayBuffer (and let them simply be undefined for consumers outside the engine) and simply defined in jstypedarrayinlines.h. Do you have feelings one way or the other?
Comment 3 Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?) 2011-06-16 10:35:12 PDT
I put them as functions for consistency. Both private and public have the same name except for js/JS.
Comment 4 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-06-16 10:36:06 PDT
> My only thought is that you could let the inlined versions still be member
> functions on ArrayBuffer

Fwiw, this was my assumption about how we'd do it.
Comment 5 Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?) 2011-06-16 11:12:25 PDT
Created attachment 539834 [details] [diff] [review]
API fix

Inline implementation is kept part of the class.
Comment 6 Blake Kaplan (:mrbkap) (please use needinfo!) 2011-06-16 11:45:08 PDT
Yeah, the consistency is a hold-over from the pre-C++ days of SpiderMonkey. We're migrating more and more towards class methods over js_* methods.
Comment 7 Blake Kaplan (:mrbkap) (please use needinfo!) 2011-06-16 12:57:55 PDT
http://hg.mozilla.org/tracemonkey/rev/8b3dc129aed8

I didn't add the bug # to the checkin message, sadly :-/.
Comment 8 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-06-16 13:03:40 PDT
Back it out and reland with the bug#?
Comment 9 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-06-16 13:04:04 PDT
But it's not clear why that wasn't there to start with....
Comment 10 Blake Kaplan (:mrbkap) (please use needinfo!) 2011-06-16 14:04:46 PDT
http://hg.mozilla.org/tracemonkey/rev/ca93cfe414a0 <-- backout
http://hg.mozilla.org/tracemonkey/rev/e3c7aa315ca5 <-- relanding

Nikhil, in general, when you write your commit messages for patches, please include the bug # in the commit message. It make it easier when doing hg archaeology to tie specific commits to bugs.
Comment 11 Chris Leary [:cdleary] (not checking bugmail) 2011-06-20 17:02:28 PDT
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/ca93cfe414a0 (backout)
http://hg.mozilla.org/mozilla-central/rev/e3c7aa315ca5

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