Summary: | REGRESSION (r207720): /more/conformance/conformance/quickCheckAPI-S_V.html test fails | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||
Component: | Canvas | Assignee: | Chris Dumez <cdumez> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, darin, dino, esprehn+autocc, graouts, gyuyoung.kim, kondapallykalyan, rniwa, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 163853 | ||||||
Attachments: |
|
Description
Chris Dumez
2017-02-20 20:36:21 PST
Created attachment 302223 [details]
Patch
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 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 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 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 on attachment 302223 [details] Patch Clearing flags on attachment: 302223 Committed r212784: <http://trac.webkit.org/changeset/212784> All reviewed patches have been landed. Closing bug. |