[Webkit-unassigned] [Bug 63615] Allow notification origin permission request when no js callback is provided

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=63615


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #108807|review?                     |review-
               Flag|                            |




--- Comment #8 from Darin Adler <darin at apple.com>  2011-10-17 12:08:34 PST ---
(From update of attachment 108807)
View in context: https://bugs.webkit.org/attachment.cgi?id=108807&action=review

review- because of the lack of a test case

> Source/WebCore/bindings/js/JSDesktopNotificationsCustom.cpp:68
> +    if (exec->argument(0).isObject()) {
> +        JSDOMGlobalObject* globalObject = toJSDOMGlobalObject(static_cast<Document*>(context), exec);
> +        RefPtr<JSCustomVoidCallback> callback = JSCustomVoidCallback::create(exec->argument(0).getObject(), globalObject);
>  
> -    PassRefPtr<JSCustomVoidCallback> callback = JSCustomVoidCallback::create(exec->argument(0).getObject(), toJSDOMGlobalObject(static_cast<Document*>(context), exec));
> +        impl()->requestPermission(callback.release());
> +    } else
> +        impl()->requestPermission(0);

It was good that you fixed the mistake where there was an local variable of type PassRefPtr.

But I’m not sure why you added a local variable for the global object. I personally don’t think it enhances readability.

Also, if we declared the callback local variable outside of the if statement we would not need an else.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.