[webkit-reviews] review granted: [Bug 23282] JSImmediate should be private. : [Attachment 26658] The patch

[bugzilla-daemon at webkit.org]

Oliver Hunt <oliver at apple.com> has granted Gavin Barraclough
<barraclough at apple.com>'s request for review:
Bug 23282: JSImmediate should be private.
https://bugs.webkit.org/show_bug.cgi?id=23282

Attachment 26658: The patch
https://bugs.webkit.org/attachment.cgi?id=26658&action=review

------- Additional Comments from Oliver Hunt <oliver at apple.com>

JavaScriptCore/API/JSCallbackObjectFunctions.h

@@ double JSCallbackObject<Base>::toNumber(
378378	       if (JSObjectConvertToTypeCallback convertToType =
jsClass->convertToType) {
379379		   JSLock::DropAllLocks dropAllLocks(exec);
380380		   if (JSValueRef value = convertToType(ctx, thisRef,
kJSTypeNumber, toRef(exec->exceptionSlot())))
381		     return toJS(value)->getNumber();
 381		     return toJS(value)->uncheckedGetNumber();
382382	       }
383383		   
384384	   return Base::toNumber(exec);

This isn't safe -- convertToType is developer defined so we can't guarantee
good behaviour, so this can't be unchecked, i think

double dValue;
return toJS(value)->getNumber(dValue) ? dValue : NaN;

Would be the correct version

Otherwise r=me