Closed Bug 917295 Opened 6 years ago Closed 6 years ago

GC: Handlify public date APIs

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: jonco, Assigned: jonco)

References

Details

(Keywords: dev-doc-needed)

Attachments

(1 file)

No description provided.
Summary: GC: Handlify public data APIs → GC: Handlify public date APIs
Attachment #806587 - Flags: review?(bzbarsky)
Attachment #806587 - Flags: review?(terrence)
Comment on attachment 806587 [details] [diff] [review]
handlify-date-apis

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

The JSAPI change looks good. Thank you for not making me look too closely at IDBKeyRange: that looks messy.
Attachment #806587 - Flags: review?(terrence) → review+
Comment on attachment 806587 [details] [diff] [review]
handlify-date-apis

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

KeyRange is horrible!

::: dom/indexedDB/IDBKeyRange.cpp
@@ +239,5 @@
>  {
>    nsresult rv;
>    nsRefPtr<IDBKeyRange> keyRange;
>  
> +  if (!JSVAL_IS_VOID(aVal) && !JSVAL_IS_NULL(aVal)) {

if (!aVal.isNullOrUndefined()) {

@@ +245,2 @@
>  
> +    JS::Rooted<JSObject*> obj(aCx, aVal.isObject() ? aVal.toObjectOrNull() : NULL);

I think we should use JS::RootedObject as well, when we use HandleObject already. If you already use isObject, then you can also use aVal.toObject()

@@ +245,3 @@
>  
> +    JS::Rooted<JSObject*> obj(aCx, aVal.isObject() ? aVal.toObjectOrNull() : NULL);
> +    if (JSVAL_IS_PRIMITIVE(aVal) ||

aVal.isPrimitive(). Seems like JS_IsArrayObject(aCx, obj) would fit on this line?
Comment on attachment 806587 [details] [diff] [review]
handlify-date-apis

The codegen bit should probably use "possibleDateObject" as the variable name, to reduce risk of collisions.

r=me with that on that part.

Someone needs to carefully review the IDB bits, still.
Attachment #806587 - Flags: review?(bzbarsky) → review+
Comment on attachment 806587 [details] [diff] [review]
handlify-date-apis

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

Also, r=me on the IDBKeyRange bits, although I don't have anything to add over what Tom found.
https://hg.mozilla.org/mozilla-central/rev/020f08616536
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Blocks: 918819
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.