Fix crash in NSSpeechSynthesizer by explicitly retaining NSString.

Fix crash in NSSpeechSynthesizer by explicitly retaining NSString.

This definitely fixes one crash that was easy to repro by
speaking text at a high rate (~900 wpm) with the Samantha
voice on OS X 10.8.2. Retaining the NSString passed to
startSpeakingString fixes the problem, so this change
makes a one-time-use subclass of NSSpeechSynthesizer that
holds onto its utterance string while it's alive.

http://openradar.appspot.com/radar?id=2854403

BUG=99116
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=188338

[dmazzoni, 2013-03-14 22:36:49]

dtseng: review speech logic
thakis: please check Obj-C style and idiosyncratic usage

[Nico, 2013-03-14 22:45:17]

https://codereview.chromium.org/12636005/diff/1/chrome/browser/speech/tts_mac.mm
File chrome/browser/speech/tts_mac.mm (right):

https://codereview.chromium.org/12636005/diff/1/chrome/browser/speech/tts_mac...
chrome/browser/speech/tts_mac.mm:28: @interface SingleUseSpeechSynthesizer :
NSSpeechSynthesizer {
Can you add a comment explaining why this exists?

Also, can you file a bug for this (ideally with a small, self-contained repro)
at bugreporter.apple.com – NSSpeechSynthesizer should be retaiing the
startSpeakingString: argument, and if it doesn't that's a bug. (After you've
filed, please copy the bug report to openradar.appspot.com and include a link to
that in the class comment)

https://codereview.chromium.org/12636005/diff/1/chrome/browser/speech/tts_mac...
chrome/browser/speech/tts_mac.mm:97: [NSString stringWithUTF8String:
utterance_.c_str()];
No space after ':'

(btw, clang_format can format objc)

[David Tseng, 2013-03-14 23:08:25]

lgtm

[dmazzoni, 2013-03-14 23:29:31]

https://codereview.chromium.org/12636005/diff/1/chrome/browser/speech/tts_mac.mm
File chrome/browser/speech/tts_mac.mm (right):

https://codereview.chromium.org/12636005/diff/1/chrome/browser/speech/tts_mac...
chrome/browser/speech/tts_mac.mm:28: @interface SingleUseSpeechSynthesizer :
NSSpeechSynthesizer {
On 2013/03/14 22:45:17, Nico wrote:
> Can you add a comment explaining why this exists?
> 
> Also, can you file a bug for this (ideally with a small, self-contained repro)
> at http://bugreporter.apple.com – NSSpeechSynthesizer should be retaiing
the
> startSpeakingString: argument, and if it doesn't that's a bug. (After you've
> filed, please copy the bug report to http://openradar.appspot.com and include
a link to
> that in the class comment)

Done.

https://codereview.chromium.org/12636005/diff/1/chrome/browser/speech/tts_mac...
chrome/browser/speech/tts_mac.mm:97: [NSString stringWithUTF8String:
utterance_.c_str()];
On 2013/03/14 22:45:17, Nico wrote:
> No space after ':'
> 
> (btw, clang_format can format objc)

Done.

[Nico, 2013-03-14 23:32:00]

https://codereview.chromium.org/12636005/diff/1/chrome/browser/speech/tts_mac.mm
File chrome/browser/speech/tts_mac.mm (right):

https://codereview.chromium.org/12636005/diff/1/chrome/browser/speech/tts_mac...
chrome/browser/speech/tts_mac.mm:28: @interface SingleUseSpeechSynthesizer :
NSSpeechSynthesizer {
On 2013/03/14 22:45:17, Nico wrote:
> Can you add a comment explaining why this exists?
> 
> Also, can you file a bug for this (ideally with a small, self-contained repro)
> at http://bugreporter.apple.com – NSSpeechSynthesizer should be retaiing
the
> startSpeakingString: argument, and if it doesn't that's a bug. (After you've
> filed, please copy the bug report to http://openradar.appspot.com and include
a link to
> that in the class comment)

I meant a link to the openradar one, rdar:// links can only be read by apple
people :-)

[dmazzoni, 2013-03-14 23:40:28]

On 2013/03/14 23:32:00, Nico wrote:
> I meant a link to the openradar one, rdar:// links can only be read by apple
> people :-)

Sorry! Done.

[Nico, 2013-03-15 00:38:36]

lgtm

Thanks!

[commit-bot: I haz the power, 2013-03-15 05:07:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmazzoni@chromium.org/12636005/7002

[commit-bot: I haz the power, 2013-03-15 07:01:21]

Retried try job too often on mac_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2013-03-15 07:21:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmazzoni@chromium.org/12636005/7002

[commit-bot: I haz the power, 2013-03-15 11:16:54]

Change committed as 188338