[webkit-reviews] review denied: [Bug 85954] Web Intents code only supports V8 : [Attachment 140924] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Christophe Dumez
<christophe.dumez at intel.com>'s request for review:
Bug 85954: Web Intents code only supports V8
https://bugs.webkit.org/show_bug.cgi?id=85954

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=140924&action=review


> Source/WebCore/Modules/intents/Intent.cpp:41
> +static void getJSObjectPropertiesAsStringHashMap(ScriptState* scriptState,
JSC::JSObject* object, WTF::HashMap<String, String>& map)

This isn't correct.  JSC-specific code should be in WebCore/bindings/js

> Source/WebCore/Modules/intents/Intent.cpp:135
> +#if USE(V8)
>      Dictionary extrasDictionary;
>      if (options.get("extras", extrasDictionary))
>	   extrasDictionary.getOwnPropertiesAsStringHashMap(extras);
> +#else
> +    ScriptValue extraValue;
> +    if (options.get("extras", extraValue))
> +	   if (extraValue.isObject())
> +	       getJSObjectPropertiesAsStringHashMap(scriptState,
extraValue.jsValue().toObject(scriptState), extras);
> +#endif

All these ifdefs are wrong.  The correct approach is to implement any missing
script abstractions for JSC.