Last Comment Bug 723517 - Drop cx argument from JS_GetClass(cx, obj)
: Drop cx argument from JS_GetClass(cx, obj)
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla13
Assigned To: Igor Bukanov
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-02 07:30 PST by Igor Bukanov
Modified: 2012-04-28 15:19 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (128.91 KB, patch)
2012-02-03 14:39 PST, Igor Bukanov
luke: review+
Details | Diff | Review

Description Igor Bukanov 2012-02-02 07:30:13 PST
Currently we declare JS_GetClass in jsapi.cpp as:

#ifdef JS_THREADSAFE
extern JS_PUBLIC_API(JSClass *)
JS_GetClass(JSContext *cx, JSObject *obj);

#define JS_GET_CLASS(cx,obj) JS_GetClass(cx, obj)
#else
extern JS_PUBLIC_API(JSClass *)
JS_GetClass(JSObject *obj);

#define JS_GET_CLASS(cx,obj) JS_GetClass(obj)
#endif

The reason for the cx argument under is JS_THREADSAFE is that when JS_THREADSAFE were introduced to the JS engine, JSObject did not have fixed slots. So the class pointer were stored in the dynamic slots. However, with thread-shared objects, the slot array could have been reallocated when accessing the object. So it was necessary to synchronize the access to the slot array using the request machinery and the cx argument allowed for that.

I suppose the cx argument should have been removed from JS_GetClass right after introduction of fat objects, but is better late then never.
Comment 1 Luke Wagner [:luke] 2012-02-02 08:33:36 PST
Agreed.  We have also stopped enforcing strict source compatibility (as long as there is an obvious transition path), so I'd be in favor of removing the JS_GET_CLASS macro altogether.
Comment 2 Igor Bukanov 2012-02-03 14:39:27 PST
Created attachment 594306 [details] [diff] [review]
v1

The patch drops the cx parameter from JS_GetClass and removes JS_GET_CLASS macro. 

Besides mechanical renaming JS_GET_CLASS into JS_GetClass the patch also propagated removal of the cx argument from the JS_GetClass callers when JS_GET_CLASS was the sole reason to pass the cx.
Comment 4 Marco Bonardo [::mak] 2012-02-04 02:45:45 PST
https://hg.mozilla.org/mozilla-central/rev/948fe2e7f1d5
Comment 5 Igor Bukanov 2012-02-04 12:34:01 PST
We need to update the documentation that JS_GET_CLASS(JSContext *cx, JSObject *obj) was removed and the code should call JS_GetClass(JSObject *obj) instead.
Comment 6 Igor Bukanov 2012-02-09 12:19:42 PST
By mistake I landed the patch for the bug 724310 with the commit message that refers to this  bug.
Comment 7 Jeff Walden [:Waldo] (remove +bmo to email) 2012-04-28 15:17:09 PDT
I split JS_GET_CLASS docs into that and one for the method, and I updated both to the current trunk state, also updating links to them to use JS_GetClass.

https://developer.mozilla.org/en/SpiderMonkey/JSAPI_Reference/JS_GET_CLASS
https://developer.mozilla.org/en/SpiderMonkey/JSAPI_Reference/JS_GetClass

The JSAPI reference overview now marks the macro as obsolete, and it links to both the macro and the method for completeness and backwards-usability.

Also, the 1.8.8 release notes mention that JS_GetClass's signature changed and that JS_GET_CLASS was removed.

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