Bug 168632 – REGRESSION (r207720): /more/conformance/conformance/quickCheckAPI-S_V.html test fails

• Component: Canvas
• Status: RESOLVED FIXED

[Chris Dumez]

After r207720, the following WebGL conformance tests started failing:
- /more/conformance/conformance/quickCheckAPI-S_V.html
- /context/context-lost.html

We started throwing security errors in case where we did not before.
Chrome and Firefox are both passing these tests so our behavior is not interoperable.

[Chris Dumez]

<rdar://problem/30620129>

[Chris Dumez]

Created attachment 302223 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/30623069>

[Darin Adler]

Comment on attachment 302223 [details]
Patch

Thanks for fixing this. I would prefer that we find a way to do this without ExceptionCode& out arguments, perhaps by having a return value of a different type.

[Alex Christensen]

Comment on attachment 302223 [details]
Patch

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

> Source/WebCore/html/canvas/WebGLRenderingContextBase.h:783
> +    bool validateHTMLImageElement(const char* functionName, HTMLImageElement*, ExceptionCode&);
> +    bool validateHTMLCanvasElement(const char* functionName, HTMLCanvasElement*, ExceptionCode&);
>  #if ENABLE(VIDEO)
> -    bool validateHTMLVideoElement(const char* functionName, HTMLVideoElement*);
> +    bool validateHTMLVideoElement(const char* functionName, HTMLVideoElement*, ExceptionCode&);

Wouldn't it be cleaner if these returned ExceptionOr<bool>?

[Chris Dumez]

Comment on attachment 302223 [details]
Patch

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

>> Source/WebCore/html/canvas/WebGLRenderingContextBase.h:783
>> +    bool validateHTMLVideoElement(const char* functionName, HTMLVideoElement*, ExceptionCode&);
> 
> Wouldn't it be cleaner if these returned ExceptionOr<bool>?

I actually started writing this as ExceptionOr<bool> but found it a bit awkward. This is why I went with a simple revert for now.
I can be convinced to go with ExceptionOr<bool> though. I think Darin may be hinting in this direction by saying we should not use ExceptionCode& parameters.

[Chris Dumez]

Comment on attachment 302223 [details]
Patch

Will land as is for now to fix the regression. We can implement a better design in a follow-up.

[WebKit Commit Bot]

Comment on attachment 302223 [details]
Patch

Clearing flags on attachment: 302223

Committed r212784: <http://trac.webkit.org/changeset/212784>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.