[Webkit-unassigned] [Bug 43477] Add speech input controller mock in WebKit and a layout test.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 4 07:37:40 PDT 2010


https://bugs.webkit.org/show_bug.cgi?id=43477





--- Comment #6 from Satish Sampath <satish at chromium.org>  2010-08-04 07:37:40 PST ---
(In reply to comment #4)

Addressed all comments, some replies below:

> (From update of attachment 63441 [details])
> WebKitTools/DumpRenderTree/chromium/LayoutTestController.h:46
>  +  #include "public/WebSpeechInputControllerMock.h"
> Are you sure you need to do the include in this file rather than forward declare?

Used forward declaration now, this required TestShell.cpp to also include the mock header file as it constructs the LayoutTestController object.

> WebKit/chromium/public/WebSpeechInputControllerMock.h:45
>  +      virtual void setMockRecognitionResult(const WebString& result) = 0;
> Why make this abstract/virtual?

since it is an interface and the implementation lives in chromium/src/WebSpeechInputControllerMockImpl.h

> WebKitTools/DumpRenderTree/chromium/LayoutTestController.cpp:1381
>  +      m_speechInputControllerMock->setMockRecognitionResult(cppVariantToWebString(arguments[0]));
> Are you sure m_speechInputControllerMock will always be created in time?  I'm not sure it will.

WebKit::WebViewImpl creates WebKit::SpeechInputClientImpl during construction, which calls the DRT WebViewClient for the speech input controller. The mock gets created in this call and returned to WebKit. So by the time the page loads, the mock is valid and available.

> WebKit/chromium/src/WebSpeechInputControllerMockImpl.cpp:64
>  +      WebCore::SpeechInputClientMock m_webcoreMock;
> Hmm....usually we use an OwnPtr, but I don't have a good reason why you'd need to.

Made to an OwnPtr now since the class declaration has been moved to impl.h.

> WebKit/chromium/src/WebSpeechInputControllerMockImpl.cpp:44
>  +  class WebSpeechInputControllerMockImpl
> I'd slightly lean towards moving this impl stuff to a header file.  It just seems weird to have an Impl.cpp whose corresponding .h is of a different name and I don't know of any other similar examples.

Moved class declaration to impl.h

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list