Bug 795658 - Make sure most methods are running from main thread. r=dougt
authorKan-Ru Chen (陳侃如) <kanru@kanru.info>
Thu, 04 Oct 2012 10:00:59 +0800
changeset 115486 3d6caa803dca37534fd4d4c8d1139579813b6bd3
parent 115485 16ac5cbc7793e3271fbe0467fbd29428ce783ef2
child 115487 d406bd8dc895eb5dfa2e9f6cbb2dee6b2eba40fa
push id1
push usersledru@mozilla.com
push dateThu, 04 Dec 2014 17:57:20 +0000
reviewersdougt
bugs795658
milestone18.0a1
Bug 795658 - Make sure most methods are running from main thread. r=dougt
dom/system/gonk/GonkGPSGeolocationProvider.cpp
dom/system/gonk/GonkGPSGeolocationProvider.h
--- a/dom/system/gonk/GonkGPSGeolocationProvider.cpp
+++ b/dom/system/gonk/GonkGPSGeolocationProvider.cpp
@@ -32,17 +32,20 @@
 #endif
 
 #define DEBUG_GPS 0
 
 using namespace mozilla;
 
 static const int kDefaultPeriod = 1000; // ms
 
-NS_IMPL_ISUPPORTS2(GonkGPSGeolocationProvider, nsIGeolocationProvider, nsIRILDataCallback)
+// While most methods of GonkGPSGeolocationProvider should only be
+// called from main thread, we deliberately put the Init and ShutdownGPS
+// methods off main thread to avoid blocking.
+NS_IMPL_THREADSAFE_ISUPPORTS2(GonkGPSGeolocationProvider, nsIGeolocationProvider, nsIRILDataCallback)
 
 GonkGPSGeolocationProvider* GonkGPSGeolocationProvider::sSingleton;
 GpsCallbacks GonkGPSGeolocationProvider::mCallbacks = {
   sizeof(GpsCallbacks),
   LocationCallback,
   StatusCallback,
   SvStatusCallback,
   NmeaCallback,
@@ -121,29 +124,39 @@ GonkGPSGeolocationProvider::NmeaCallback
   printf_stderr("nmea:     \t%s\n", nmea);
   printf_stderr("length:   \t%d\n", length);
 #endif
 }
 
 void
 GonkGPSGeolocationProvider::SetCapabilitiesCallback(uint32_t capabilities)
 {
-  // Called by GPS engine in init(), hence we don't have to
-  // protect the memebers
-
-  nsRefPtr<GonkGPSGeolocationProvider> provider =
-    GonkGPSGeolocationProvider::GetSingleton();
+  class UpdateCapabilitiesEvent : public nsRunnable {
+  public:
+    UpdateCapabilitiesEvent(uint32_t aCapabilities)
+      : mCapabilities(aCapabilities)
+    {}
+    NS_IMETHOD Run() {
+      nsRefPtr<GonkGPSGeolocationProvider> provider =
+        GonkGPSGeolocationProvider::GetSingleton();
 
-  provider->mSupportsScheduling = capabilities & GPS_CAPABILITY_SCHEDULING;
-  provider->mSupportsMSB = capabilities & GPS_CAPABILITY_MSB;
-  provider->mSupportsMSA = capabilities & GPS_CAPABILITY_MSA;
-  provider->mSupportsSingleShot = capabilities & GPS_CAPABILITY_SINGLE_SHOT;
+      provider->mSupportsScheduling = mCapabilities & GPS_CAPABILITY_SCHEDULING;
+      provider->mSupportsMSB = mCapabilities & GPS_CAPABILITY_MSB;
+      provider->mSupportsMSA = mCapabilities & GPS_CAPABILITY_MSA;
+      provider->mSupportsSingleShot = mCapabilities & GPS_CAPABILITY_SINGLE_SHOT;
 #ifdef GPS_CAPABILITY_ON_DEMAND_TIME
-  provider->mSupportsTimeInjection = capabilities & GPS_CAPABILITY_ON_DEMAND_TIME;
+      provider->mSupportsTimeInjection = mCapabilities & GPS_CAPABILITY_ON_DEMAND_TIME;
 #endif
+      return NS_OK;
+    }
+  private:
+    uint32_t mCapabilities;
+  };
+
+  NS_DispatchToMainThread(new UpdateCapabilitiesEvent(capabilities));
 }
 
 void
 GonkGPSGeolocationProvider::AcquireWakelockCallback()
 {
 }
 
 void
@@ -176,31 +189,40 @@ GonkGPSGeolocationProvider::RequestUtcTi
 {
 }
 
 void
 GonkGPSGeolocationProvider::AGPSStatusCallback(AGpsStatus* status)
 {
   MOZ_ASSERT(status);
 
-  nsRefPtr<GonkGPSGeolocationProvider> provider =
-    GonkGPSGeolocationProvider::GetSingleton();
+  class AGPSStatusEvent : public nsRunnable {
+  public:
+    AGPSStatusEvent(AGpsStatusValue aStatus)
+      : mStatus(aStatus)
+    {}
+    NS_IMETHOD Run() {
+      nsRefPtr<GonkGPSGeolocationProvider> provider =
+        GonkGPSGeolocationProvider::GetSingleton();
 
-  nsCOMPtr<nsIRunnable> event;
-  switch (status->status) {
-    case GPS_REQUEST_AGPS_DATA_CONN:
-      event = NS_NewRunnableMethod(provider, &GonkGPSGeolocationProvider::RequestDataConnection);
-      break;
-    case GPS_RELEASE_AGPS_DATA_CONN:
-      event = NS_NewRunnableMethod(provider, &GonkGPSGeolocationProvider::ReleaseDataConnection);
-      break;
-  }
-  if (event) {
-    NS_DispatchToMainThread(event);
-  }
+      switch (mStatus) {
+        case GPS_REQUEST_AGPS_DATA_CONN:
+          provider->RequestDataConnection();
+          break;
+        case GPS_RELEASE_AGPS_DATA_CONN:
+          provider->ReleaseDataConnection();
+          break;
+      }
+      return NS_OK;
+    }
+  private:
+    AGpsStatusValue mStatus;
+  };
+
+  NS_DispatchToMainThread(new AGPSStatusEvent(status->status));
 }
 
 void
 GonkGPSGeolocationProvider::AGPSRILSetIDCallback(uint32_t flags)
 {
   class RequestSetIDEvent : public nsRunnable {
   public:
     RequestSetIDEvent(uint32_t flags)
@@ -240,23 +262,27 @@ GonkGPSGeolocationProvider::GonkGPSGeolo
   , mSupportsSingleShot(false)
   , mSupportsTimeInjection(false)
   , mGpsInterface(nullptr)
 {
 }
 
 GonkGPSGeolocationProvider::~GonkGPSGeolocationProvider()
 {
-  ShutdownNow();
+  MOZ_ASSERT(NS_IsMainThread());
+  MOZ_ASSERT(!mStarted, "Must call Shutdown before destruction");
+
   sSingleton = nullptr;
 }
 
 already_AddRefed<GonkGPSGeolocationProvider>
 GonkGPSGeolocationProvider::GetSingleton()
 {
+  MOZ_ASSERT(NS_IsMainThread());
+
   if (!sSingleton)
     sSingleton = new GonkGPSGeolocationProvider();
 
   NS_ADDREF(sSingleton);
   return sSingleton;
 }
 
 const GpsInterface*
@@ -278,16 +304,18 @@ GonkGPSGeolocationProvider::GetGPSInterf
     return nullptr;
   }
   return result;
 }
 
 void
 GonkGPSGeolocationProvider::RequestDataConnection()
 {
+  MOZ_ASSERT(NS_IsMainThread());
+
   if (!mRIL) {
     return;
   }
 
   // TODO: Bug 772747 - We should ask NetworkManager or RIL to open
   // SUPL type connection for us.
   const nsAdoptingString& apnName = Preferences::GetString("geo.gps.apn.name");
   const nsAdoptingString& apnUser = Preferences::GetString("geo.gps.apn.user");
@@ -299,16 +327,18 @@ GonkGPSGeolocationProvider::RequestDataC
                         3 /* DATACALL_AUTH_PAP_OR_CHAP */,
                         NS_LITERAL_STRING("IP") /* pdptype */);
   }
 }
 
 void
 GonkGPSGeolocationProvider::ReleaseDataConnection()
 {
+  MOZ_ASSERT(NS_IsMainThread());
+
   if (!mRIL) {
     return;
   }
 
   if (mCid.IsEmpty()) {
     // We didn't request data call or the data call failed, bail out.
     return;
   }
@@ -416,16 +446,17 @@ GonkGPSGeolocationProvider::Init()
   }
 
   NS_DispatchToMainThread(NS_NewRunnableMethod(this, &GonkGPSGeolocationProvider::StartGPS));
 }
 
 void
 GonkGPSGeolocationProvider::StartGPS()
 {
+  MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(mGpsInterface);
 
   int32_t update = Preferences::GetInt("geo.default.update", kDefaultPeriod);
 
   if (mSupportsMSA || mSupportsMSB) {
     SetupAGPS();
   }
 
@@ -452,17 +483,17 @@ GonkGPSGeolocationProvider::StartGPS()
 
   mGpsInterface->start();
 }
 
 void
 GonkGPSGeolocationProvider::SetupAGPS()
 {
   MOZ_ASSERT(NS_IsMainThread());
-  MOZ_ASSERT(mAGpsRilInterface);
+  MOZ_ASSERT(mAGpsInterface);
 
   const nsAdoptingCString& suplServer = Preferences::GetCString("geo.gps.supl_server");
   int32_t suplPort = Preferences::GetInt("geo.gps.supl_port", -1);
   if (!suplServer.IsEmpty() && suplPort > 0) {
     mAGpsInterface->set_server(AGPS_TYPE_SUPL, suplServer.get(), suplPort);
   } else {
     NS_WARNING("Cannot get SUPL server settings");
     return;
@@ -478,16 +509,18 @@ GonkGPSGeolocationProvider::SetupAGPS()
   }
 
   return;
 }
 
 NS_IMETHODIMP
 GonkGPSGeolocationProvider::Startup()
 {
+  MOZ_ASSERT(NS_IsMainThread());
+
   if (mStarted) {
     return NS_OK;
   }
 
   if (!mInitThread) {
     nsresult rv = NS_NewThread(getter_AddRefs(mInitThread));
     NS_ENSURE_SUCCESS(rv, rv);
   }
@@ -497,46 +530,47 @@ GonkGPSGeolocationProvider::Startup()
 
   mStarted = true;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 GonkGPSGeolocationProvider::Watch(nsIGeolocationUpdate* aCallback)
 {
+  MOZ_ASSERT(NS_IsMainThread());
+
   mLocationCallback = aCallback;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 GonkGPSGeolocationProvider::Shutdown()
 {
+  MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(mInitThread);
 
   if (!mStarted) {
     return NS_OK;
   }
+  mStarted = false;
 
-  mInitThread->Dispatch(NS_NewRunnableMethod(this, &GonkGPSGeolocationProvider::ShutdownNow),
+  if (mRIL) {
+    mRIL->UnregisterDataCallCallback(this);
+  }
+
+  mInitThread->Dispatch(NS_NewRunnableMethod(this, &GonkGPSGeolocationProvider::ShutdownGPS),
                         NS_DISPATCH_NORMAL);
 
   return NS_OK;
 }
 
 void
-GonkGPSGeolocationProvider::ShutdownNow()
+GonkGPSGeolocationProvider::ShutdownGPS()
 {
-  if (!mStarted) {
-    return;
-  }
-  mStarted = false;
-
-  if (mRIL) {
-    mRIL->UnregisterDataCallCallback(this);
-  }
+  MOZ_ASSERT(!mStarted, "Should only be called after Shutdown");
 
   if (mGpsInterface) {
     mGpsInterface->stop();
     mGpsInterface->cleanup();
   }
 
   mInitThread = nullptr;
 }
@@ -547,16 +581,17 @@ GonkGPSGeolocationProvider::SetHighAccur
   return NS_OK;
 }
 
 /** nsIRILDataCallback interface **/
 
 NS_IMETHODIMP
 GonkGPSGeolocationProvider::DataCallStateChanged(nsIRILDataCallInfo* aDataCall)
 {
+  MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aDataCall);
   MOZ_ASSERT(mAGpsInterface);
   nsCOMPtr<nsIRILDataCallInfo> datacall = aDataCall;
 
   uint32_t callState;
   nsresult rv = datacall->GetState(&callState);
   NS_ENSURE_SUCCESS(rv, rv);
 
--- a/dom/system/gonk/GonkGPSGeolocationProvider.h
+++ b/dom/system/gonk/GonkGPSGeolocationProvider.h
@@ -58,17 +58,17 @@ private:
 
   static GpsCallbacks mCallbacks;
   static AGpsCallbacks mAGPSCallbacks;
   static AGpsRilCallbacks mAGPSRILCallbacks;
 
   void Init();
   void SetupAGPS();
   void StartGPS();
-  void ShutdownNow();
+  void ShutdownGPS();
   void RequestDataConnection();
   void ReleaseDataConnection();
   void RequestSetID(uint32_t flags);
   void SetReferenceLocation();
 
   const GpsInterface* GetGPSInterface();
 
   static GonkGPSGeolocationProvider* sSingleton;