Bug 1346872 - part2 : only access agent related codes in nsNPAPIPluginInstance. draft
authorAlastor Wu <alwu@mozilla.com>
Thu, 30 Mar 2017 14:27:42 +0800
changeset 553509 5a26483edd2fb8d03bf08c971dc64ca8a01949a6
parent 553508 3d25a41f5f6ce519fbfd061307c1666a9f097dea
child 553510 45fdf691f72ef70be35be39c92ef58a312aa79ef
push id51658
push useralwu@mozilla.com
push dateThu, 30 Mar 2017 06:27:26 +0000
bugs1346872
milestone55.0a1
Bug 1346872 - part2 : only access agent related codes in nsNPAPIPluginInstance. nsNPAPIPlugin doesn't need to know about agent, nsNPAPIPluginInstance should wrap all of the details in its memeber function. MozReview-Commit-ID: 3LqTlH2flbt
dom/plugins/base/nsNPAPIPlugin.cpp
dom/plugins/base/nsNPAPIPluginInstance.cpp
dom/plugins/base/nsNPAPIPluginInstance.h
--- a/dom/plugins/base/nsNPAPIPlugin.cpp
+++ b/dom/plugins/base/nsNPAPIPlugin.cpp
@@ -2220,63 +2220,29 @@ NPError
     }
 
     case NPPVpluginUsesDOMForCursorBool: {
       bool useDOMForCursor = (result != nullptr);
       return inst->SetUsesDOMForCursor(useDOMForCursor);
     }
 
     case NPPVpluginIsPlayingAudio: {
-      bool isMuted = !result;
+      const bool isPlaying = result;
 
       nsNPAPIPluginInstance* inst = (nsNPAPIPluginInstance*) npp->ndata;
       MOZ_ASSERT(inst);
 
-      if (isMuted && !inst->HasAudioChannelAgent()) {
-        return NPERR_NO_ERROR;
-      }
-
-      nsCOMPtr<nsIAudioChannelAgent> agent;
-      nsresult rv = inst->GetOrCreateAudioChannelAgent(getter_AddRefs(agent));
-      if (NS_WARN_IF(NS_FAILED(rv))) {
+      if (!isPlaying && !inst->HasAudioChannelAgent()) {
         return NPERR_NO_ERROR;
       }
 
-      MOZ_ASSERT(agent);
-
-      if (isMuted) {
-        rv = agent->NotifyStoppedPlaying();
-        if (NS_WARN_IF(NS_FAILED(rv))) {
-          return NPERR_NO_ERROR;
-        }
+      if (isPlaying) {
+        inst->NotifyStartedPlaying();
       } else {
-
-        dom::AudioPlaybackConfig config;
-        rv = agent->NotifyStartedPlaying(&config,
-                                         dom::AudioChannelService::AudibleState::eAudible);
-        if (NS_WARN_IF(NS_FAILED(rv))) {
-          return NPERR_NO_ERROR;
-        }
-
-        rv = inst->WindowVolumeChanged(config.mVolume, config.mMuted);
-        if (NS_WARN_IF(NS_FAILED(rv))) {
-          return NPERR_NO_ERROR;
-        }
-
-        // Since we only support for muting now, the implementation of suspend
-        // is equal to muting. Therefore, if we have already muted the plugin,
-        // then we don't need to call WindowSuspendChanged() again.
-        if (config.mMuted) {
-          return NPERR_NO_ERROR;
-        }
-
-        rv = inst->WindowSuspendChanged(config.mSuspend);
-        if (NS_WARN_IF(NS_FAILED(rv))) {
-          return NPERR_NO_ERROR;
-        }
+        inst->NotifyStoppedPlaying();
       }
 
       return NPERR_NO_ERROR;
     }
 
 #ifndef MOZ_WIDGET_ANDROID
     // On android, their 'drawing model' uses the same constant!
     case NPPVpluginDrawingModel: {
--- a/dom/plugins/base/nsNPAPIPluginInstance.cpp
+++ b/dom/plugins/base/nsNPAPIPluginInstance.cpp
@@ -1708,41 +1708,87 @@ nsNPAPIPluginInstance::GetRunID(uint32_t
   if (!library) {
     return NS_ERROR_FAILURE;
   }
 
   return library->GetRunID(aRunID);
 }
 
 nsresult
-nsNPAPIPluginInstance::GetOrCreateAudioChannelAgent(nsIAudioChannelAgent** aAgent)
+nsNPAPIPluginInstance::CreateAudioChannelAgentIfNeeded()
 {
-  if (!mAudioChannelAgent) {
-    nsresult rv;
-    mAudioChannelAgent = do_CreateInstance("@mozilla.org/audiochannelagent;1", &rv);
-    if (NS_WARN_IF(!mAudioChannelAgent)) {
-      return NS_ERROR_FAILURE;
-    }
+  if (mAudioChannelAgent) {
+    return NS_OK;
+  }
+
+  nsresult rv;
+  mAudioChannelAgent = do_CreateInstance("@mozilla.org/audiochannelagent;1", &rv);
+  if (NS_WARN_IF(!mAudioChannelAgent)) {
+    return NS_ERROR_FAILURE;
+  }
 
-    nsCOMPtr<nsPIDOMWindowOuter> window = GetDOMWindow();
-    if (NS_WARN_IF(!window)) {
-      return NS_ERROR_FAILURE;
-    }
+  nsCOMPtr<nsPIDOMWindowOuter> window = GetDOMWindow();
+  if (NS_WARN_IF(!window)) {
+    return NS_ERROR_FAILURE;
+  }
 
-    rv = mAudioChannelAgent->Init(window->GetCurrentInnerWindow(),
-                                 (int32_t)AudioChannelService::GetDefaultAudioChannel(),
-                                 this);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
-    }
+  rv = mAudioChannelAgent->Init(window->GetCurrentInnerWindow(),
+                               (int32_t)AudioChannelService::GetDefaultAudioChannel(),
+                               this);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+  return NS_OK;
+}
+
+void
+nsNPAPIPluginInstance::NotifyStartedPlaying()
+{
+  nsresult rv = CreateAudioChannelAgentIfNeeded();
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return;
   }
 
-  nsCOMPtr<nsIAudioChannelAgent> agent = mAudioChannelAgent;
-  agent.forget(aAgent);
-  return NS_OK;
+  MOZ_ASSERT(mAudioChannelAgent);
+  dom::AudioPlaybackConfig config;
+  rv = mAudioChannelAgent->NotifyStartedPlaying(&config,
+                                                dom::AudioChannelService::AudibleState::eAudible);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return;
+  }
+
+  rv = WindowVolumeChanged(config.mVolume, config.mMuted);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return;
+  }
+
+  // Since we only support muting for now, the implementation of suspend
+  // is equal to muting. Therefore, if we have already muted the plugin,
+  // then we don't need to call WindowSuspendChanged() again.
+  if (config.mMuted) {
+    return;
+  }
+
+  rv = WindowSuspendChanged(config.mSuspend);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return;
+  }
+}
+
+void
+nsNPAPIPluginInstance::NotifyStoppedPlaying()
+{
+  MOZ_ASSERT(mAudioChannelAgent);
+
+  // Reset the attribute.
+  mMuted = false;
+  nsresult rv = mAudioChannelAgent->NotifyStoppedPlaying();
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return;
+  }
 }
 
 NS_IMETHODIMP
 nsNPAPIPluginInstance::WindowVolumeChanged(float aVolume, bool aMuted)
 {
   MOZ_LOG(AudioChannelService::GetAudioChannelLog(), LogLevel::Debug,
          ("nsNPAPIPluginInstance, WindowVolumeChanged, "
           "this = %p, aVolume = %f, aMuted = %s\n",
--- a/dom/plugins/base/nsNPAPIPluginInstance.h
+++ b/dom/plugins/base/nsNPAPIPluginInstance.h
@@ -131,17 +131,18 @@ public:
   void SetOwner(nsPluginInstanceOwner *aOwner);
   void DidComposite();
 
   bool HasAudioChannelAgent() const
   {
     return !!mAudioChannelAgent;
   }
 
-  nsresult GetOrCreateAudioChannelAgent(nsIAudioChannelAgent** aAgent);
+  void NotifyStartedPlaying();
+  void NotifyStoppedPlaying();
 
   nsresult SetMuted(bool aIsMuted);
 
   nsNPAPIPlugin* GetPlugin();
 
   nsresult GetNPP(NPP * aNPP);
 
   NPError SetWindowless(bool aWindowless);
@@ -331,16 +332,18 @@ protected:
 
   nsresult GetTagType(nsPluginTagType *result);
 
   // check if this is a Java applet and affected by bug 750480
   void CheckJavaC2PJSObjectQuirk(uint16_t paramCount,
                                  const char* const* names,
                                  const char* const* values);
 
+  nsresult CreateAudioChannelAgentIfNeeded();
+
   // The structure used to communicate between the plugin instance and
   // the browser.
   NPP_t mNPP;
 
   NPDrawingModel mDrawingModel;
 
 #ifdef MOZ_WIDGET_ANDROID
   uint32_t mANPDrawingModel;