Fix crash in NSSpeechSynthesizer by explicitly retaining NSString.

chrome/browser/speech/tts_mac.mm

Index: chrome/browser/speech/tts_mac.mm
diff --git a/chrome/browser/speech/tts_mac.mm b/chrome/browser/speech/tts_mac.mm
index 44f5ed3419d5c258fd1e449184c4f5a513f82e24..ce02ebc9f2dc700c2ed2c2b3cce2e3b4eac2c1b0 100644
--- a/chrome/browser/speech/tts_mac.mm
+++ b/chrome/browser/speech/tts_mac.mm
@@ -25,6 +25,18 @@ class TtsPlatformImplMac;
 
 @end
 
+@interface SingleUseSpeechSynthesizer : NSSpeechSynthesizer {

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

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)

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

Done.

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

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

+ @private
+  scoped_nsobject<NSString> utterance_;
+  bool didSpeak_;
+}
+
+- (id)initWithUtterance:(NSString*)utterance;
+- (bool)startSpeakingRetainedUtterance;
+- (bool)startSpeakingString:(NSString*)utterance;
+
+@end
+
 class TtsPlatformImplMac : public TtsPlatformImpl {
  public:
   virtual bool PlatformImplAvailable() OVERRIDE {
@@ -57,7 +69,7 @@ class TtsPlatformImplMac : public TtsPlatformImpl {
   TtsPlatformImplMac();
   virtual ~TtsPlatformImplMac();
 
-  scoped_nsobject<NSSpeechSynthesizer> speech_synthesizer_;
+  scoped_nsobject<SingleUseSpeechSynthesizer> speech_synthesizer_;
   scoped_nsobject<ChromeTtsDelegate> delegate_;
   int utterance_id_;
   std::string utterance_;
@@ -78,20 +90,25 @@ bool TtsPlatformImplMac::Speak(
     const std::string& utterance,
     const std::string& lang,
     const UtteranceContinuousParameters& params) {
+  // TODO: convert SSML to SAPI xml. http://crbug.com/88072
+  utterance_ = utterance;
+
+  NSString* utterance_nsstring =
+      [NSString stringWithUTF8String: utterance_.c_str()];

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

No space after ':'

(btw, clang_format can format objc)

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

Done.

+
   // Deliberately construct a new speech synthesizer every time Speak is
   // called, otherwise there's no way to know whether calls to the delegate
   // apply to the current utterance or a previous utterance. In
   // experimentation, the overhead of constructing and destructing a
   // NSSpeechSynthesizer is minimal.
-  speech_synthesizer_.reset([[NSSpeechSynthesizer alloc] init]);
+  speech_synthesizer_.reset(
+      [[SingleUseSpeechSynthesizer alloc]
+        initWithUtterance:utterance_nsstring]);
   [speech_synthesizer_ setDelegate:delegate_];
 
   utterance_id_ = utterance_id;
   sent_start_event_ = false;
 
-  // TODO: convert SSML to SAPI xml. http://crbug.com/88072
-  utterance_ = utterance;
-
   // TODO: support languages other than the default: crbug.com/88059
 
   if (params.rate >= 0.0) {
@@ -114,8 +131,7 @@ bool TtsPlatformImplMac::Speak(
         forProperty:NSSpeechVolumeProperty error:nil];
   }
 
-  return [speech_synthesizer_ startSpeakingString:
-             [NSString stringWithUTF8String: utterance.c_str()]];
+  return [speech_synthesizer_ startSpeakingRetainedUtterance];
 }
 
 bool TtsPlatformImplMac::StopSpeaking() {
@@ -206,3 +222,28 @@ TtsPlatformImplMac* TtsPlatformImplMac::GetInstance() {
 }
 
 @end
+
+@implementation SingleUseSpeechSynthesizer
+
+- (id)initWithUtterance:(NSString*)utterance {
+  self = [super init];
+  if (self) {
+    utterance_.reset([utterance retain]);
+    didSpeak_ = false;
+  }
+  return self;
+}
+
+- (bool)startSpeakingRetainedUtterance {
+  CHECK(!didSpeak_);
+  CHECK(utterance_);
+  didSpeak_ = true;
+  return [super startSpeakingString:utterance_];
+}
+
+- (bool)startSpeakingString:(NSString*)utterance {
+  CHECK(false);
+  return false;
+}
+
+@end