Copied code from the same method and accepts HTMLVideoElement.
Created attachment 32603 [details] patch
Comment on attachment 32603 [details] patch > + if (ec != 0) { > + V8Proxy::setDOMException(ec); > + return notHandledByInterceptor(); > + } This probably should be if (ec) return throwError(ec); > + if (ec != 0) { > + V8Proxy::setDOMException(ec); > + return notHandledByInterceptor(); > + } Ditto. > + break; > + default: > + V8Proxy::throwError(V8Proxy::SyntaxError, "drawImage: Invalid number of arguments"); > + return v8::Undefined(); return throwError(V8Proxy::SyntaxError, "drawImage: Invalid number of arguments"); > Index: ChangeLog > =================================================================== > --- ChangeLog (revision 45742) > +++ ChangeLog (working copy) > @@ -1,3 +1,12 @@ > +2009-07-07 Alpha Lam <hclam@chromium.org> > + > + Reviewed by NOBODY(OOPS!). > + > + Changed CanvasRenderingContext2DDrawImage to accept HTMLVideoElement > + as a parameter of drawImage of HTMLCanvasElement. > + Wrong ChangeLog? Should be WebCore/ChangeLog.
Created attachment 32672 [details] patch V8Proxy::throwError takes an ErrorType rather than error code (i.e. |ec|). So I keep part where error code is useful.
Is this a V8-only issue, or a V8-only fix to an issue that affects WebKit even without V8?
(In reply to comment #4) > Is this a V8-only issue, or a V8-only fix to an issue that affects WebKit even > without V8? This is a V8-only issue. It only applies when V8 is used with WebKit.
(In reply to comment #5) > This is a V8-only issue. It only applies when V8 is used with WebKit. OK. Please mention that in the bug title in the future.
Here are the throwError helpers I was suggesting to use: http://trac.webkit.org/browser/trunk/WebCore/bindings/v8/V8Proxy.h (look at the bottom, trac down atm).
Created attachment 32678 [details] patch
Created attachment 32679 [details] patch
I have updated the patch using the throwError helper method.
Note that "return throwError(...)" will give you v8::Undefined(), whereas "return notHandledByInterceptor()" will produce v8::Null() (which is a signal for the interceptor handler). I am not sure why we were using notHandledByInterceptor() here and worry if changing this may introduce layout test regressions?
(In reply to comment #11) > Note that "return throwError(...)" will give you v8::Undefined(), whereas > "return notHandledByInterceptor()" will produce v8::Null() (which is a signal > for the interceptor handler). I am not sure why we were using > notHandledByInterceptor() here and worry if changing this may introduce layout > test regressions? It might break some layout tests, I'd be safe and do what is in the original code.
Ok. Sorry for the run-around. Pls upload the updated patch and I'll r+. Can you also file a WebKit bug to investigate why we have these weird notHandledByInterceptor retvals?
Created attachment 32726 [details] patch
Comment on attachment 32726 [details] patch Good to go.
Committed as http://trac.webkit.org/changeset/45929 Fixed changelog to have a link to the bug in http://trac.webkit.org/changeset/45932