[webkit-reviews] review granted: [Bug 27737] [V8] Remove parameterless frame/window retrieval methods from V8Proxy : [Attachment 33572] Remove parameterless frame/window retrieval methods, v1.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jul 27 15:28:28 PDT 2009
David Levin <levin at chromium.org> has granted Dimitri Glazkov (Google)
<dglazkov at chromium.org>'s request for review:
Bug 27737: [V8] Remove parameterless frame/window retrieval methods from
V8Proxy
https://bugs.webkit.org/show_bug.cgi?id=27737
Attachment 33572: Remove parameterless frame/window retrieval methods, v1.
https://bugs.webkit.org/attachment.cgi?id=33572&action=review
------- Additional Comments from David Levin <levin at chromium.org>
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"
More information about the webkit-reviews
mailing list