Bug 80019

Summary: Speech JavaScript API: compile- and runtime flags
Product: WebKit Reporter: Hans Wennborg <hans>
Component: New BugsAssignee: Hans Wennborg <hans>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, arun.patole, cc-bugs, cgarcia, donggwan.kim, fishd, gns, jamesr, japhet, levin+threading, macpherson, menard, ojan, peter, rakuco, satish, tkent, vestbo, webkit.review.bot, zan, zoltan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 80261    
Attachments:
Description Flags
Patch
none
Patch none

Description Hans Wennborg 2012-03-01 04:03:56 PST
Implement Speech JavaScript API
Comment 1 Hans Wennborg 2012-03-01 04:11:51 PST
Created attachment 129679 [details]
Patch
Comment 2 WebKit Review Bot 2012-03-01 04:15:33 PST
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 3 Hans Wennborg 2012-03-01 05:53:57 PST
Created attachment 129689 [details]
Patch

Let's add the compile- and runtime flags first to make this more reviewable.
Comment 4 WebKit Review Bot 2012-03-01 05:57:44 PST
Attachment 129689 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." exit_code: 1
WARNING: File exempt from style guide. Skipping: "Source/WebKit2/UIProcess/API/gtk/WebKitDefines.h"
Source/WebKit2/WebProcess/gtk/WebProcessMainGtk.cpp:71:  Use 0 instead of NULL.  [readability/null] [5]
Total errors found: 1 in 166 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Adam Barth 2012-03-03 15:34:12 PST
Comment on attachment 129689 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=129689&action=review

> Source/WebCore/bindings/generic/RuntimeEnabledFeatures.h:157
> +#if ENABLE(SCRIPTED_SPEECH)
> +    static void setScriptedSpeechEnabled(bool isEnabled) { isScriptedSpeechEnabled = isEnabled; }
> +    static bool scriptedSpeechEnabled() { return isScriptedSpeechEnabled; }
> +    static bool webkitSpeechRecognitionEnabled() { return isScriptedSpeechEnabled; }
> +    static bool webkitSpeechRecognitionErrorEnabled() { return isScriptedSpeechEnabled; }
> +    static bool webkitSpeechGrammarEnabled() { return isScriptedSpeechEnabled; }
> +    static bool webkitSpeechGrammarListEnabled() { return isScriptedSpeechEnabled; }
> +#endif

This change is slightly ahead of what you need in this patch.
Comment 6 Satish Sampath 2012-03-03 16:27:10 PST
Comment on attachment 129689 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=129689&action=review

Tools/Scripts/build-webkit: line 98
> $scriptedSpeechSupport

I see the rest of this list is alphabetically sorted and I understand $scriptedSpeechSupport is placed next to $inputSpeechSupport. But since in the long term $inputSpeechSupport should be going away so it may make sense to insert $scriptedSpeechSupport in the right alphabetical order instead of the current location.

Same for ENABLE_SCRIPTED_SPEECH in lines 232-234

>> Source/WebCore/bindings/generic/RuntimeEnabledFeatures.h:157
>> +#endif
> 
> This change is slightly ahead of what you need in this patch.

I think Hans's idea was to have this patch start both the compile time and runtime flags for the feature. In that sense may be the first 2 in the above list make sense but the rest could be added in subsequent patches where they are required.
Comment 7 Adam Barth 2012-03-03 17:00:34 PST
It's not a big deal either way.
Comment 8 WebKit Review Bot 2012-03-03 17:06:25 PST
Comment on attachment 129689 [details]
Patch

Clearing flags on attachment: 129689

Committed r109667: <http://trac.webkit.org/changeset/109667>
Comment 9 WebKit Review Bot 2012-03-03 17:06:32 PST
All reviewed patches have been landed.  Closing bug.