Fix crash in NSSpeechSynthesizer by explicitly retaining NSString.

chrome/browser/speech/tts_mac.mm

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <string>
 
 #include "base/memory/scoped_nsobject.h"
 #include "base/memory/singleton.h"
 #include "base/sys_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/extension_function.h"
 #include "chrome/browser/speech/tts_controller.h"
 #include "chrome/browser/speech/tts_platform.h"
 
 #import <Cocoa/Cocoa.h>
 
 class TtsPlatformImplMac;
 
 @interface ChromeTtsDelegate : NSObject <NSSpeechSynthesizerDelegate> {
  @private
   TtsPlatformImplMac* ttsImplMac_;  // weak.
 }
 
 - (id)initWithPlatformImplMac:(TtsPlatformImplMac*)ttsImplMac;
 
 @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 {
     return true;
   }
 
   virtual bool Speak(
       int utterance_id,
       const std::string& utterance,
       const std::string& lang,
@@ skipping 12 matching lines @@
                      int char_index,
                      const std::string& error_message);
 
   // Get the single instance of this class.
   static TtsPlatformImplMac* GetInstance();
 
  private:
   TtsPlatformImplMac();
   virtual ~TtsPlatformImplMac();
 
-  scoped_nsobject<NSSpeechSynthesizer> speech_synthesizer_;
+  scoped_nsobject<SingleUseSpeechSynthesizer> speech_synthesizer_;
   scoped_nsobject<ChromeTtsDelegate> delegate_;
   int utterance_id_;
   std::string utterance_;
   bool sent_start_event_;
 
   friend struct DefaultSingletonTraits<TtsPlatformImplMac>;
 
   DISALLOW_COPY_AND_ASSIGN(TtsPlatformImplMac);
 };
 
 // static
 TtsPlatformImpl* TtsPlatformImpl::GetInstance() {
   return TtsPlatformImplMac::GetInstance();
 }
 
 bool TtsPlatformImplMac::Speak(
     int utterance_id,
     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) {
     // The TTS api defines rate via words per minute. Let 200 be the default.
     [speech_synthesizer_
         setObject:[NSNumber numberWithInt:params.rate * 200]
         forProperty:NSSpeechRateProperty error:nil];
   }
 
   if (params.pitch >= 0.0) {
     // The TTS api allows an approximate range of 30 to 65 for speech pitch.
     [speech_synthesizer_
         setObject: [NSNumber numberWithInt:(params.pitch * 17 + 30)]
         forProperty:NSSpeechPitchBaseProperty error:nil];
   }
 
   if (params.volume >= 0.0) {
     [speech_synthesizer_
         setObject: [NSNumber numberWithFloat:params.volume]
         forProperty:NSSpeechVolumeProperty error:nil];
   }
 
-  return [speech_synthesizer_ startSpeakingString:
-             [NSString stringWithUTF8String: utterance.c_str()]];
+  return [speech_synthesizer_ startSpeakingRetainedUtterance];
 }
 
 bool TtsPlatformImplMac::StopSpeaking() {
   if (speech_synthesizer_.get()) {
     [speech_synthesizer_ stopSpeaking];
     speech_synthesizer_.reset(nil);
   }
   return true;
 }
 
@@ skipping 70 matching lines @@
 - (void)speechSynthesizer:(NSSpeechSynthesizer*)sender
  didEncounterErrorAtIndex:(NSUInteger)character_index
                  ofString:(NSString*)string
                   message:(NSString*)message {
   std::string message_utf8 = base::SysNSStringToUTF8(message);
   ttsImplMac_->OnSpeechEvent(sender, TTS_EVENT_ERROR, character_index,
       message_utf8);
 }
 
 @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