Summary: | WebSpeech: plumb through a method to generate fake speech jobs for testing | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | chris fleizach <cfleizach> | ||||||||||||||||
Component: | Accessibility | Assignee: | chris fleizach <cfleizach> | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | abarth, buildbot, dmazzoni, gyuyoung.kim, rakuco, rniwa, webkit-ews, webkit.review.bot | ||||||||||||||||
Priority: | P2 | ||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||
Bug Depends on: | |||||||||||||||||||
Bug Blocks: | 106742 | ||||||||||||||||||
Attachments: |
|
Description
chris fleizach
2013-01-18 17:46:52 PST
Created attachment 187796 [details]
patch
Comment on attachment 187796 [details] patch Attachment 187796 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/16493714 Comment on attachment 187796 [details] patch Attachment 187796 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/16454092 Created attachment 187881 [details]
patch
Adam and Dominic, what do you think of this approach to fake the speech synthesis output? The idea is have a WebCore::Settings() method that can be checked, and then the platform can do something to fake the simulation that the speaking job completed Comment on attachment 187881 [details]
patch
This isn't a very appealing approach to mocking out speak synthesis. It's generally better to cleanly separate mock code from production code. Ideally the mock code isn't linked into the production binary at all. In other places, we've mocked out subsystems at the Platform API boundary, but I understand that might not work well for ports like apple-mac that interact directly with the underlying operating system.
(In reply to comment #6) > (From update of attachment 187881 [details]) > This isn't a very appealing approach to mocking out speak synthesis. It's generally better to cleanly separate mock code from production code. Ideally the mock code isn't linked into the production binary at all. In other places, we've mocked out subsystems at the Platform API boundary, but I understand that might not work well for ports like apple-mac that interact directly with the underlying operating system. Do you think I should do something like MockPagePopup, (where we have a MockPlatformSynthesizer) and then it's enabled through an Internals setting like void setEnableMockPagePopup(in boolean enabled) raises(DOMException); Is the mock linked into the production binary? I'd much prefer not to link it into the production binary. (In reply to comment #8) > Is the mock linked into the production binary? I'd much prefer not to link it into the production binary. That one specifically is, but I see other Mocks*, like MockCDM that is only linked in WebCoreTestSupport, so perhaps I can do that as well If we can link it into WebCoreTestSupport, that's likely to be better. Created attachment 188237 [details]
patch
New patch uses a method in Internals to modify enter testing mode. Doing that will create a mock platform synthesizer. This mock synthesizer lives only in WebCoreSupport. Comment on attachment 188237 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=188237&action=review > Source/WebCore/Modules/speech/SpeechSynthesis.h:76 > - PlatformSpeechSynthesizer m_platformSpeechSynthesizer; > + RefPtr<PlatformSpeechSynthesizer> m_platformSpeechSynthesizer; RefPtr -> why not OwnPtr? Comment on attachment 188237 [details]
patch
The general approach looks fine, but you've only modified one of the N build systems used by WebKit.
(In reply to comment #14) > (From update of attachment 188237 [details]) > The general approach looks fine, but you've only modified one of the N build systems used by WebKit. Thanks for your feedback. Didn't want to do to most painful part of adding new files unless it looked like a good approach Yeah, this isn't the best design for ports that have a Platform API, but this is fine for ports like apple-mac that talk to the operating system directly. Created attachment 188367 [details]
patch
Comment on attachment 188367 [details] patch Attachment 188367 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/16580436 Created attachment 188440 [details]
patch
Attachment 188440 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/platform/mac/fast/speechsynthesis/speech-synthesis-speak.html', u'LayoutTests/platform/mac/fast/speechsynthesis/speech-synthesis-voices.html', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/Modules/speech/SpeechSynthesis.cpp', u'Source/WebCore/Modules/speech/SpeechSynthesis.h', u'Source/WebCore/Target.pri', u'Source/WebCore/WebCore.exp.in', u'Source/WebCore/WebCore.gypi', u'Source/WebCore/WebCore.vcproj/WebCoreTestSupport.vcproj', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/platform/PlatformSpeechSynthesizer.cpp', u'Source/WebCore/platform/PlatformSpeechSynthesizer.h', u'Source/WebCore/platform/mock/PlatformSpeechSynthesizerMock.cpp', u'Source/WebCore/platform/mock/PlatformSpeechSynthesizerMock.h', u'Source/WebCore/testing/Internals.cpp', u'Source/WebCore/testing/Internals.h', u'Source/WebCore/testing/Internals.idl']" exit_code: 1
Source/WebCore/platform/mock/PlatformSpeechSynthesizerMock.h:88: "PlatformSpeechSynthesizer.h" already included at Source/WebCore/platform/mock/PlatformSpeechSynthesizerMock.h:31 [build/include] [4]
Source/WebCore/platform/mock/PlatformSpeechSynthesizerMock.h:89: "Timer.h" already included at Source/WebCore/platform/mock/PlatformSpeechSynthesizerMock.h:32 [build/include] [4]
Source/WebCore/platform/mock/PlatformSpeechSynthesizerMock.h:90: "wtf/PassOwnPtr.h" already included at Source/WebCore/platform/mock/PlatformSpeechSynthesizerMock.h:33 [build/include] [4]
Source/WebCore/platform/mock/PlatformSpeechSynthesizerMock.cpp:99: "config.h" already included at Source/WebCore/platform/mock/PlatformSpeechSynthesizerMock.cpp:26 [build/include] [4]
Source/WebCore/platform/mock/PlatformSpeechSynthesizerMock.cpp:100: "PlatformSpeechSynthesizerMock.h" already included at Source/WebCore/platform/mock/PlatformSpeechSynthesizerMock.cpp:27 [build/include] [4]
Total errors found: 5 in 19 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 188447 [details]
patch
Comment on attachment 188447 [details] patch Attachment 188447 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/16584015 Created attachment 188491 [details]
patch
Adam, does this latest patch look ok to you? Comment on attachment 188491 [details]
patch
Yep. Looks good. Thanks!
|