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 109172 3d6caa803dca37534fd4d4c8d1139579813b6bd3
parent 109171 16ac5cbc7793e3271fbe0467fbd29428ce783ef2
child 109173 d406bd8dc895eb5dfa2e9f6cbb2dee6b2eba40fa
push id15882
push userkchen@mozilla.com
push dateThu, 04 Oct 2012 02:01:15 +0000
treeherdermozilla-inbound@3d6caa803dca [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdougt
bugs795658
milestone18.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 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;