Bug 107351 - WebSpeech: plumb through a method to generate fake speech jobs for testing
Summary: WebSpeech: plumb through a method to generate fake speech jobs for testing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: chris fleizach
URL:
Keywords:
Depends on:
Blocks: 106742
  Show dependency treegraph
 
Reported: 2013-01-18 17:46 PST by chris fleizach
Modified: 2013-02-17 13:02 PST (History)
8 users (show)

See Also:


Attachments
patch (18.15 KB, patch)
2013-02-12 00:38 PST, chris fleizach
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
patch (21.82 KB, patch)
2013-02-12 09:10 PST, chris fleizach
abarth: review-
Details | Formatted Diff | Diff
patch (25.52 KB, patch)
2013-02-13 17:50 PST, chris fleizach
abarth: review-
Details | Formatted Diff | Diff
patch (31.94 KB, patch)
2013-02-14 09:06 PST, chris fleizach
eflews.bot: commit-queue-
Details | Formatted Diff | Diff
patch (36.62 KB, patch)
2013-02-14 15:59 PST, chris fleizach
no flags Details | Formatted Diff | Diff
patch (31.33 KB, patch)
2013-02-14 16:19 PST, chris fleizach
buildbot: commit-queue-
Details | Formatted Diff | Diff
patch (31.48 KB, patch)
2013-02-14 23:40 PST, chris fleizach
abarth: review+
cfleizach: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description chris fleizach 2013-01-18 17:46:52 PST
We need to be able to tell DRT that we want to send fake speech jobs, then get the system to essentially fake up the speech job. we don't want real speech jobs to be sent lest they cause strange output on a persons system who's running layout tests
Comment 1 chris fleizach 2013-02-12 00:38:31 PST
Created attachment 187796 [details]
patch
Comment 2 Early Warning System Bot 2013-02-12 00:51:32 PST
Comment on attachment 187796 [details]
patch

Attachment 187796 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/16493714
Comment 3 EFL EWS Bot 2013-02-12 01:00:22 PST
Comment on attachment 187796 [details]
patch

Attachment 187796 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/16454092
Comment 4 chris fleizach 2013-02-12 09:10:18 PST
Created attachment 187881 [details]
patch
Comment 5 chris fleizach 2013-02-12 11:37:46 PST
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 6 Adam Barth 2013-02-12 12:25:14 PST
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.
Comment 7 chris fleizach 2013-02-12 14:07:07 PST
(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);
Comment 8 Adam Barth 2013-02-12 14:26:27 PST
Is the mock linked into the production binary?  I'd much prefer not to link it into the production binary.
Comment 9 chris fleizach 2013-02-12 14:28:19 PST
(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
Comment 10 Adam Barth 2013-02-12 15:14:42 PST
If we can link it into WebCoreTestSupport, that's likely to be better.
Comment 11 chris fleizach 2013-02-13 17:50:59 PST
Created attachment 188237 [details]
patch
Comment 12 chris fleizach 2013-02-13 17:52:19 PST
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 13 Adam Barth 2013-02-13 17:53:17 PST
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 14 Adam Barth 2013-02-13 17:59:05 PST
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.
Comment 15 chris fleizach 2013-02-13 18:03:34 PST
(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
Comment 16 Adam Barth 2013-02-13 18:07:23 PST
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.
Comment 17 chris fleizach 2013-02-14 09:06:54 PST
Created attachment 188367 [details]
patch
Comment 18 EFL EWS Bot 2013-02-14 09:18:39 PST
Comment on attachment 188367 [details]
patch

Attachment 188367 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/16580436
Comment 19 chris fleizach 2013-02-14 15:59:20 PST
Created attachment 188440 [details]
patch
Comment 20 WebKit Review Bot 2013-02-14 16:03:20 PST
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.
Comment 21 chris fleizach 2013-02-14 16:19:26 PST
Created attachment 188447 [details]
patch
Comment 22 Build Bot 2013-02-14 20:42:33 PST
Comment on attachment 188447 [details]
patch

Attachment 188447 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16584015
Comment 23 chris fleizach 2013-02-14 23:40:21 PST
Created attachment 188491 [details]
patch
Comment 24 chris fleizach 2013-02-17 00:46:37 PST
Adam, does this latest patch look ok to you?
Comment 25 Adam Barth 2013-02-17 09:24:27 PST
Comment on attachment 188491 [details]
patch

Yep.  Looks good.  Thanks!
Comment 26 chris fleizach 2013-02-17 13:02:10 PST
http://trac.webkit.org/changeset/143136