Bug 1541298 - Implement an async shutdown blocker for SpeechRecognition. r=jib
authorAndreas Pehrson <apehrson@mozilla.com>
Fri, 12 Apr 2019 11:38:36 +0000
changeset 469262 3119da540c3170ee981b575fef593c3bcf8b6eaa
parent 469261 e8717a523b34291683440914deda399199dc6247
child 469263 85272d58ac1a65a2e03c78c0ffb6ba8134f38b8f
push id112776
push usershindli@mozilla.com
push dateFri, 12 Apr 2019 16:20:17 +0000
treeherdermozilla-inbound@b4501ced5619 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjib
bugs1541298
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1541298 - Implement an async shutdown blocker for SpeechRecognition. r=jib This will let us clear the SpeechTrackListener's ref-cycle and avoid leaking. Differential Revision: https://phabricator.services.mozilla.com/D26724
dom/media/webspeech/recognition/SpeechRecognition.cpp
dom/media/webspeech/recognition/SpeechRecognition.h
dom/media/webspeech/recognition/SpeechTrackListener.cpp
dom/media/webspeech/recognition/SpeechTrackListener.h
--- a/dom/media/webspeech/recognition/SpeechRecognition.cpp
+++ b/dom/media/webspeech/recognition/SpeechRecognition.cpp
@@ -98,16 +98,34 @@ already_AddRefed<nsISpeechRecognitionSer
   }
 
   nsresult rv;
   nsCOMPtr<nsISpeechRecognitionService> recognitionService;
   recognitionService = do_GetService(speechRecognitionServiceCID.get(), &rv);
   return recognitionService.forget();
 }
 
+class SpeechRecognitionShutdownBlocker : public media::ShutdownBlocker {
+ public:
+  explicit SpeechRecognitionShutdownBlocker(SpeechRecognition* aRecognition)
+      : media::ShutdownBlocker(NS_LITERAL_STRING("SpeechRecognition shutdown")),
+        mRecognition(aRecognition) {}
+
+  NS_IMETHOD BlockShutdown(nsIAsyncShutdownClient*) override {
+    MOZ_ASSERT(NS_IsMainThread());
+
+    // AbortSilently will eventually clear the blocker.
+    mRecognition->Abort();
+    return NS_OK;
+  }
+
+ private:
+  const RefPtr<SpeechRecognition> mRecognition;
+};
+
 NS_IMPL_CYCLE_COLLECTION_INHERITED(SpeechRecognition, DOMEventTargetHelper,
                                    mStream, mTrack, mRecognitionService,
                                    mSpeechGrammarList)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(SpeechRecognition)
   NS_INTERFACE_MAP_ENTRY(nsIObserver)
 NS_INTERFACE_MAP_END_INHERITING(DOMEventTargetHelper)
 
@@ -531,27 +549,43 @@ SpeechRecognition::StartRecording(RefPtr
   mTrack = aTrack;
 
   if (NS_WARN_IF(mTrack->Ended())) {
     return NS_ERROR_UNEXPECTED;
   }
   mSpeechListener = new SpeechTrackListener(this);
   mTrack->AddListener(mSpeechListener);
 
+  mShutdownBlocker = MakeAndAddRef<SpeechRecognitionShutdownBlocker>(this);
+  RefPtr<nsIAsyncShutdownClient> shutdown = media::GetShutdownBarrier();
+  shutdown->AddBlocker(mShutdownBlocker, NS_LITERAL_STRING(__FILE__), __LINE__,
+                       NS_LITERAL_STRING("SpeechRecognition shutdown"));
+
   mEndpointer.StartSession();
 
   return mSpeechDetectionTimer->Init(this, kSPEECH_DETECTION_TIMEOUT_MS,
                                      nsITimer::TYPE_ONE_SHOT);
 }
 
 NS_IMETHODIMP
 SpeechRecognition::StopRecording() {
-  // we only really need to remove the listener explicitly when testing,
-  // as our JS code still holds a reference to mTrack and only assigning
-  // it to nullptr isn't guaranteed to free the stream and the listener.
+  if (mShutdownBlocker) {
+    // Block shutdown until the speech track listener has been removed from the
+    // MSG, as it holds a reference to us, and we reference the world, which we
+    // don't want to leak.
+    mSpeechListener->mRemovedPromise->Then(
+        GetCurrentThreadSerialEventTarget(), __func__,
+        [blocker = std::move(mShutdownBlocker)] {
+          RefPtr<nsIAsyncShutdownClient> shutdown = media::GetShutdownBarrier();
+          nsresult rv = shutdown->RemoveBlocker(blocker);
+          MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv));
+        });
+  }
+  MOZ_ASSERT(!mShutdownBlocker);
+
   mStream->UnregisterTrackListener(this);
   mTrack->RemoveListener(mSpeechListener);
   mStream = nullptr;
   mSpeechListener = nullptr;
   mTrack = nullptr;
 
   mEndpointer.EndSession();
   DispatchTrustedEvent(NS_LITERAL_STRING("audioend"));
--- a/dom/media/webspeech/recognition/SpeechRecognition.h
+++ b/dom/media/webspeech/recognition/SpeechRecognition.h
@@ -35,16 +35,17 @@ namespace mozilla {
 namespace dom {
 
 #define SPEECH_RECOGNITION_TEST_EVENT_REQUEST_TOPIC \
   "SpeechRecognitionTest:RequestEvent"
 #define SPEECH_RECOGNITION_TEST_END_TOPIC "SpeechRecognitionTest:End"
 
 class GlobalObject;
 class AudioStreamTrack;
+class SpeechRecognitionShutdownBlocker;
 class SpeechEvent;
 class SpeechTrackListener;
 
 LogModule* GetSpeechRecognitionLog();
 #define SR_LOG(...) \
   MOZ_LOG(GetSpeechRecognitionLog(), mozilla::LogLevel::Debug, (__VA_ARGS__))
 
 class SpeechRecognition final : public DOMEventTargetHelper,
@@ -181,16 +182,17 @@ class SpeechRecognition final : public D
   void NotifyFinalResult(SpeechEvent* aEvent);
   void DoNothing(SpeechEvent* aEvent);
   void AbortSilently(SpeechEvent* aEvent);
   void AbortError(SpeechEvent* aEvent);
 
   RefPtr<DOMMediaStream> mStream;
   RefPtr<AudioStreamTrack> mTrack;
   RefPtr<SpeechTrackListener> mSpeechListener;
+  RefPtr<SpeechRecognitionShutdownBlocker> mShutdownBlocker;
   nsCOMPtr<nsISpeechRecognitionService> mRecognitionService;
 
   FSMState mCurrentState;
 
   Endpointer mEndpointer;
   uint32_t mEstimationSamples;
 
   uint32_t mAudioSamplesPerChunk;
--- a/dom/media/webspeech/recognition/SpeechTrackListener.cpp
+++ b/dom/media/webspeech/recognition/SpeechTrackListener.cpp
@@ -8,21 +8,24 @@
 
 #include "SpeechRecognition.h"
 #include "nsProxyRelease.h"
 
 namespace mozilla {
 namespace dom {
 
 SpeechTrackListener::SpeechTrackListener(SpeechRecognition* aRecognition)
-    : mRecognition(aRecognition) {}
-
-SpeechTrackListener::~SpeechTrackListener() {
-  NS_ReleaseOnMainThreadSystemGroup("SpeechTrackListener::mRecognition",
-                                    mRecognition.forget());
+    : mRecognition(aRecognition),
+      mRemovedPromise(
+          mRemovedHolder.Ensure("SpeechTrackListener::mRemovedPromise")) {
+  MOZ_ASSERT(NS_IsMainThread());
+  mRemovedPromise->Then(GetCurrentThreadSerialEventTarget(), __func__,
+                        [self = RefPtr<SpeechTrackListener>(this), this] {
+                          mRecognition = nullptr;
+                        });
 }
 
 void SpeechTrackListener::NotifyQueuedChanges(
     MediaStreamGraph* aGraph, StreamTime aTrackOffset,
     const MediaSegment& aQueuedMedia) {
   AudioSegment* audio = const_cast<AudioSegment*>(
       static_cast<const AudioSegment*>(&aQueuedMedia));
 
@@ -74,10 +77,14 @@ void SpeechTrackListener::ConvertAndDisp
 
   mRecognition->FeedAudioData(samples.forget(), aDuration, this, aTrackRate);
 }
 
 void SpeechTrackListener::NotifyEnded() {
   // TODO dispatch SpeechEnd event so services can be informed
 }
 
+void SpeechTrackListener::NotifyRemoved() {
+  mRemovedHolder.ResolveIfExists(true, __func__);
+}
+
 }  // namespace dom
 }  // namespace mozilla
--- a/dom/media/webspeech/recognition/SpeechTrackListener.h
+++ b/dom/media/webspeech/recognition/SpeechTrackListener.h
@@ -5,39 +5,46 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_dom_SpeechStreamListener_h
 #define mozilla_dom_SpeechStreamListener_h
 
 #include "MediaStreamGraph.h"
 #include "MediaStreamListener.h"
 #include "AudioSegment.h"
+#include "mozilla/MozPromise.h"
 
 namespace mozilla {
 
 class AudioSegment;
 
 namespace dom {
 
 class SpeechRecognition;
 
 class SpeechTrackListener : public MediaStreamTrackListener {
  public:
   explicit SpeechTrackListener(SpeechRecognition* aRecognition);
-  ~SpeechTrackListener();
+  ~SpeechTrackListener() = default;
 
   void NotifyQueuedChanges(MediaStreamGraph* aGraph, StreamTime aTrackOffset,
                            const MediaSegment& aQueuedMedia) override;
 
   void NotifyEnded() override;
 
+  void NotifyRemoved() override;
+
  private:
   template <typename SampleFormatType>
   void ConvertAndDispatchAudioChunk(int aDuration, float aVolume,
                                     SampleFormatType* aData,
                                     TrackRate aTrackRate);
   RefPtr<SpeechRecognition> mRecognition;
+  MozPromiseHolder<GenericNonExclusivePromise> mRemovedHolder;
+
+ public:
+  const RefPtr<GenericNonExclusivePromise> mRemovedPromise;
 };
 
 }  // namespace dom
 }  // namespace mozilla
 
 #endif