Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(246)

Issue 315133004: Enable Oilpan by default in modules/speech/ (Closed)

Created:
6 years, 6 months ago by sof
Modified:
6 years, 6 months ago
CC:
blink-reviews, jamesr, dglazkov+blink, abarth-chromium
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Patch Set 1 #

Patch Set 2 : Use DEFINE_EVENT_TARGET_REFCOUNTING() #

Patch Set 3 : Non-Oilpan compile fixes #

Patch Set 4 : Events aren't on the heap by default yet; adjust member decl. #

Total comments: 3

Patch Set 5 : Remove unused Dictionary support for SpeechRecognitionError + fix Speech*Event create() types. #

Total comments: 7

Patch Set 6 : Stop using consts in conversion ctors + copy assignment ops #

Patch Set 7 : Rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+118 lines, -161 lines) Patch
M Source/bindings/v8/Dictionary.h View 1 2 3 4 2 chunks +2 lines, -4 lines 0 comments Download
M Source/bindings/v8/Dictionary.cpp View 1 2 3 4 4 chunks +2 lines, -14 lines 0 comments Download
M Source/modules/speech/DOMWindowSpeechSynthesis.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/speech/SpeechGrammar.h View 1 chunk +3 lines, -4 lines 0 comments Download
M Source/modules/speech/SpeechGrammar.cpp View 1 chunk +4 lines, -4 lines 0 comments Download
M Source/modules/speech/SpeechGrammar.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/speech/SpeechGrammarList.h View 2 chunks +3 lines, -5 lines 0 comments Download
M Source/modules/speech/SpeechGrammarList.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/modules/speech/SpeechGrammarList.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/speech/SpeechRecognition.h View 1 2 3 4 5 6 5 chunks +8 lines, -12 lines 0 comments Download
M Source/modules/speech/SpeechRecognition.cpp View 1 2 3 4 5 6 2 chunks +6 lines, -6 lines 0 comments Download
M Source/modules/speech/SpeechRecognition.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/speech/SpeechRecognitionAlternative.h View 1 chunk +2 lines, -3 lines 0 comments Download
M Source/modules/speech/SpeechRecognitionAlternative.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/modules/speech/SpeechRecognitionAlternative.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/speech/SpeechRecognitionError.h View 1 2 3 4 1 chunk +0 lines, -1 line 0 comments Download
M Source/modules/speech/SpeechRecognitionEvent.h View 1 2 3 4 4 chunks +5 lines, -6 lines 0 comments Download
M Source/modules/speech/SpeechRecognitionEvent.cpp View 1 2 3 4 4 chunks +5 lines, -6 lines 0 comments Download
M Source/modules/speech/SpeechRecognitionResult.h View 1 2 2 chunks +4 lines, -9 lines 0 comments Download
M Source/modules/speech/SpeechRecognitionResult.cpp View 2 chunks +3 lines, -3 lines 0 comments Download
M Source/modules/speech/SpeechRecognitionResult.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/speech/SpeechRecognitionResultList.h View 2 chunks +4 lines, -6 lines 0 comments Download
M Source/modules/speech/SpeechRecognitionResultList.cpp View 2 chunks +3 lines, -3 lines 0 comments Download
M Source/modules/speech/SpeechRecognitionResultList.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/speech/SpeechSynthesis.h View 1 2 3 4 5 6 3 chunks +6 lines, -9 lines 0 comments Download
M Source/modules/speech/SpeechSynthesis.cpp View 1 2 3 4 5 6 3 chunks +3 lines, -7 lines 0 comments Download
M Source/modules/speech/SpeechSynthesis.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/speech/SpeechSynthesisEvent.h View 1 2 3 4 2 chunks +4 lines, -2 lines 0 comments Download
M Source/modules/speech/SpeechSynthesisEvent.cpp View 1 2 3 4 2 chunks +1 line, -6 lines 0 comments Download
M Source/modules/speech/SpeechSynthesisUtterance.h View 1 2 3 4 5 6 2 chunks +4 lines, -5 lines 0 comments Download
M Source/modules/speech/SpeechSynthesisUtterance.cpp View 1 2 3 4 5 6 1 chunk +2 lines, -2 lines 0 comments Download
M Source/modules/speech/SpeechSynthesisUtterance.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/speech/SpeechSynthesisVoice.h View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/modules/speech/SpeechSynthesisVoice.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/modules/speech/SpeechSynthesisVoice.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/web/SpeechRecognitionClientProxy.cpp View 1 chunk +13 lines, -13 lines 0 comments Download
M Source/web/WebSpeechGrammar.cpp View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
M Source/web/WebSpeechRecognitionHandle.cpp View 1 2 3 4 5 1 chunk +3 lines, -3 lines 0 comments Download
M Source/web/WebSpeechRecognitionResult.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M public/web/WebSpeechGrammar.h View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
M public/web/WebSpeechRecognitionHandle.h View 1 2 3 4 5 1 chunk +3 lines, -3 lines 0 comments Download
M public/web/WebSpeechRecognitionResult.h View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 44 (0 generated)
sof
Please take a look. (Will of course ask for go-ahead from owners & other stakeholders.. ...
6 years, 6 months ago (2014-06-05 09:59:13 UTC) #1
haraken
LGTM in terms of oilpan. Let's wait for an approval of speech experts. https://codereview.chromium.org/315133004/diff/60001/Source/modules/speech/SpeechRecognitionEvent.cpp File ...
6 years, 6 months ago (2014-06-05 14:30:28 UTC) #2
sof
I went one step too far with the speech events, forgetting that Event still needs ...
6 years, 6 months ago (2014-06-05 16:30:43 UTC) #3
dmazzoni
lgtm for speech synthesis, I don't know the recognition code. https://codereview.chromium.org/315133004/diff/70001/Source/web/SpeechRecognitionClientProxy.cpp File Source/web/SpeechRecognitionClientProxy.cpp (right): https://codereview.chromium.org/315133004/diff/70001/Source/web/SpeechRecognitionClientProxy.cpp#newcode79 ...
6 years, 6 months ago (2014-06-05 23:43:07 UTC) #4
haraken
LGTM. https://codereview.chromium.org/315133004/diff/70001/Source/modules/speech/SpeechSynthesis.h File Source/modules/speech/SpeechSynthesis.h (right): https://codereview.chromium.org/315133004/diff/70001/Source/modules/speech/SpeechSynthesis.h#newcode89 Source/modules/speech/SpeechSynthesis.h:89: OwnPtr<PlatformSpeechSynthesizer> m_platformSpeechSynthesizer; PlatformSpeechSynthesizer would be another target to ...
6 years, 6 months ago (2014-06-06 01:11:16 UTC) #5
sof
https://codereview.chromium.org/315133004/diff/70001/Source/web/SpeechRecognitionClientProxy.cpp File Source/web/SpeechRecognitionClientProxy.cpp (right): https://codereview.chromium.org/315133004/diff/70001/Source/web/SpeechRecognitionClientProxy.cpp#newcode79 Source/web/SpeechRecognitionClientProxy.cpp:79: SpeechRecognition* recognition(handle); On 2014/06/05 23:43:06, dmazzoni wrote: > This ...
6 years, 6 months ago (2014-06-06 06:21:09 UTC) #6
sof
+tommyw: would you be able to look at the speech recognition specific parts? (see https://groups.google.com/a/chromium.org/forum/#!topic/oilpan-reviews/1JuCGgqDrXQ ...
6 years, 6 months ago (2014-06-06 07:25:59 UTC) #7
haraken
> +tommyw: would you be able to look at the speech recognition specific parts? > ...
6 years, 6 months ago (2014-06-06 07:28:07 UTC) #8
sof
thanks, good to know. +tommi: would you be able to look at the speech recognition ...
6 years, 6 months ago (2014-06-06 07:44:46 UTC) #9
tommi (sloooow) - chröme
lgtm
6 years, 6 months ago (2014-06-06 10:06:17 UTC) #10
sof
The CQ bit was checked by sigbjornf@opera.com
6 years, 6 months ago (2014-06-06 12:39:22 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sigbjornf@opera.com/315133004/70001
6 years, 6 months ago (2014-06-06 12:39:49 UTC) #12
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: blink_presubmit on tryserver.blink ...
6 years, 6 months ago (2014-06-06 13:48:29 UTC) #13
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 6 months ago (2014-06-06 13:50:46 UTC) #14
commit-bot: I haz the power
Try jobs failed on following builders: blink_presubmit on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/7156)
6 years, 6 months ago (2014-06-06 13:50:47 UTC) #15
sof
On 2014/06/06 13:50:47, I haz the power (commit-bot) wrote: > Try jobs failed on following ...
6 years, 6 months ago (2014-06-06 13:56:19 UTC) #16
tkent
https://codereview.chromium.org/315133004/diff/70001/Source/web/WebSpeechGrammar.cpp File Source/web/WebSpeechGrammar.cpp (right): https://codereview.chromium.org/315133004/diff/70001/Source/web/WebSpeechGrammar.cpp#newcode44 Source/web/WebSpeechGrammar.cpp:44: WebSpeechGrammar::WebSpeechGrammar(const WebCore::SpeechGrammar* value) This is not a mechanical change. ...
6 years, 6 months ago (2014-06-09 01:28:46 UTC) #17
sof
https://codereview.chromium.org/315133004/diff/70001/Source/web/WebSpeechGrammar.cpp File Source/web/WebSpeechGrammar.cpp (right): https://codereview.chromium.org/315133004/diff/70001/Source/web/WebSpeechGrammar.cpp#newcode44 Source/web/WebSpeechGrammar.cpp:44: WebSpeechGrammar::WebSpeechGrammar(const WebCore::SpeechGrammar* value) On 2014/06/09 01:28:46, tkent wrote: > ...
6 years, 6 months ago (2014-06-09 12:24:32 UTC) #18
sof
On 2014/06/09 12:24:32, sof wrote: > https://codereview.chromium.org/315133004/diff/70001/Source/web/WebSpeechGrammar.cpp > File Source/web/WebSpeechGrammar.cpp (right): > > https://codereview.chromium.org/315133004/diff/70001/Source/web/WebSpeechGrammar.cpp#newcode44 > ...
6 years, 6 months ago (2014-06-09 12:28:16 UTC) #19
tkent
https://codereview.chromium.org/315133004/diff/70001/Source/web/WebSpeechGrammar.cpp File Source/web/WebSpeechGrammar.cpp (right): https://codereview.chromium.org/315133004/diff/70001/Source/web/WebSpeechGrammar.cpp#newcode44 Source/web/WebSpeechGrammar.cpp:44: WebSpeechGrammar::WebSpeechGrammar(const WebCore::SpeechGrammar* value) On 2014/06/09 12:24:32, sof wrote: > ...
6 years, 6 months ago (2014-06-10 03:19:14 UTC) #20
sof
On 2014/06/10 03:19:14, tkent wrote: > https://codereview.chromium.org/315133004/diff/70001/Source/web/WebSpeechGrammar.cpp > File Source/web/WebSpeechGrammar.cpp (right): > > https://codereview.chromium.org/315133004/diff/70001/Source/web/WebSpeechGrammar.cpp#newcode44 > ...
6 years, 6 months ago (2014-06-10 06:28:41 UTC) #21
tkent
lgtm
6 years, 6 months ago (2014-06-10 06:34:52 UTC) #22
sof
The CQ bit was checked by sigbjornf@opera.com
6 years, 6 months ago (2014-06-10 06:42:42 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sigbjornf@opera.com/315133004/90001
6 years, 6 months ago (2014-06-10 06:43:03 UTC) #24
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: win_gpu_triggered_tests on tryserver.chromium.gpu ...
6 years, 6 months ago (2014-06-10 07:35:45 UTC) #25
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 6 months ago (2014-06-10 07:35:58 UTC) #26
commit-bot: I haz the power
Try jobs failed on following builders: win_gpu_triggered_tests on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered_tests/builds/15632)
6 years, 6 months ago (2014-06-10 07:35:59 UTC) #27
sof
The CQ bit was checked by sigbjornf@opera.com
6 years, 6 months ago (2014-06-10 07:39:36 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sigbjornf@opera.com/315133004/90001
6 years, 6 months ago (2014-06-10 07:40:22 UTC) #29
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: win_gpu_triggered_tests on tryserver.chromium.gpu ...
6 years, 6 months ago (2014-06-10 07:40:52 UTC) #30
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 6 months ago (2014-06-10 07:41:06 UTC) #31
commit-bot: I haz the power
Try jobs failed on following builders: win_gpu_triggered_tests on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered_tests/builds/15632)
6 years, 6 months ago (2014-06-10 07:41:07 UTC) #32
sof
The CQ bit was checked by sigbjornf@opera.com
6 years, 6 months ago (2014-06-10 08:29:08 UTC) #33
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sigbjornf@opera.com/315133004/90001
6 years, 6 months ago (2014-06-10 08:29:57 UTC) #34
commit-bot: I haz the power
Change committed as 175872
6 years, 6 months ago (2014-06-10 08:30:39 UTC) #35
Pat Meenan
A revert of this CL has been created in https://codereview.chromium.org/330463002/ by pmeenan@chromium.org. The reason for ...
6 years, 6 months ago (2014-06-11 15:30:11 UTC) #36
sof
With the fix for https://code.google.com/p/chromium/issues/detail?id=345061 in place, relanding.
6 years, 6 months ago (2014-06-12 11:13:41 UTC) #37
sof
The CQ bit was checked by sigbjornf@opera.com
6 years, 6 months ago (2014-06-12 11:14:51 UTC) #38
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sigbjornf@opera.com/315133004/90001
6 years, 6 months ago (2014-06-12 11:15:54 UTC) #39
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 6 months ago (2014-06-12 11:16:19 UTC) #40
commit-bot: I haz the power
Failed to apply patch for Source/modules/speech/SpeechRecognition.cpp: While running patch -p1 --forward --force --no-backup-if-mismatch; patching file ...
6 years, 6 months ago (2014-06-12 11:16:21 UTC) #41
sof
The CQ bit was checked by sigbjornf@opera.com
6 years, 6 months ago (2014-06-12 12:21:02 UTC) #42
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sigbjornf@opera.com/315133004/110001
6 years, 6 months ago (2014-06-12 12:22:14 UTC) #43
commit-bot: I haz the power
6 years, 6 months ago (2014-06-12 13:27:56 UTC) #44
Message was sent while issue was closed.
Change committed as 176025

Powered by Google App Engine
This is Rietveld 408576698