Bug 1363163 - Playing sound in a seperated thread to avoid jank draft
authorThomas Nguyen <tnguyen@mozilla.com>
Thu, 08 Jun 2017 11:08:49 +0800
changeset 599755 2f74226bcceb6dd5d0443074b89e163e98e7e01d
parent 599733 594cc32b632396a867ef1f98428968b224d82151
child 634838 429c03a2d7bd2b706dceb5aca468553c2675b927
push id65590
push userbmo:tnguyen@mozilla.com
push dateFri, 23 Jun 2017 14:27:30 +0000
bugs1363163
milestone56.0a1
Bug 1363163 - Playing sound in a seperated thread to avoid jank MozReview-Commit-ID: KYN7fhA541S
toolkit/components/typeaheadfind/nsTypeAheadFind.cpp
widget/windows/nsSound.cpp
widget/windows/nsSound.h
widget/windows/nsWidgetFactory.cpp
--- a/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp
+++ b/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp
@@ -106,26 +106,39 @@ nsTypeAheadFind::Init(nsIDocShell* aDocS
     return NS_ERROR_FAILURE;
 
   SetDocShell(aDocShell);
 
   if (!mDidAddObservers) {
     mDidAddObservers = true;
     // ----------- Listen to prefs ------------------
     nsresult rv = prefInternal->AddObserver("accessibility.browsewithcaret", this, true);
+    rv = prefInternal->AddObserver("accessibility.typeaheadfind", this, true);
     NS_ENSURE_SUCCESS(rv, rv);
 
     // ----------- Get initial preferences ----------
     PrefsReset();
 
     nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
     if (os) {
       os->AddObserver(this, DOM_WINDOW_DESTROYED_TOPIC, true);
     }
   }
+
+  if (!mIsSoundInitialized && !mNotFoundSoundURL.IsEmpty()) {
+    // This makes sure system sound library is loaded so that
+    // there's no lag before the first sound is played
+    // by waiting for the first keystroke, we still get the startup time benefits.
+    mIsSoundInitialized = true;
+    mSoundInterface = do_CreateInstance("@mozilla.org/sound;1");
+    if (mSoundInterface && !mNotFoundSoundURL.EqualsLiteral("beep")) {
+      mSoundInterface->Init();
+    }
+  }
+
   return NS_OK;
 }
 
 nsresult
 nsTypeAheadFind::PrefsReset()
 {
   nsCOMPtr<nsIPrefBranch> prefBranch(do_GetService(NS_PREFSERVICE_CONTRACTID));
   NS_ENSURE_TRUE(prefBranch, NS_ERROR_FAILURE);
@@ -137,16 +150,28 @@ nsTypeAheadFind::PrefsReset()
   prefBranch->GetBoolPref("accessibility.typeaheadfind.enablesound",
                            &isSoundEnabled);
   nsXPIDLCString soundStr;
   if (isSoundEnabled)
     prefBranch->GetCharPref("accessibility.typeaheadfind.soundURL", getter_Copies(soundStr));
 
   mNotFoundSoundURL = soundStr;
 
+  if (!mNotFoundSoundURL.IsEmpty() && !mNotFoundSoundURL.EqualsLiteral("beep")) {
+    if (!mSoundInterface) {
+      mSoundInterface = do_CreateInstance("@mozilla.org/sound;1");
+    }
+
+    // Init to load the system sound library if the lib is not ready
+    if (mSoundInterface) {
+      mIsSoundInitialized = true;
+      mSoundInterface->Init();
+    }
+  }
+
   prefBranch->GetBoolPref("accessibility.browsewithcaret",
                           &mCaretBrowsingOn);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsTypeAheadFind::SetCaseSensitive(bool isCaseSensitive)
@@ -1020,34 +1045,16 @@ nsTypeAheadFind::Find(const nsAString& a
     const nsAString& oldStr2 = Substring(mTypeAheadBuffer, 0, aSearchString.Length());
     if (oldStr2.Equals(newStr2))
       atEnd = true;
     
     if (!atEnd)
       mStartFindRange = nullptr;
   }
 
-  if (!mIsSoundInitialized && !mNotFoundSoundURL.IsEmpty()) {
-    // This makes sure system sound library is loaded so that
-    // there's no lag before the first sound is played
-    // by waiting for the first keystroke, we still get the startup time benefits.
-    mIsSoundInitialized = true;
-    mSoundInterface = do_CreateInstance("@mozilla.org/sound;1");
-    if (mSoundInterface && !mNotFoundSoundURL.EqualsLiteral("beep")) {
-      mSoundInterface->Init();
-    }
-  }
-
-#ifdef XP_WIN
-  // After each keystroke, ensure sound object is destroyed, to free up memory 
-  // allocated for error sound, otherwise Windows' nsISound impl 
-  // holds onto the last played sound, using up memory.
-  mSoundInterface = nullptr;
-#endif
-
   int32_t bufferLength = mTypeAheadBuffer.Length();
 
   mTypeAheadBuffer = aSearchString;
 
   bool isFirstVisiblePreferred = false;
 
   // --------- Initialize find if 1st char ----------
   if (bufferLength == 0) {
--- a/widget/windows/nsSound.cpp
+++ b/widget/windows/nsSound.cpp
@@ -14,146 +14,141 @@
 #include <mmsystem.h>
 
 #include "nsSound.h"
 #include "nsIURL.h"
 #include "nsNetUtil.h"
 #include "nsIChannel.h"
 #include "nsContentUtils.h"
 #include "nsCRT.h"
+#include "nsIObserverService.h"
 
 #include "mozilla/Logging.h"
 #include "prtime.h"
 #include "prmem.h"
 
 #include "nsNativeCharsetUtils.h"
 #include "nsThreadUtils.h"
+#include "mozilla/ClearOnShutdown.h"
 
 using mozilla::LogLevel;
 
 static mozilla::LazyLogModule gWin32SoundLog("nsSound");
 
 class nsSoundPlayer: public mozilla::Runnable {
 public:
-  nsSoundPlayer(nsSound *aSound, const wchar_t* aSoundName) :
-    mSoundName(aSoundName), mSound(aSound)
+  explicit nsSoundPlayer(const nsAString& aSoundName)
+    : mSoundName(aSoundName)
+    , mSoundData(nullptr)
   {
-    Init();
   }
 
-  nsSoundPlayer(nsSound *aSound, const nsAString& aSoundName) :
-    mSoundName(aSoundName), mSound(aSound)
+  nsSoundPlayer(const uint8_t *aData, size_t aSize)
+    : mSoundName(EmptyString())
   {
-    Init();
+    MOZ_ASSERT(aSize > 0, "Size should not be zero");
+    MOZ_ASSERT(aData, "Data shoud not be null");
+
+    // We will disptach nsSoundPlayer to playerthread, so keep a data copy
+    mSoundData = new uint8_t[aSize];
+    memcpy(mSoundData, aData, aSize);
   }
 
   NS_DECL_NSIRUNNABLE
 
 protected:
-  nsString mSoundName;
-  nsSound *mSound; // Strong, but this will be released from SoundReleaser.
-  nsCOMPtr<nsIThread> mThread;
-
-  void Init()
-  {
-    NS_GetCurrentThread(getter_AddRefs(mThread));
-    NS_ASSERTION(mThread, "failed to get current thread");
-    NS_IF_ADDREF(mSound);
-  }
+  ~nsSoundPlayer();
 
-  class SoundReleaser: public mozilla::Runnable {
-  public:
-    explicit SoundReleaser(nsSound* aSound) :
-      mSound(aSound)
-    {
-    }
-
-    NS_DECL_NSIRUNNABLE
-
-  protected:
-    nsSound *mSound;
-  };
+  nsString mSoundName;
+  uint8_t* mSoundData;
 };
 
 NS_IMETHODIMP
 nsSoundPlayer::Run()
 {
-  NS_SetCurrentThreadName("Play Sound");
+  MOZ_ASSERT(!mSoundName.IsEmpty() || mSoundData,
+    "Sound name or sound data should be specified");
+  DWORD flags = SND_NODEFAULT | SND_ASYNC;
 
-  NS_PRECONDITION(!mSoundName.IsEmpty(), "Sound name should not be empty");
-  ::PlaySoundW(mSoundName.get(), nullptr,
-               SND_NODEFAULT | SND_ALIAS | SND_ASYNC);
-  nsCOMPtr<nsIRunnable> releaser = new SoundReleaser(mSound);
-  // Don't release nsSound from here, because here is not an owning thread of
-  // the nsSound. nsSound must be released in its owning thread.
-  mThread->Dispatch(releaser, NS_DISPATCH_NORMAL);
+  if (mSoundData) {
+    flags |= SND_MEMORY;
+    ::PlaySoundW(reinterpret_cast<LPCWSTR>(mSoundData), nullptr, flags);
+  } else {
+    flags |= SND_ALIAS;
+    ::PlaySoundW(mSoundName.get(), nullptr, flags);
+  }
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsSoundPlayer::SoundReleaser::Run()
+nsSoundPlayer::~nsSoundPlayer()
 {
-  mSound->ShutdownOldPlayerThread();
-  NS_IF_RELEASE(mSound);
-  return NS_OK;
+  delete [] mSoundData;
 }
 
+mozilla::StaticRefPtr<nsISound> nsSound::sInstance;
+
+/* static */ already_AddRefed<nsISound>
+nsSound::GetInstance()
+{
+  if (!sInstance) {
+    RefPtr<nsSound> sound = new nsSound();
+    nsresult rv = sound->CreatePlayerThread();
+    if(NS_WARN_IF(NS_FAILED(rv))) {
+      return nullptr;
+    }
+    sInstance = sound.forget();
+    ClearOnShutdown(&sInstance);
+  }
+
+  RefPtr<nsISound> service = sInstance;
+  return service.forget();
+}
 
 #ifndef SND_PURGE
 // Not available on Windows CE, and according to MSDN
 // doesn't do anything on recent windows either.
 #define SND_PURGE 0
 #endif
 
 NS_IMPL_ISUPPORTS(nsSound, nsISound, nsIStreamLoaderObserver)
 
 
 nsSound::nsSound()
+  : mInited(false)
 {
-    mLastSound = nullptr;
 }
 
 nsSound::~nsSound()
 {
-  NS_ASSERTION(!mPlayerThread, "player thread is not null but should be");
-  PurgeLastSound();
 }
 
-void nsSound::ShutdownOldPlayerThread()
+void nsSound::PurgeLastSound()
 {
-  nsCOMPtr<nsIThread> playerThread(mPlayerThread.forget());
-  if (playerThread)
-    playerThread->Shutdown();
-}
-
-void nsSound::PurgeLastSound() 
-{
-  if (mLastSound) {
-    // Halt any currently playing sound.
-    ::PlaySound(nullptr, nullptr, SND_PURGE);
-
-    // Now delete the buffer.
-    free(mLastSound);
-    mLastSound = nullptr;
+  // Halt any currently playing sound.
+  if (mPlayerThread) {
+    mPlayerThread->Dispatch(NS_NewRunnableFunction([]() {
+      ::PlaySound(nullptr, nullptr, SND_PURGE);
+    }), NS_DISPATCH_NORMAL);
   }
 }
 
 NS_IMETHODIMP nsSound::Beep()
 {
   ::MessageBeep(0);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP nsSound::OnStreamComplete(nsIStreamLoader *aLoader,
                                         nsISupports *context,
                                         nsresult aStatus,
                                         uint32_t dataLen,
                                         const uint8_t *data)
 {
+  MOZ_ASSERT(mPlayerThread, "player thread should not be null ");
   // print a load error on bad status
   if (NS_FAILED(aStatus)) {
 #ifdef DEBUG
     if (aLoader) {
       nsCOMPtr<nsIRequest> request;
       nsCOMPtr<nsIChannel> channel;
       aLoader->GetRequest(getter_AddRefs(request));
       if (request)
@@ -168,29 +163,26 @@ NS_IMETHODIMP nsSound::OnStreamComplete(
                  ("Failed to load %s\n", uriSpec.get()));
         }
       }
     }
 #endif
     return aStatus;
   }
 
-  ShutdownOldPlayerThread();
   PurgeLastSound();
 
   if (data && dataLen > 0) {
-    DWORD flags = SND_MEMORY | SND_NODEFAULT;
-    // We try to make a copy so we can play it async.
-    mLastSound = (uint8_t *) malloc(dataLen);
-    if (mLastSound) {
-      memcpy(mLastSound, data, dataLen);
-      data = mLastSound;
-      flags |= SND_ASYNC;
+    RefPtr<nsSoundPlayer> player = new nsSoundPlayer(data, dataLen);
+    MOZ_ASSERT(player, "Could not create player");
+
+    nsresult rv = mPlayerThread->Dispatch(player, NS_DISPATCH_NORMAL);
+    if (NS_WARN_IF(FAILED(rv))) {
+      return rv;
     }
-    ::PlaySoundW(reinterpret_cast<LPCWSTR>(data), 0, flags);
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP nsSound::Play(nsIURL *aURL)
 {
   nsresult rv;
@@ -207,44 +199,91 @@ NS_IMETHODIMP nsSound::Play(nsIURL *aURL
                           aURL,
                           this, // aObserver
                           nsContentUtils::GetSystemPrincipal(),
                           nsILoadInfo::SEC_ALLOW_CROSS_ORIGIN_DATA_IS_NULL,
                           nsIContentPolicy::TYPE_OTHER);
   return rv;
 }
 
+nsresult
+nsSound::CreatePlayerThread()
+{
+  if (mPlayerThread) {
+    return NS_OK;
+  }
+  if (NS_WARN_IF(NS_FAILED(
+        NS_NewNamedThread("PlayEventSound", getter_AddRefs(mPlayerThread))))) {
+    return NS_ERROR_FAILURE;
+  }
+
+  // Add an observer for shutdown event to release the thread at that time
+  nsCOMPtr<nsIObserverService> observerService =
+    mozilla::services::GetObserverService();
+  if (!observerService) {
+    return NS_ERROR_FAILURE;
+  }
+
+  observerService->AddObserver(this, "xpcom-shutdown-threads", false);
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+nsSound::Observe(nsISupports *aSubject, const char *aTopic,
+                 const char16_t *aData)
+{
+  if (!strcmp(aTopic, "xpcom-shutdown-threads")) {
+    PurgeLastSound();
+
+    if (mPlayerThread) {
+      mPlayerThread->Shutdown();
+      mPlayerThread = nullptr;
+    }
+  }
+
+  return NS_OK;
+}
 
 NS_IMETHODIMP nsSound::Init()
 {
+  if (mInited) {
+    return NS_OK;
+  }
+
+  MOZ_ASSERT(mPlayerThread, "player thread should not be null ");
   // This call halts a sound if it was still playing.
   // We have to use the sound library for something to make sure
   // it is initialized.
   // If we wait until the first sound is played, there will
   // be a time lag as the library gets loaded.
-  ::PlaySound(nullptr, nullptr, SND_PURGE);
+  // This should be done in player thread otherwise it will block main thread
+  // at the first time loading sound library.
+  mPlayerThread->Dispatch(NS_NewRunnableFunction([]() {
+    ::PlaySound(nullptr, nullptr, SND_PURGE);
+  }), NS_DISPATCH_NORMAL);
+
+  mInited = true;
 
   return NS_OK;
 }
 
-
 NS_IMETHODIMP nsSound::PlaySystemSound(const nsAString &aSoundAlias)
 {
-  ShutdownOldPlayerThread();
+  MOZ_ASSERT(mPlayerThread, "player thread should not be null ");
   PurgeLastSound();
 
   if (!NS_IsMozAliasSound(aSoundAlias)) {
     if (aSoundAlias.IsEmpty())
       return NS_OK;
-    nsCOMPtr<nsIRunnable> player = new nsSoundPlayer(this, aSoundAlias);
-    NS_ENSURE_TRUE(player, NS_ERROR_OUT_OF_MEMORY);
-    nsresult rv =
-      NS_NewNamedThread("PlaySystemSound", getter_AddRefs(mPlayerThread),
-                        player);
-    NS_ENSURE_SUCCESS(rv, rv);
+    nsCOMPtr<nsIRunnable> player = new nsSoundPlayer(aSoundAlias);
+    MOZ_ASSERT(player, "Could not create player");
+    nsresult rv = mPlayerThread->Dispatch(player, NS_DISPATCH_NORMAL);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
     return NS_OK;
   }
 
   NS_WARNING("nsISound::playSystemSound is called with \"_moz_\" events, they are obsolete, use nsISound::playEventSound instead");
 
   uint32_t eventId;
   if (aSoundAlias.Equals(NS_SYSSOUND_MAIL_BEEP))
     eventId = EVENT_NEW_MAIL_RECEIVED;
@@ -259,17 +298,17 @@ NS_IMETHODIMP nsSound::PlaySystemSound(c
   else
     return NS_OK;
 
   return PlayEventSound(eventId);
 }
 
 NS_IMETHODIMP nsSound::PlayEventSound(uint32_t aEventId)
 {
-  ShutdownOldPlayerThread();
+  MOZ_ASSERT(mPlayerThread, "player thread should not be null ");
   PurgeLastSound();
 
   const wchar_t *sound = nullptr;
   switch (aEventId) {
     case EVENT_NEW_MAIL_RECEIVED:
       sound = L"MailBeep";
       break;
     case EVENT_ALERT_DIALOG_OPEN:
@@ -289,15 +328,16 @@ NS_IMETHODIMP nsSound::PlayEventSound(ui
       break;
     default:
       // Win32 plays no sounds at NS_SYSSOUND_PROMPT_DIALOG and
       // NS_SYSSOUND_SELECT_DIALOG.
       return NS_OK;
   }
   NS_ASSERTION(sound, "sound is null");
 
-  nsCOMPtr<nsIRunnable> player = new nsSoundPlayer(this, sound);
-  NS_ENSURE_TRUE(player, NS_ERROR_OUT_OF_MEMORY);
-  nsresult rv =
-    NS_NewNamedThread("PlayEventSound", getter_AddRefs(mPlayerThread), player);
-  NS_ENSURE_SUCCESS(rv, rv);
+  nsCOMPtr<nsIRunnable> player = new nsSoundPlayer(nsDependentString(sound));
+  MOZ_ASSERT(player, "Could not create player");
+  nsresult rv = mPlayerThread->Dispatch(player, NS_DISPATCH_NORMAL);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
   return NS_OK;
 }
--- a/widget/windows/nsSound.h
+++ b/widget/windows/nsSound.h
@@ -3,35 +3,43 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef __nsSound_h__
 #define __nsSound_h__
 
 #include "nsISound.h"
+#include "nsIObserver.h"
 #include "nsIStreamLoader.h"
 #include "nsCOMPtr.h"
+#include "mozilla/StaticPtr.h"
 
 class nsIThread;
 
-class nsSound : public nsISound,
-                public nsIStreamLoaderObserver
+class nsSound : public nsISound
+              , public nsIStreamLoaderObserver
+              , public nsIObserver
 
 {
-public: 
+public:
   nsSound();
-  void ShutdownOldPlayerThread();
+  static already_AddRefed<nsISound> GetInstance();
 
   NS_DECL_ISUPPORTS
   NS_DECL_NSISOUND
   NS_DECL_NSISTREAMLOADEROBSERVER
+  NS_DECL_NSIOBSERVER
 
 private:
   virtual ~nsSound();
   void PurgeLastSound();
 
 private:
-  uint8_t* mLastSound;
+  nsresult CreatePlayerThread();
+
   nsCOMPtr<nsIThread> mPlayerThread;
+  bool mInited;
+
+  static mozilla::StaticRefPtr<nsISound> sInstance;
 };
 
 #endif /* __nsSound_h__ */
--- a/widget/windows/nsWidgetFactory.cpp
+++ b/widget/windows/nsWidgetFactory.cpp
@@ -101,18 +101,18 @@ ColorPickerConstructor(nsISupports *aOut
   }
   nsCOMPtr<nsIColorPicker> picker = new nsColorPicker;
   return picker->QueryInterface(aIID, aResult);
 }
 
 NS_GENERIC_FACTORY_SINGLETON_CONSTRUCTOR(ScreenManager, ScreenManager::GetAddRefedSingleton)
 NS_GENERIC_FACTORY_SINGLETON_CONSTRUCTOR(nsIdleServiceWin, nsIdleServiceWin::GetInstance)
 NS_GENERIC_FACTORY_SINGLETON_CONSTRUCTOR(nsIClipboard, nsClipboard::GetInstance)
+NS_GENERIC_FACTORY_SINGLETON_CONSTRUCTOR(nsISound, nsSound::GetInstance)
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsClipboardHelper)
-NS_GENERIC_FACTORY_CONSTRUCTOR(nsSound)
 NS_GENERIC_FACTORY_CONSTRUCTOR(WinTaskbar)
 NS_GENERIC_FACTORY_CONSTRUCTOR(JumpListBuilder)
 NS_GENERIC_FACTORY_CONSTRUCTOR(JumpListItem)
 NS_GENERIC_FACTORY_CONSTRUCTOR(JumpListSeparator)
 NS_GENERIC_FACTORY_CONSTRUCTOR(JumpListLink)
 NS_GENERIC_FACTORY_CONSTRUCTOR(JumpListShortcut)
 NS_GENERIC_FACTORY_CONSTRUCTOR(WindowsUIUtils)
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsTransferable)
@@ -174,17 +174,17 @@ static const mozilla::Module::CIDEntry k
   { &kNS_APPSHELL_CID, false, nullptr, nsAppShellConstructor, Module::ALLOW_IN_GPU_PROCESS },
   { &kNS_SCREENMANAGER_CID, false, nullptr, ScreenManagerConstructor,
     Module::MAIN_PROCESS_ONLY },
   { &kNS_GFXINFO_CID, false, nullptr, GfxInfoConstructor },
   { &kNS_THEMERENDERER_CID, false, nullptr, NS_NewNativeTheme },
   { &kNS_IDLE_SERVICE_CID, false, nullptr, nsIdleServiceWinConstructor },
   { &kNS_CLIPBOARD_CID, false, nullptr, nsIClipboardConstructor, Module::MAIN_PROCESS_ONLY },
   { &kNS_CLIPBOARDHELPER_CID, false, nullptr, nsClipboardHelperConstructor },
-  { &kNS_SOUND_CID, false, nullptr, nsSoundConstructor, Module::MAIN_PROCESS_ONLY },
+  { &kNS_SOUND_CID, false, nullptr, nsISoundConstructor, Module::MAIN_PROCESS_ONLY },
   { &kNS_TRANSFERABLE_CID, false, nullptr, nsTransferableConstructor },
   { &kNS_HTMLFORMATCONVERTER_CID, false, nullptr, nsHTMLFormatConverterConstructor },
   { &kNS_WIN_TASKBAR_CID, false, nullptr, WinTaskbarConstructor },
   { &kNS_WIN_JUMPLISTBUILDER_CID, false, nullptr, JumpListBuilderConstructor },
   { &kNS_WIN_JUMPLISTITEM_CID, false, nullptr, JumpListItemConstructor },
   { &kNS_WIN_JUMPLISTSEPARATOR_CID, false, nullptr, JumpListSeparatorConstructor },
   { &kNS_WIN_JUMPLISTLINK_CID, false, nullptr, JumpListLinkConstructor },
   { &kNS_WIN_JUMPLISTSHORTCUT_CID, false, nullptr, JumpListShortcutConstructor },