Bug 89217 - Speech JavaScript API: SpeechRecognition should hook up with ActiveDOMObject more
Summary: Speech JavaScript API: SpeechRecognition should hook up with ActiveDOMObject ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hans Wennborg
URL:
Keywords:
Depends on:
Blocks: 80261
  Show dependency tree / graph
 
Reported: 2012-06-15 08:16 PDT by Hans Wennborg
Modified: 2012-06-21 03:23 PDT (History)
5 users (show)

See Also:

Attachments
Patch (4.08 KB, patch)
2012-06-15 08:18 PDT, Hans Wennborg 		no flags Details | Formatted Diff | Diff
Patch (16.65 KB, patch)
2012-06-18 10:15 PDT, Hans Wennborg 		no flags Details | Formatted Diff | Diff
Patch (23.59 KB, patch)
2012-06-19 08:43 PDT, Hans Wennborg 		no flags Details | Formatted Diff | Diff
Patch (23.54 KB, patch)
2012-06-20 02:29 PDT, Hans Wennborg 		abarth: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Hans Wennborg 2012-06-15 08:16:02 PDT 
Speech JavaScript API: SpeechRecognition should hook up with ActiveDOMObject more
Comment 1 Hans Wennborg 2012-06-15 08:18:53 PDT 
Created attachment 147826 [details]
Patch
Comment 2 Hans Wennborg 2012-06-15 08:19:46 PDT 
I'll change the Chromium plumbing in the next patch so that it doesn't keep a reference to the SpeechRecognitionObject.
Comment 3 Satish Sampath 2012-06-15 10:21:39 PDT 
Comment on attachment 147826 [details]
Patch

The change looks fine. Can we add a layout test, perhaps one that uses a child iframe and navigates in that to verify that speech recognition is aborted?
Comment 4 Hans Wennborg 2012-06-18 10:15:26 PDT 
(In reply to comment #3)
> (From update of attachment 147826 [details])
> The change looks fine. Can we add a layout test, perhaps one that uses a child iframe and navigates in that to verify that speech recognition is aborted?

Coming up. As usual, the test is way more complicated than the code :/
Comment 5 Hans Wennborg 2012-06-18 10:15:57 PDT 
Created attachment 148120 [details]
Patch
Comment 6 Hans Wennborg 2012-06-19 08:42:44 PDT 
After discussing with Satish, trying to rebuild the mock object a little bit so that there is no need for the setWaitAfterStart() function.

New patch coming up.
Comment 7 Hans Wennborg 2012-06-19 08:43:01 PDT 
Created attachment 148344 [details]
Patch
Comment 8 Satish Sampath 2012-06-19 13:46:10 PDT 
Comment on attachment 148344 [details]
Patch

The change itself looks good. It would be shorter to change all 'hasBeenAborted/hasXxxxxBeenAborted' to 'wasAborted' style :)
Comment 9 Hans Wennborg 2012-06-20 02:29:33 PDT 
(In reply to comment #8)
> (From update of attachment 148344 [details])
> The change itself looks good. It would be shorter to change all 'hasBeenAborted/hasXxxxxBeenAborted' to 'wasAborted' style :)
Done.
Comment 10 Hans Wennborg 2012-06-20 02:29:52 PDT 
Created attachment 148526 [details]
Patch
Comment 11 Hans Wennborg 2012-06-20 05:51:04 PDT 
Adam, would you like to take a look?
Comment 12 Adam Barth 2012-06-20 10:15:28 PDT 
Comment on attachment 148526 [details]
Patch

LGTM!
Comment 13 WebKit Review Bot 2012-06-21 02:53:56 PDT 
Comment on attachment 148526 [details]
Patch

Rejecting attachment 148526 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
eLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/fast/speech/scripted/navigate-away-expected.txt
patching file LayoutTests/fast/speech/scripted/navigate-away-iframe-expected.txt
patching file LayoutTests/fast/speech/scripted/navigate-away-iframe.html
patching file LayoutTests/fast/speech/scripted/navigate-away.html

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Adam Barth']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue/

Full output: http://queues.webkit.org/results/13030049
Comment 14 Hans Wennborg 2012-06-21 03:23:35 PDT 
Committed r120913: <http://trac.webkit.org/changeset/120913>