[webkit-reviews] review granted: [Bug 24561] Add custom V8 bindings for HTMLElementCanvas, Location : [Attachment 28556] patch

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has granted Mike Belshe
<mike at belshe.com>'s request for review:
Bug 24561: Add custom V8 bindings for HTMLElementCanvas, Location
https://bugs.webkit.org/show_bug.cgi?id=24561

Attachment 28556: patch
https://bugs.webkit.org/attachment.cgi?id=28556&action=review

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
Looks good, except for very minor style nits:

> +	   * bindings/v8/custom/V8HTMLCanvasElementCustom.cpp: Added.
> +	   (WebCore::CALLBACK_FUNC_DECL):
> +	   * bindings/v8/custom/V8LocationCustom.cpp: Added.
> +	   (WebCore::navigateIfAllowed):
> +	   (WebCore::ACCESSOR_SETTER):
> +	   (WebCore::ACCESSOR_GETTER):
> +	   (WebCore::CALLBACK_FUNC_DECL):
> +	   (WebCore::INDEXED_ACCESS_CHECK):
> +	   (WebCore::NAMED_ACCESS_CHECK):

Remove func lines here unless there's a useful comment to go with them.

> +    v8::Handle<v8::Value> holder = args.Holder();
> +    HTMLCanvasElement* imp =
V8Proxy::DOMWrapperToNode<HTMLCanvasElement>(holder);
> +    String contextId = ToWebCoreString(args[0]);
toWebCoreString

> +	   return sharedTemplate->GetFunction();
> +    } else {
> +	   return privateTemplate->GetFunction();
> +    }

No braces for one-liners.

> +	   return sharedTemplate->GetFunction();
> +    } else {
> +	   return privateTemplate->GetFunction();
> +    }

Ditto.

> +	   return sharedTemplate->GetFunction();
> +    } else {
> +	   return privateTemplate->GetFunction();
> +    }

Ditto.