Last Comment Bug 697351 - Implement a has() trap on nodelist that doesn't involve dealing with property descriptors
: Implement a has() trap on nodelist that doesn't involve dealing with property...
Status: RESOLVED FIXED
: perf
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: All All
: P1 normal (vote)
: mozilla10
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
Depends on:
Blocks: 697343
  Show dependency treegraph
 
Reported: 2011-10-25 22:54 PDT by Boris Zbarsky [:bz]
Modified: 2011-10-26 17:04 PDT (History)
4 users (show)
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Implement has() on the nodelist proxy handler so that we don't have to do the property descriptor song and dance for it. (1.41 KB, patch)
2011-10-25 22:55 PDT, Boris Zbarsky [:bz]
peterv: review+
Details | Diff | Review

Description Boris Zbarsky [:bz] 2011-10-25 22:54:11 PDT
As the summary says.
Comment 1 Boris Zbarsky [:bz] 2011-10-25 22:55:20 PDT
Created attachment 569610 [details] [diff] [review]
Implement has() on the nodelist proxy handler so that we don't have to do the property descriptor song and dance for it.
Comment 2 Peter Van der Beken [:peterv] 2011-10-26 01:39:05 PDT
Comment on attachment 569610 [details] [diff] [review]
Implement has() on the nodelist proxy handler so that we don't have to do the property descriptor song and dance for it.

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

::: js/xpconnect/src/dombindings.cpp
@@ +821,5 @@
> +    JSBool protoHasProp;
> +    bool ok = JS_HasPropertyById(cx, proto, id, &protoHasProp);
> +    if (ok)
> +        *bp = protoHasProp;
> +    return ok;

Isn't it fine to do

  return JS_HasPropertyById(cx, proto, id, bp);

?
Comment 3 Boris Zbarsky [:bz] 2011-10-26 05:12:04 PDT
> Isn't it fine to do

Sadly, no.  That's what I wrote first, but bp is a bool* and JS_HasPropertyById has JSBool* as its last argument, and the two are not compatible.
Comment 4 Boris Zbarsky [:bz] 2011-10-26 05:55:06 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/2fef1412e0e8

Thanks for the fast review!
Comment 5 Ed Morley [:emorley] 2011-10-26 17:04:50 PDT
https://hg.mozilla.org/mozilla-central/rev/8f1b74f0b569

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