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

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has granted 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 135316: Patch
https://bugs.webkit.org/attachment.cgi?id=135316&action=review

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


> Source/WebKit/chromium/public/WebSpeechRecognizerClient.h:39
> +public:

this interface should probably have a protected destructor to ensure
that the WebSpeechRecognizer implementation does not try to delete
its client.

> Source/WebKit/chromium/src/SpeechRecognitionClientProxy.cpp:106
> +    RefPtr<SpeechRecognition> recognition =
static_cast<PassRefPtr<SpeechRecognition> >(handle);

OK... looking around the codebase, the more common pattern in cases like this
is to do:

  RefPtr<SpeechRecognition> recognition =
PassRefPtr<SpeechRecognition>(handle);

^^^ slightly less verbosity

It seems like it would be cool if we had a global unwrap function that somehow
did the right thing here.

  unwrap<SpeechRecognition>(handle)->didEndAudio()

But, such a change would probably be a bigger effort.  I'd say it seems
out-of-scope
for this patch.