[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