[webkit-reviews] review denied: [Bug 81667] Speech JavaScript API: Plumbing for Chromium : [Attachment 134578] Patch

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Hans Wennborg
<hans at chromium.org>'s request for review:
Bug 81667: Speech JavaScript API: Plumbing for Chromium
https://bugs.webkit.org/show_bug.cgi?id=81667

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

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=134578&action=review


> Source/WebKit/chromium/public/WebSpeechRecognitionClient.h:60
> +    WEBKIT_EXPORT bool equals(const WebSpeechRecognitionClient&) const;

I think it probably makes more sense for this to be an interface and for the
recognizer to be instantiated each time you need to start a speech recognition
task.

> Source/WebKit/chromium/public/WebSpeechRecognitionParameters.h:39
> +    WebSpeechRecognitionParameters(const WebVector<WebSpeechGrammar>&, const
WebString& language, bool continuous);

nit: elsewhere, we use a suffix of "Params"

> Source/WebKit/chromium/public/WebSpeechRecognitionParameters.h:41
> +    WEBKIT_EXPORT const WebVector<WebSpeechGrammar>& grammars() const;

nit: here you can just define these as inline getters since they just simply
return member variables.

> Source/WebKit/chromium/public/WebViewClient.h:321
> +    virtual WebSpeechRecognizer* speechRecognizer() { return 0; }

I really like the WebSpeechRecognizer name!  However, maybe this should return
an instance and be named createSpeechRecognizer?

Maybe WebSpeechRecogitionClient should be named WebSpeechRecognizerClient, and
it should also be an interface, which gets passed to createSpeechRecognizer.