Speech JavaScript API: mock WebSpeechRecognizer for DumpRenderTree
Created attachment 145094 [details] Patch
Can you implement the mock in Source/WebCore/testing/ ? Implementing a Chromium-specific mock is not great for code sharing.
In https://bugs.webkit.org/show_bug.cgi?id=76443#c22 it was questioned why the mock for speech input was living in WebCore, and it was suggested that the mock should just be part of DumpRenderTree, which is why I moved it there. That also means we didn't have to have a bunch of wrappers around it. This new mock is modeled on that previous MockWebSpeechInputController.
(In reply to comment #3) > In https://bugs.webkit.org/show_bug.cgi?id=76443#c22 it was questioned why the mock for speech input was living in WebCore, and it was suggested that the mock should just be part of DumpRenderTree, which is why I moved it there. That also means we didn't have to have a bunch of wrappers around it. > > This new mock is modeled on that previous MockWebSpeechInputController. It seems fishd wasn't negative about putting a mock to WebCore/testing/. It's ok to have mocks in DRT/chromium/, but I think putting mocks to WebCore/testing/ is better.
(In reply to comment #4) > (In reply to comment #3) > > In https://bugs.webkit.org/show_bug.cgi?id=76443#c22 it was questioned why the mock for speech input was living in WebCore, and it was suggested that the mock should just be part of DumpRenderTree, which is why I moved it there. That also means we didn't have to have a bunch of wrappers around it. > > > > This new mock is modeled on that previous MockWebSpeechInputController. > > It seems fishd wasn't negative about putting a mock to WebCore/testing/. > It's ok to have mocks in DRT/chromium/, but I think putting mocks to WebCore/testing/ is better. I don't see any full-blown mocks in WebCore/testing/, it looks more like hooks into different WebCore objects. There are some client mocks in WebCore/platform/mock/. For example, that's where the SpeechInputController mock lived before it was moved into DRT. The problem with these mocks is that the embedder is supposed to pass them in when creating the Page object. In Chromium's case, this means we have to wrap it. In this case, I think the overhead of the wrapping stuff would become more code than the mock itself. If it's OK to have the mock in DRT/chromium, I would prefer that. It's much simpler, and it matches what we do for MockWebSpeechInputController.
About the offline request to use the copyright header from Tools/DumpRenderTree/chromium/DumpRenderTree.cpp instead: I believe that's actually the old copyright header, that we're not using for new code anymore. I believe the one I used in my patch is the correct one. (See e.g. jamesr's comment in https://bugs.webkit.org/show_bug.cgi?id=66008#c12)
Comment on attachment 145094 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=145094&action=review > Tools/DumpRenderTree/chromium/MockWebSpeechRecognizer.cpp:94 > + WebVector<WebString> transcripts(static_cast<size_t>(1)); > + WebVector<float> confidences(static_cast<size_t>(1)); Do we need static_cast<>? Doesn't 1 or 1u work? > Tools/DumpRenderTree/chromium/MockWebSpeechRecognizer.h:94 > + // Task class for calling a client function that does not take any parameters. > + typedef void (WebKit::WebSpeechRecognizerClient::*ClientFunctionPointer)(const WebKit::WebSpeechRecognitionHandle&); > + class ClientCallTask : public MethodTask<MockWebSpeechRecognizer> { > + public: > + ClientCallTask(MockWebSpeechRecognizer* mock, ClientFunctionPointer function) : MethodTask<MockWebSpeechRecognizer>(mock), m_function(function) { } > + virtual void runIfValid() OVERRIDE { (m_object->m_client->*m_function)(m_object->m_handle); } > + > + private: > + ClientFunctionPointer m_function; > + }; > + > + // Task for delivering a result event. > + class ResultTask : public MethodTask<MockWebSpeechRecognizer> { > + public: > + ResultTask(MockWebSpeechRecognizer* mock, const WebKit::WebString transcript, float confidence) : MethodTask<MockWebSpeechRecognizer>(mock), m_transcript(transcript), m_confidence(confidence) { } > + virtual void runIfValid() OVERRIDE; > + > + private: > + WebKit::WebString m_transcript; > + float m_confidence; > + }; > + > + // Task for delivering a nomatch event. > + class NoMatchTask : public MethodTask<MockWebSpeechRecognizer> { > + public: > + NoMatchTask(MockWebSpeechRecognizer* mock) : MethodTask<MockWebSpeechRecognizer>(mock) { } > + virtual void runIfValid() OVERRIDE { m_object->m_client->didReceiveNoMatch(m_object->m_handle, WebKit::WebSpeechRecognitionResult()); } > + }; Can you move them to MockWebSpeechRecognizer.cpp?
(In reply to comment #7) > (From update of attachment 145094 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=145094&action=review > > > Tools/DumpRenderTree/chromium/MockWebSpeechRecognizer.cpp:94 > > + WebVector<WebString> transcripts(static_cast<size_t>(1)); > > + WebVector<float> confidences(static_cast<size_t>(1)); > > Do we need static_cast<>? Doesn't 1 or 1u work? Yes, they're needed. If I try 1U or 1, the compiler will try to use the "template <typename C> WebVector(const C& other)" constructor: WebVector.h:84:29: error: member reference base type 'const unsigned int' is not a structure or union initializeFrom(other.size() ? &other[0] : 0, other.size()); ~~~~~^~~~~ Tools/DumpRenderTree/chromium/MockWebSpeechRecognizer.cpp:93:26: note: in instantiation of function template specialization 'WebKit::WebVector<WebKit::WebString>::WebVector<unsigned int>' requested here WebVector<WebString> transcripts(1U); ^ > > Tools/DumpRenderTree/chromium/MockWebSpeechRecognizer.h:94 > > + // Task class for calling a client function that does not take any parameters. > > + typedef void (WebKit::WebSpeechRecognizerClient::*ClientFunctionPointer)(const WebKit::WebSpeechRecognitionHandle&); > > + class ClientCallTask : public MethodTask<MockWebSpeechRecognizer> { > > + public: > > + ClientCallTask(MockWebSpeechRecognizer* mock, ClientFunctionPointer function) : MethodTask<MockWebSpeechRecognizer>(mock), m_function(function) { } > > + virtual void runIfValid() OVERRIDE { (m_object->m_client->*m_function)(m_object->m_handle); } > > + > > + private: > > + ClientFunctionPointer m_function; > > + }; > > + > > + // Task for delivering a result event. > > + class ResultTask : public MethodTask<MockWebSpeechRecognizer> { > > + public: > > + ResultTask(MockWebSpeechRecognizer* mock, const WebKit::WebString transcript, float confidence) : MethodTask<MockWebSpeechRecognizer>(mock), m_transcript(transcript), m_confidence(confidence) { } > > + virtual void runIfValid() OVERRIDE; > > + > > + private: > > + WebKit::WebString m_transcript; > > + float m_confidence; > > + }; > > + > > + // Task for delivering a nomatch event. > > + class NoMatchTask : public MethodTask<MockWebSpeechRecognizer> { > > + public: > > + NoMatchTask(MockWebSpeechRecognizer* mock) : MethodTask<MockWebSpeechRecognizer>(mock) { } > > + virtual void runIfValid() OVERRIDE { m_object->m_client->didReceiveNoMatch(m_object->m_handle, WebKit::WebSpeechRecognitionResult()); } > > + }; > > Can you move them to MockWebSpeechRecognizer.cpp? Done.
Created attachment 146523 [details] Patch
Comment on attachment 146523 [details] Patch ok
Comment on attachment 146523 [details] Patch Clearing flags on attachment: 146523 Committed r119818: <http://trac.webkit.org/changeset/119818>
All reviewed patches have been landed. Closing bug.