[webkit-reviews] review granted: [Bug 24727] Need to upstream V8XMLHttpRequest* : [Attachment 28801] Proposed fix.

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has granted David Levin
<levin at chromium.org>'s request for review:
Bug 24727: Need to upstream V8XMLHttpRequest*
https://bugs.webkit.org/show_bug.cgi?id=24727

Attachment 28801: Proposed fix.
https://bugs.webkit.org/attachment.cgi?id=28801&action=review

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
Proper design for all these hidden dependency helpers is out of scope for this
scrubbing, so LGTM, good to land after these changes:

> +    if (ec) {
> +	   V8Proxy::SetDOMException(ec);
> +	   return v8::Handle<v8::Value>();
> +    }

I have a helper in V8Proxy.h that might be useful here, throwError

> +    if (ec) {
> +	   V8Proxy::SetDOMException(ec);
> +	   return v8::Handle<v8::Value>();
> +    }

... and here.

> +    if (args.Length() < 2) {
> +	   V8Proxy::ThrowError(V8Proxy::SYNTAX_ERROR, "Not enough arguments");
> +	   return v8::Undefined();
> +    }

... and here.

> +    if (ec) {
> +	   V8Proxy::SetDOMException(ec);
> +	   return v8::Handle<v8::Value>();
> +    }

... and here.

> +    INC_STATS("DOM.XMLHttpRequest.getResponseHeader()");
> +    if (args.Length() < 1) {
> +	   V8Proxy::ThrowError(V8Proxy::SYNTAX_ERROR, "Not enough arguments");
> +	   return v8::Undefined();
> +    }

Ditto.

> +    if (ec) {
> +	   V8Proxy::SetDOMException(ec);
> +	   return v8::Handle<v8::Value>();
> +    }
> +    return v8StringOrNull(result);
> +}

Same thing.

> +    if (args.Length() < 1) {
> +	   V8Proxy::ThrowError(V8Proxy::SYNTAX_ERROR, "Not enough arguments");
> +	   return v8::Undefined();
> +    }

Analogously.