Bug 1007398 - Properly handle racing destruction of PTelephonyChild actors, r=hsinyi.

dom/telephony/ipc/TelephonyChild.cpp

@@ -3,7 +3,8 @@
  * 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/. */
 
-#include "mozilla/dom/telephony/TelephonyChild.h"
+#include "TelephonyChild.h"
+#include "TelephonyIPCProvider.h"
 
 USING_TELEPHONY_NAMESPACE
 
@@ -11,16 +12,23 @@ USING_TELEPHONY_NAMESPACE
  * TelephonyChild
  ******************************************************************************/
 
-TelephonyChild::TelephonyChild(nsITelephonyListener* aListener)
-  : mListener(aListener)
+TelephonyChild::TelephonyChild(TelephonyIPCProvider* aProvider)
+  : mProvider(aProvider)
+{
+  MOZ_ASSERT(aProvider);
+}
+
+TelephonyChild::~TelephonyChild()
 {
-  MOZ_ASSERT(aListener);
 }
 
 void
 TelephonyChild::ActorDestroy(ActorDestroyReason aWhy)
 {
-  mListener = nullptr;
+  if (mProvider) {
+    mProvider->NoteActorDestroyed();
+    mProvider = nullptr;
+  }
 }
 
 PTelephonyRequestChild*
@@ -41,9 +49,9 @@ TelephonyChild::RecvNotifyCallError(const uint32_t& aClientId,
                                     const int32_t& aCallIndex,
                                     const nsString& aError)
 {
-  MOZ_ASSERT(mListener);
+  MOZ_ASSERT(mProvider);
 
-  mListener->NotifyError(aClientId, aCallIndex, aError);
+  mProvider->NotifyError(aClientId, aCallIndex, aError);
   return true;
 }
 
@@ -51,9 +59,9 @@ bool
 TelephonyChild::RecvNotifyCallStateChanged(const uint32_t& aClientId,
                                            const IPCCallStateData& aData)
 {
-  MOZ_ASSERT(mListener);
+  MOZ_ASSERT(mProvider);
 
-  mListener->CallStateChanged(aClientId,
+  mProvider->CallStateChanged(aClientId,
                               aData.callIndex(),
                               aData.callState(),
                               aData.number(),
@@ -70,18 +78,18 @@ bool
 TelephonyChild::RecvNotifyCdmaCallWaiting(const uint32_t& aClientId,
                                           const nsString& aNumber)
 {
-  MOZ_ASSERT(mListener);
+  MOZ_ASSERT(mProvider);
 
-  mListener->NotifyCdmaCallWaiting(aClientId, aNumber);
+  mProvider->NotifyCdmaCallWaiting(aClientId, aNumber);
   return true;
 }
 
 bool
 TelephonyChild::RecvNotifyConferenceCallStateChanged(const uint16_t& aCallState)
 {
-  MOZ_ASSERT(mListener);
+  MOZ_ASSERT(mProvider);
 
-  mListener->ConferenceCallStateChanged(aCallState);
+  mProvider->ConferenceCallStateChanged(aCallState);
   return true;
 }
 
@@ -89,9 +97,9 @@ bool
 TelephonyChild::RecvNotifyConferenceError(const nsString& aName,
                                           const nsString& aMessage)
 {
-  MOZ_ASSERT(mListener);
+  MOZ_ASSERT(mProvider);
 
-  mListener->NotifyConferenceError(aName, aMessage);
+  mProvider->NotifyConferenceError(aName, aMessage);
   return true;
 }
 
@@ -100,9 +108,9 @@ TelephonyChild::RecvNotifySupplementaryService(const uint32_t& aClientId,
                                                const int32_t& aCallIndex,
                                                const uint16_t& aNotification)
 {
-  MOZ_ASSERT(mListener);
+  MOZ_ASSERT(mProvider);
 
-  mListener->SupplementaryServiceNotification(aClientId, aCallIndex,
+  mProvider->SupplementaryServiceNotification(aClientId, aCallIndex,
                                               aNotification);
   return true;
 }

dom/telephony/ipc/TelephonyChild.h

@@ -13,13 +13,15 @@
 
 BEGIN_TELEPHONY_NAMESPACE
 
+class TelephonyIPCProvider;
+
 class TelephonyChild : public PTelephonyChild
 {
 public:
-  TelephonyChild(nsITelephonyListener* aListener);
+  TelephonyChild(TelephonyIPCProvider* aProvider);
 
 protected:
-  virtual ~TelephonyChild() {}
+  virtual ~TelephonyChild();
 
   virtual void
   ActorDestroy(ActorDestroyReason aWhy) MOZ_OVERRIDE;
@@ -55,7 +57,7 @@ protected:
                                  const uint16_t& aNotification) MOZ_OVERRIDE;
 
 private:
-  nsCOMPtr<nsITelephonyListener> mListener;
+  nsRefPtr<TelephonyIPCProvider> mProvider;
 };
 
 class TelephonyRequestChild : public PTelephonyRequestChild

dom/telephony/ipc/TelephonyIPCProvider.cpp

@@ -3,7 +3,7 @@
  * 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/. */
 
-#include "ipc/TelephonyIPCProvider.h"
+#include "TelephonyIPCProvider.h"
 
 #include "mozilla/dom/ContentChild.h"
 #include "mozilla/dom/telephony/TelephonyChild.h"
@@ -53,7 +53,17 @@ TelephonyIPCProvider::TelephonyIPCProvider()
 
 TelephonyIPCProvider::~TelephonyIPCProvider()
 {
-  mPTelephonyChild->Send__delete__(mPTelephonyChild);
+  if (mPTelephonyChild) {
+    mPTelephonyChild->Send__delete__(mPTelephonyChild);
+    mPTelephonyChild = nullptr;
+  }
+}
+
+void
+TelephonyIPCProvider::NoteActorDestroyed()
+{
+  MOZ_ASSERT(mPTelephonyChild);
+
   mPTelephonyChild = nullptr;
 }
 
@@ -94,6 +104,11 @@ TelephonyIPCProvider::RegisterListener(nsITelephonyListener *aListener)
 {
   MOZ_ASSERT(!mListeners.Contains(aListener));
 
+  if (!mPTelephonyChild) {
+    NS_WARNING("TelephonyProvider used after shutdown has begun!");
+    return NS_ERROR_FAILURE;
+  }
+
   // nsTArray doesn't fail.
   mListeners.AppendElement(aListener);
 
@@ -108,6 +123,11 @@ TelephonyIPCProvider::UnregisterListener(nsITelephonyListener *aListener)
 {
   MOZ_ASSERT(mListeners.Contains(aListener));
 
+  if (!mPTelephonyChild) {
+    NS_WARNING("TelephonyProvider used after shutdown has begun!");
+    return NS_ERROR_FAILURE;
+  }
+
   // We always have the element here, so it can't fail.
   mListeners.RemoveElement(aListener);
 
@@ -122,6 +142,11 @@ TelephonyIPCProvider::SendRequest(nsITelephonyListener *aListener,
                                   nsITelephonyCallback *aCallback,
                                   const IPCTelephonyRequest& aRequest)
 {
+  if (!mPTelephonyChild) {
+    NS_WARNING("TelephonyProvider used after shutdown has begun!");
+    return NS_ERROR_FAILURE;
+  }
+
   // Life time of newly allocated TelephonyRequestChild instance is managed by
   // IPDL itself.
   TelephonyRequestChild* actor = new TelephonyRequestChild(aListener, aCallback);
@@ -146,6 +171,11 @@ TelephonyIPCProvider::Dial(uint32_t aClientId, const nsAString& aNumber,
 NS_IMETHODIMP
 TelephonyIPCProvider::HangUp(uint32_t aClientId, uint32_t aCallIndex)
 {
+  if (!mPTelephonyChild) {
+    NS_WARNING("TelephonyProvider used after shutdown has begun!");
+    return NS_ERROR_FAILURE;
+  }
+
   mPTelephonyChild->SendHangUpCall(aClientId, aCallIndex);
   return NS_OK;
 }
@@ -153,6 +183,11 @@ TelephonyIPCProvider::HangUp(uint32_t aClientId, uint32_t aCallIndex)
 NS_IMETHODIMP
 TelephonyIPCProvider::AnswerCall(uint32_t aClientId, uint32_t aCallIndex)
 {
+  if (!mPTelephonyChild) {
+    NS_WARNING("TelephonyProvider used after shutdown has begun!");
+    return NS_ERROR_FAILURE;
+  }
+
   mPTelephonyChild->SendAnswerCall(aClientId, aCallIndex);
   return NS_OK;
 }
@@ -160,6 +195,11 @@ TelephonyIPCProvider::AnswerCall(uint32_t aClientId, uint32_t aCallIndex)
 NS_IMETHODIMP
 TelephonyIPCProvider::RejectCall(uint32_t aClientId, uint32_t aCallIndex)
 {
+  if (!mPTelephonyChild) {
+    NS_WARNING("TelephonyProvider used after shutdown has begun!");
+    return NS_ERROR_FAILURE;
+  }
+
   mPTelephonyChild->SendRejectCall(aClientId, aCallIndex);
   return NS_OK;
 }
@@ -167,6 +207,11 @@ TelephonyIPCProvider::RejectCall(uint32_t aClientId, uint32_t aCallIndex)
 NS_IMETHODIMP
 TelephonyIPCProvider::HoldCall(uint32_t aClientId, uint32_t aCallIndex)
 {
+  if (!mPTelephonyChild) {
+    NS_WARNING("TelephonyProvider used after shutdown has begun!");
+    return NS_ERROR_FAILURE;
+  }
+
   mPTelephonyChild->SendHoldCall(aClientId, aCallIndex);
   return NS_OK;
 }
@@ -174,6 +219,11 @@ TelephonyIPCProvider::HoldCall(uint32_t aClientId, uint32_t aCallIndex)
 NS_IMETHODIMP
 TelephonyIPCProvider::ResumeCall(uint32_t aClientId, uint32_t aCallIndex)
 {
+  if (!mPTelephonyChild) {
+    NS_WARNING("TelephonyProvider used after shutdown has begun!");
+    return NS_ERROR_FAILURE;
+  }
+
   mPTelephonyChild->SendResumeCall(aClientId, aCallIndex);
   return NS_OK;
 }
@@ -181,6 +231,11 @@ TelephonyIPCProvider::ResumeCall(uint32_t aClientId, uint32_t aCallIndex)
 NS_IMETHODIMP
 TelephonyIPCProvider::ConferenceCall(uint32_t aClientId)
 {
+  if (!mPTelephonyChild) {
+    NS_WARNING("TelephonyProvider used after shutdown has begun!");
+    return NS_ERROR_FAILURE;
+  }
+
   mPTelephonyChild->SendConferenceCall(aClientId);
   return NS_OK;
 }
@@ -188,6 +243,11 @@ TelephonyIPCProvider::ConferenceCall(uint32_t aClientId)
 NS_IMETHODIMP
 TelephonyIPCProvider::SeparateCall(uint32_t aClientId, uint32_t aCallIndex)
 {
+  if (!mPTelephonyChild) {
+    NS_WARNING("TelephonyProvider used after shutdown has begun!");
+    return NS_ERROR_FAILURE;
+  }
+
   mPTelephonyChild->SendSeparateCall(aClientId, aCallIndex);
   return NS_OK;
 }
@@ -195,6 +255,11 @@ TelephonyIPCProvider::SeparateCall(uint32_t aClientId, uint32_t aCallIndex)
 NS_IMETHODIMP
 TelephonyIPCProvider::HoldConference(uint32_t aClientId)
 {
+  if (!mPTelephonyChild) {
+    NS_WARNING("TelephonyProvider used after shutdown has begun!");
+    return NS_ERROR_FAILURE;
+  }
+
   mPTelephonyChild->SendHoldConference(aClientId);
   return NS_OK;
 }
@@ -202,6 +267,11 @@ TelephonyIPCProvider::HoldConference(uint32_t aClientId)
 NS_IMETHODIMP
 TelephonyIPCProvider::ResumeConference(uint32_t aClientId)
 {
+  if (!mPTelephonyChild) {
+    NS_WARNING("TelephonyProvider used after shutdown has begun!");
+    return NS_ERROR_FAILURE;
+  }
+
   mPTelephonyChild->SendResumeConference(aClientId);
   return NS_OK;
 }
@@ -209,6 +279,11 @@ TelephonyIPCProvider::ResumeConference(uint32_t aClientId)
 NS_IMETHODIMP
 TelephonyIPCProvider::StartTone(uint32_t aClientId, const nsAString& aDtmfChar)
 {
+  if (!mPTelephonyChild) {
+    NS_WARNING("TelephonyProvider used after shutdown has begun!");
+    return NS_ERROR_FAILURE;
+  }
+
   mPTelephonyChild->SendStartTone(aClientId, nsString(aDtmfChar));
   return NS_OK;
 }
@@ -216,6 +291,11 @@ TelephonyIPCProvider::StartTone(uint32_t aClientId, const nsAString& aDtmfChar)
 NS_IMETHODIMP
 TelephonyIPCProvider::StopTone(uint32_t aClientId)
 {
+  if (!mPTelephonyChild) {
+    NS_WARNING("TelephonyProvider used after shutdown has begun!");
+    return NS_ERROR_FAILURE;
+  }
+
   mPTelephonyChild->SendStopTone(aClientId);
   return NS_OK;
 }
@@ -223,6 +303,11 @@ TelephonyIPCProvider::StopTone(uint32_t aClientId)
 NS_IMETHODIMP
 TelephonyIPCProvider::GetMicrophoneMuted(bool* aMuted)
 {
+  if (!mPTelephonyChild) {
+    NS_WARNING("TelephonyProvider used after shutdown has begun!");
+    return NS_ERROR_FAILURE;
+  }
+
   mPTelephonyChild->SendGetMicrophoneMuted(aMuted);
   return NS_OK;
 }
@@ -230,6 +315,11 @@ TelephonyIPCProvider::GetMicrophoneMuted(bool* aMuted)
 NS_IMETHODIMP
 TelephonyIPCProvider::SetMicrophoneMuted(bool aMuted)
 {
+  if (!mPTelephonyChild) {
+    NS_WARNING("TelephonyProvider used after shutdown has begun!");
+    return NS_ERROR_FAILURE;
+  }
+
   mPTelephonyChild->SendSetMicrophoneMuted(aMuted);
   return NS_OK;
 }
@@ -237,6 +327,11 @@ TelephonyIPCProvider::SetMicrophoneMuted(bool aMuted)
 NS_IMETHODIMP
 TelephonyIPCProvider::GetSpeakerEnabled(bool* aEnabled)
 {
+  if (!mPTelephonyChild) {
+    NS_WARNING("TelephonyProvider used after shutdown has begun!");
+    return NS_ERROR_FAILURE;
+  }
+
   mPTelephonyChild->SendGetSpeakerEnabled(aEnabled);
   return NS_OK;
 }
@@ -244,6 +339,11 @@ TelephonyIPCProvider::GetSpeakerEnabled(bool* aEnabled)
 NS_IMETHODIMP
 TelephonyIPCProvider::SetSpeakerEnabled(bool aEnabled)
 {
+  if (!mPTelephonyChild) {
+    NS_WARNING("TelephonyProvider used after shutdown has begun!");
+    return NS_ERROR_FAILURE;
+  }
+
   mPTelephonyChild->SendSetSpeakerEnabled(aEnabled);
   return NS_OK;
 }

dom/telephony/ipc/TelephonyIPCProvider.h

@@ -28,10 +28,11 @@ public:
 
   TelephonyIPCProvider();
 
-protected:
-  virtual ~TelephonyIPCProvider();
+  void NoteActorDestroyed();
 
 private:
+  ~TelephonyIPCProvider();
+
   nsTArray<nsCOMPtr<nsITelephonyListener> > mListeners;
   PTelephonyChild* mPTelephonyChild;
   uint32_t mDefaultServiceId;