[webkit-reviews] review denied: [Bug 227792] Canvas and OffscreenCanvas getContext should check if argument is an object before trying to convert it to a dictionary : [Attachment 433328] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has denied Chris Lord <clord at igalia.com>'s request
for review:
Bug 227792: Canvas and OffscreenCanvas getContext should check if argument is
an object before trying to convert it to a dictionary
https://bugs.webkit.org/show_bug.cgi?id=227792

Attachment 433328: Patch

https://bugs.webkit.org/attachment.cgi?id=433328&action=review




--- Comment #5 from Sam Weinig <sam at webkit.org> ---
Comment on attachment 433328
  --> https://bugs.webkit.org/attachment.cgi?id=433328
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433328&action=review

> Source/WebCore/html/HTMLCanvasElement.cpp:288
> +	   auto settings =
convert<IDLDictionary<CanvasRenderingContext2DSettings>>(state,
(!arguments.isEmpty() && arguments[0].isObject()) ? arguments[0].get() :
JSC::jsUndefined());

I don't think we need the isEmpty() check if we are checking isObject().  That
said, I would perfer we make this a bit closer o the spec language (e.g.
https://html.spec.whatwg.org/multipage/canvas.html#dom-canvas-getcontext) and
use jsNull() if not an object).