[webkit-reviews] review denied: [Bug 68949] [MutationObservers] Add stub implementation of WebKitMutationObserver : [Attachment 110394] Use ASSERT instead of CRASH

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Oct 10 14:16:08 PDT 2011


Oliver Hunt <oliver at apple.com> has denied Adam Klein <adamk at chromium.org>'s
request for review:
Bug 68949: [MutationObservers] Add stub implementation of
WebKitMutationObserver
https://bugs.webkit.org/show_bug.cgi?id=68949

Attachment 110394: Use ASSERT instead of CRASH
https://bugs.webkit.org/attachment.cgi?id=110394&action=review

------- Additional Comments from Oliver Hunt <oliver at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=110394&action=review


r- due to the noted issues in the JSC bindings sorry :-/

> Source/WebCore/bindings/js/JSWebKitMutationObserverCustom.cpp:53
> +    if (!jsConstructor)
> +	   return throwVMError(exec, createReferenceError(exec,
"WebKitMutationObserver constructor callee is unavailable"));

This should be an ASSERT(jsConstructor);  and also a
ASSERT(jsConstructor->inherits(&JSWebKitMutationObserverConstructor::s_info));

Have you seen a case where you have a null callee?

> Source/WebCore/bindings/js/JSWebKitMutationObserverCustom.cpp:60
> +    if (!exec->argument(0).isUndefinedOrNull()) {
> +	   if (JSObject* object = exec->argument(0).getObject())

You should be able to simplify this to

JSObject* object = exec->argument(0).getObject();
if (!object)
    return throwVMError(exec, createTypeError(exec, "Argument is not an
Object"));
RefPtr<MutationCallback> callback = JSMutationCallback::create(object,
jsConstructor->globalObject());
return JSValue::encode(toJS(exec, jsConstructor->globalObject(),
WebKitMutationObserver::create(callback.release())));


More information about the webkit-reviews mailing list