[webkit-reviews] review denied: [Bug 25739] Upstream V8 bindings for HTMLOptionsCollection and HTMLSelectElementCollection : [Attachment 30291] patch3

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has denied Nate Chapin
<japhet at google.com>'s request for review:
Bug 25739: Upstream V8 bindings for HTMLOptionsCollection and
HTMLSelectElementCollection
https://bugs.webkit.org/show_bug.cgi?id=25739

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

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
Don't rush :)

> +	   * bindings/v8/custom/V8ElementCustom.cpp:
> +	   (WebCore::ACCESSOR_SETTER): Moved from v8_custom.cpp.

That doesn't sound right. We haven't moved anything like this here.

> -#include "HTMLOptionsCollection.h"
> +#include "V8HTMLOptionsCollectionCustom.h"

be careful here. There is no V8HTMLOptionsCollectionCustom in this CL.
  
> +INDEXED_PROPERTY_SETTER(HTMLOptionsCollection)
> +{
> +    INC_STATS("DOM.HTMLOptionsCollection.IndexedPropertySetter");
> +    HTMLOptionsCollection* collection =
V8Proxy::ToNativeObject<HTMLOptionsCollection>(V8ClassIndex::HTMLOPTIONSCOLLECT
ION, info.Holder());
> +    HTMLSelectElement* base =
static_cast<HTMLSelectElement*>(collection->base());
> +    return getOptionsCollectionSetter(index, value, base);

toOptions...

> +    HTMLSelectElement* select =
V8Proxy::DOMWrapperToNode<HTMLSelectElement>(info.Holder());
> +    return getOptionsCollectionSetter(index, value, select);

toOptions ..