[Chromium] Allow building without speech input enabled.
Created attachment 110569 [details] Patch
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Comment on attachment 110569 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=110569&action=review > Source/WebKit/chromium/public/WebSpeechInputListener.h:34 > +#if ENABLE_INPUT_SPEECH we don't put ENABLE_ options in WebKit API headers. chromium doesn't see config.h. these should only be used in the .cpp files to protect against using WebCore classes and functions that are only defined under ENABLE_INPUT_SPEECH.
Created attachment 110610 [details] Patch
I propose to remove the #if ENABLE(INPUT_SPEECH) guards from the WebCore headers so that WebKit/chromium/src/WebSpeechInputControllerMockImpl.h header file can compile as it inherits from WebCore::SpeechInputListener. An alternative would be to put the guards into the WebKit/chromium/src/WebSpeechInputControllerMockImpl.h file (perhaps ok, since it's in the src directory?)
(In reply to comment #5) > I propose to remove the #if ENABLE(INPUT_SPEECH) guards from the WebCore headers so that WebKit/chromium/src/WebSpeechInputControllerMockImpl.h header file can compile as it inherits from WebCore::SpeechInputListener. > > An alternative would be to put the guards into the WebKit/chromium/src/WebSpeechInputControllerMockImpl.h file (perhaps ok, since it's in the src directory?) Yes, you can use ENABLE(FOO) in chromium/src/. You just have to hide ENABLE(FOO) from the consumer of the API since they don't get to see config.h.
Comment on attachment 110610 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=110610&action=review > Source/WebCore/page/SpeechInputListener.h:-34 > -#if ENABLE(INPUT_SPEECH) I don't understand why you are removing these guards. Don't we want to prevent this code from being seen when people are not interested in building with INPUT_SPEECH enabled? > Tools/DumpRenderTree/chromium/LayoutTestController.cpp:90 > +#if ENABLE(INPUT_SPEECH) I would not expect to see any DumpRenderTree changes since DumpRenderTree just consumes WebKit via the WebKit API, and the API entrypoints exist regardless of the value of ENABLE(INPUT_SPEECH).
Created attachment 110732 [details] Patch
Thanks again for review, Darin. This patch leaves the WebCore files alone. I still needed to make changes to DumpRenderTree because when INPUT_SPEECH is disabled, I cannot call WebSpeechInputControllerMock::create to create the mock since this is provided by WebKit/chromium/src/WebSpeechInputControllerMockImpl.cpp which is compiled out. I think I can avoid the changes to DumpRenderTree completely if I provide an implementation of WebSpeechInputControllerMock::create (for INPUT_SPEECH disabled) that would yield a dummy mock object that ignores method invocations. Would this be preferable?
Another approach would be make WebSpeechInputControllerMock::create return NULL for INPUT_SPEECH disabled.
(In reply to comment #10) > Another approach would be make WebSpeechInputControllerMock::create return NULL for INPUT_SPEECH disabled. Yes, you should do that.
Created attachment 110768 [details] Patch
Looks good, thanks for this change John.
Comment on attachment 110768 [details] Patch Clearing flags on attachment: 110768 Committed r97379: <http://trac.webkit.org/changeset/97379>
All reviewed patches have been landed. Closing bug.