Bug 168632 - REGRESSION (r207720): /more/conformance/conformance/quickCheckAPI-S_V.html test fails
Summary: REGRESSION (r207720): /more/conformance/conformance/quickCheckAPI-S_V.html te...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks: 163853
  Show dependency treegraph
 
Reported: 2017-02-20 20:36 PST by Chris Dumez
Modified: 2017-02-21 17:21 PST (History)
9 users (show)

See Also:


Attachments
Patch (177.81 KB, patch)
2017-02-20 20:53 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2017-02-20 20:36:21 PST
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.
Comment 1 Chris Dumez 2017-02-20 20:38:12 PST
<rdar://problem/30620129>
Comment 2 Chris Dumez 2017-02-20 20:53:13 PST
Created attachment 302223 [details]
Patch
Comment 3 Radar WebKit Bug Importer 2017-02-20 20:53:33 PST
<rdar://problem/30623069>
Comment 4 Darin Adler 2017-02-21 11:07:41 PST
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.
Comment 5 Alex Christensen 2017-02-21 11:09:21 PST
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>?
Comment 6 Chris Dumez 2017-02-21 12:04:47 PST
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.
Comment 7 Chris Dumez 2017-02-21 16:54:24 PST
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.
Comment 8 WebKit Commit Bot 2017-02-21 17:21:29 PST
Comment on attachment 302223 [details]
Patch

Clearing flags on attachment: 302223

Committed r212784: <http://trac.webkit.org/changeset/212784>
Comment 9 WebKit Commit Bot 2017-02-21 17:21:34 PST
All reviewed patches have been landed.  Closing bug.