[webkit-reviews] review granted: [Bug 217780] Add stubs for SpeechRecognition : [Attachment 411883] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Sihui Liu <sihui_liu at apple.com>'s
request for review:
Bug 217780: Add stubs for SpeechRecognition
https://bugs.webkit.org/show_bug.cgi?id=217780

Attachment 411883: Patch

https://bugs.webkit.org/attachment.cgi?id=411883&action=review




--- Comment #29 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 411883
  --> https://bugs.webkit.org/attachment.cgi?id=411883
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=411883&action=review

> Source/WebCore/Modules/speech/SpeechRecognition.h:41
> +    void setLang(const String& lang) { m_lang = lang; }

String&& probably and WFMOve

> Source/WebCore/Modules/speech/SpeechRecognition.h:60
> +    SpeechRecognition(Document&);

explicit

> Source/WebCore/Modules/speech/SpeechRecognitionAlternative.cpp:41
> +    : m_transcript(transcript)

WTFMove()

> Source/WebCore/Modules/speech/SpeechRecognitionAlternative.h:39
> +    String transcript() const { return m_transcript; }

const String&?

> Source/WebCore/Modules/speech/SpeechRecognitionAlternative.h:43
> +    explicit SpeechRecognitionAlternative(String&& transcript, double
confidence);

explicit not needed

> Source/WebCore/Modules/speech/SpeechRecognitionErrorEvent.cpp:35
> +Ref<SpeechRecognitionErrorEvent> SpeechRecognitionErrorEvent::create(const
AtomString& type, const Init& init, IsTrusted isTrusted)

Could probably have Init&& to be able to move m_message

> Source/WebCore/Modules/speech/SpeechRecognitionErrorEvent.h:45
> +    String message() const { return m_message; }

const String&?

> Source/WebCore/Modules/speech/SpeechRecognitionErrorEvent.h:51
> +    EventInterface eventInterface() const override;

final

> Source/WebCore/Modules/speech/SpeechRecognitionEvent.cpp:46
> +SpeechRecognitionEvent::SpeechRecognitionEvent(const AtomString& type, const
Init& init, IsTrusted isTrusted)

Init&&

> Source/WebCore/Modules/speech/SpeechRecognitionEvent.cpp:49
> +    , m_results(init.results)

WTFMove()

> Source/WebCore/Modules/speech/SpeechRecognitionEvent.h:52
> +    EventInterface eventInterface() const override;

final

> Source/WebCore/Modules/speech/SpeechRecognitionEvent.h:55
> +    RefPtr<SpeechRecognitionResultList> m_results;

Can it be a Ref<>? It seems results is required so we could mandate it even
though the struct is a RefPtr.

> Source/WebCore/Modules/speech/SpeechRecognitionResultList.h:48
> +    SpeechRecognitionResultList(Vector<Ref<SpeechRecognitionResult>>&&);

explicit

> Source/WebCore/Modules/webaudio/AudioWorkletNode.h:35
> +// #include <JavaScriptCore/JSGlobalObject.h>

Not needed

> Source/WebKitLegacy/mac/WebView/WebPreferencesPrivate.h:351
> + at property (nonatomic) BOOL speechRecognitionEnabled;

Should it be _speechRecognitionEnabled?

> Tools/DumpRenderTree/TestOptions.h:67
> +    bool enableSpeechRecognition { true };

Unneeded?