As part of cleaning up V8 bindings, I am removing parameterless retrieveFrame/retrieveWindow methods from V8Proxy to taper developers' temptation to use "the easiest-looking methods".
Created attachment 33572 [details] Remove parameterless frame/window retrieval methods, v1. WebCore/ChangeLog | 34 ++++++++++++++++++++ WebCore/bindings/v8/ScriptCallStack.cpp | 2 +- WebCore/bindings/v8/V8NPUtils.cpp | 2 +- WebCore/bindings/v8/V8Proxy.cpp | 18 +--------- WebCore/bindings/v8/V8Proxy.h | 5 --- WebCore/bindings/v8/custom/V8DatabaseCustom.cpp | 30 +++++++---------- .../v8/custom/V8HTMLAudioElementConstructor.cpp | 8 +++- .../v8/custom/V8HTMLImageElementConstructor.cpp | 8 +++- .../v8/custom/V8HTMLOptionElementConstructor.cpp | 8 +++- .../v8/custom/V8MessageChannelConstructor.cpp | 2 +- .../bindings/v8/custom/V8SQLTransactionCustom.cpp | 2 +- WebCore/bindings/v8/custom/V8WorkerCustom.cpp | 2 +- .../v8/custom/V8XMLHttpRequestConstructor.cpp | 10 +++++- 13 files changed, 80 insertions(+), 51 deletions(-)
Comment on attachment 33572 [details] Remove parameterless frame/window retrieval methods, v1. Some minor things to fix on check-in. > diff --git a/WebCore/bindings/v8/custom/V8DatabaseCustom.cpp b/WebCore/bindings/v8/custom/V8DatabaseCustom.cpp > + if (args.Length() == 0) Since you're changing this line, may I suggest if (!args.Length()) > + return throwError("Transaction callback is required.", V8Proxy::SyntaxError); > > + if (!args[1]->IsObject()) > + throwError("Transaction success callback must be of valid type."); Missing "return" before "throwError" > diff --git a/WebCore/bindings/v8/custom/V8XMLHttpRequestConstructor.cpp b/WebCore/bindings/v8/custom/V8XMLHttpRequestConstructor.cpp > index 8c447c6..a3e523c 100644 > --- a/WebCore/bindings/v8/custom/V8XMLHttpRequestConstructor.cpp > +++ b/WebCore/bindings/v8/custom/V8XMLHttpRequestConstructor.cpp > @@ -55,9 +55,15 @@ CALLBACK_FUNC_DECL(XMLHttpRequestConstructor) > WorkerContextExecutionProxy* proxy = WorkerContextExecutionProxy::retrieve(); > if (proxy) > context = proxy->workerContext(); > - else > + else { > +#endif > + Frame* frame = V8Proxy::retrieveFrameForCurrentContext(); > + if (!frame) > + return throwError("Frame associated with XMLHttpRequest constructor is not available.", V8Proxy::ReferenceError); Why did you go with a different style of error message here? This would match the format of the others: "XMLHttpRequest constructor associated frame is not available" But actually I like the format in this one better. Also, I think they would read better with a 's "XMLHttpRequest constructor's associated frame is not available"
Thanks for review and comments, Dave! Landed as http://trac.webkit.org/changeset/46435 with your comments addressed.