Bug 809288 - nsGeolocationService GetInstance / GetGeolocationService inconsistencies. r=mrbkap a=blocking-basecamp
authorDoug Turner <dougt@dougt.org>
Thu, 08 Nov 2012 15:36:50 -0800
changeset 116771 21922e8da10f03aa6cfdd7db62b14605576ca060
parent 116770 cb6d20b909c02d7a8a24562571f5dfde7a5450c7
child 116772 38bff1f270d015c391a0b245d2c79145ffda78d7
push id1708
push userakeybl@mozilla.com
push dateMon, 19 Nov 2012 21:10:21 +0000
treeherdermozilla-beta@27b14fe50103 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap, blocking-basecamp
bugs809288
milestone18.0a2
Bug 809288 - nsGeolocationService GetInstance / GetGeolocationService inconsistencies. r=mrbkap a=blocking-basecamp
dom/src/geolocation/nsGeolocation.cpp
dom/src/geolocation/nsGeolocation.h
--- a/dom/src/geolocation/nsGeolocation.cpp
+++ b/dom/src/geolocation/nsGeolocation.cpp
@@ -37,16 +37,17 @@
 #include "nsIPermissionManager.h"
 #include "nsIObserverService.h"
 #include "nsIJSContextStack.h"
 #include "nsThreadUtils.h"
 #include "mozilla/Services.h"
 #include "mozilla/unused.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/Attributes.h"
+#include "mozilla/ClearOnShutdown.h"
 
 #include <math.h>
 
 #ifdef MOZ_MAEMO_LIBLOCATION
 #include "MaemoLocationProvider.h"
 #endif
 
 #ifdef MOZ_ENABLE_QTMOBILITY
@@ -108,20 +109,19 @@ public:
 
     // Default it's enabled:
     MozSettingValue(true);
     return NS_OK;
   }
 
   void MozSettingValue(const bool aValue)
   {
-    // It is theoretically possible to shut down before the first settings check
-    // has completed (though extremely unlikely).
-    if (nsGeolocationService::gService) {
-      nsGeolocationService::gService->HandleMozsettingValue(aValue);
+    nsRefPtr<nsGeolocationService> gs = nsGeolocationService::GetGeolocationService();
+    if (gs) {
+      gs->HandleMozsettingValue(aValue);
     }
   }
 };
 
 NS_IMPL_ISUPPORTS1(GeolocationSettingsCallback, nsISettingsServiceCallback);
 
 class RequestPromptEvent : public nsRunnable
 {
@@ -379,28 +379,28 @@ nsGeolocationRequest::Cancel()
 
   NotifyError(nsIDOMGeoPositionError::PERMISSION_DENIED);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsGeolocationRequest::Allow()
 {
-  nsRefPtr<nsGeolocationService> geoService = nsGeolocationService::GetInstance();
+  nsRefPtr<nsGeolocationService> gs = nsGeolocationService::GetGeolocationService();
 
   // Kick off the geo device, if it isn't already running
-  nsresult rv = geoService->StartDevice();
+  nsresult rv = gs->StartDevice();
 
   if (NS_FAILED(rv)) {
     // Location provider error
     NotifyError(nsIDOMGeoPositionError::POSITION_UNAVAILABLE);
     return NS_OK;
   }
 
-  nsCOMPtr<nsIDOMGeoPosition> lastPosition = geoService->GetCachedPosition();
+  nsCOMPtr<nsIDOMGeoPosition> lastPosition = gs->GetCachedPosition();
   DOMTimeStamp cachedPositionTime;
   if (lastPosition) {
     lastPosition->GetTimestamp(&cachedPositionTime);
   }
 
   // check to see if we can use a cached value
   //
   // either:
@@ -409,17 +409,17 @@ nsGeolocationRequest::Allow()
   // b) the cached position time is some reasonable value to return to the user (<30s)
 
   uint32_t maximumAge = 30 * PR_MSEC_PER_SEC;
   if (mOptions) {
     if (mOptions->maximumAge >= 0) {
       maximumAge = mOptions->maximumAge;
     }
     if (mOptions->enableHighAccuracy) {
-      geoService->SetHigherAccuracy(true);
+      gs->SetHigherAccuracy(true);
     }
   }
 
   if (lastPosition && maximumAge > 0 &&
       ( PRTime(PR_Now() / PR_USEC_PER_MSEC) - maximumAge <=
         PRTime(cachedPositionTime) )) {
     // okay, we can return a cached position
     mAllowed = true;
@@ -517,19 +517,19 @@ nsGeolocationRequest::Update(nsIDOMGeoPo
   NS_DispatchToMainThread(ev);
   return true;
 }
 
 void
 nsGeolocationRequest::Shutdown()
 {
   if (mOptions && mOptions->enableHighAccuracy) {
-    nsRefPtr<nsGeolocationService> geoService = nsGeolocationService::GetInstance();
-    if (geoService) {
-      geoService->SetHigherAccuracy(false);
+    nsRefPtr<nsGeolocationService> gs = nsGeolocationService::GetGeolocationService();
+    if (gs) {
+      gs->SetHigherAccuracy(false);
     }
   }
 
   if (mTimeoutTimer) {
     mTimeoutTimer->Cancel();
     mTimeoutTimer = nullptr;
   }
   mCleared = true;
@@ -898,41 +898,34 @@ nsGeolocationService::StopDevice()
   for (int32_t i = 0; i < mProviders.Count(); i++) {
     mProviders[i]->Shutdown();
     obs->NotifyObservers(mProviders[i],
                          "geolocation-device-events",
                          NS_LITERAL_STRING("shutdown").get());
   }
 }
 
-nsGeolocationService* nsGeolocationService::gService = nullptr;
-
-nsGeolocationService*
-nsGeolocationService::GetInstance()
-{
-  if (!nsGeolocationService::gService) {
-    nsGeolocationService::gService = new nsGeolocationService();
-    NS_ASSERTION(nsGeolocationService::gService, "null nsGeolocationService.");
+nsRefPtr<nsGeolocationService> nsGeolocationService::sService;
 
-    if (nsGeolocationService::gService) {
-      if (NS_FAILED(nsGeolocationService::gService->Init())) {
-        delete nsGeolocationService::gService;
-        nsGeolocationService::gService = nullptr;
-      }
-    }
-  }
-  return nsGeolocationService::gService;
-}
-
-nsGeolocationService*
+already_AddRefed<nsGeolocationService>
 nsGeolocationService::GetGeolocationService()
 {
-  nsGeolocationService* inst = nsGeolocationService::GetInstance();
-  NS_IF_ADDREF(inst);
-  return inst;
+  nsRefPtr<nsGeolocationService> result;
+  if (nsGeolocationService::sService) {
+    result = nsGeolocationService::sService;
+    return result.forget();
+  }
+
+  result = new nsGeolocationService();
+  if (NS_FAILED(result->Init())) {
+    return nullptr;
+  }
+  ClearOnShutdown(&nsGeolocationService::sService);
+  nsGeolocationService::sService = result;
+  return result.forget();
 }
 
 void
 nsGeolocationService::AddLocator(nsGeolocation* aLocator)
 {
   mGeolocators.AppendElement(aLocator);
 }
 
@@ -1012,17 +1005,17 @@ nsGeolocation::Init(nsIDOMWindow* aConte
     }
 
     mPrincipal = doc->NodePrincipal();
   }
 
   // If no aContentDom was passed into us, we are being used
   // by chrome/c++ and have no mOwner, no mPrincipal, and no need
   // to prompt.
-  mService = nsGeolocationService::GetInstance();
+  mService = nsGeolocationService::GetGeolocationService();
   if (mService) {
     mService->AddLocator(this);
   }
   return NS_OK;
 }
 
 void
 nsGeolocation::Shutdown()
--- a/dom/src/geolocation/nsGeolocation.h
+++ b/dom/src/geolocation/nsGeolocation.h
@@ -93,19 +93,18 @@ class nsGeolocationRequest
 
 /**
  * Singleton that manages the geolocation provider
  */
 class nsGeolocationService MOZ_FINAL : public nsIGeolocationUpdate, public nsIObserver
 {
 public:
 
-  static nsGeolocationService* GetGeolocationService();
-  static nsGeolocationService* GetInstance();  // Non-Addref'ing
-  static nsGeolocationService* gService;
+  static already_AddRefed<nsGeolocationService> GetGeolocationService();
+  static nsRefPtr<nsGeolocationService> sService;
 
   NS_DECL_ISUPPORTS
   NS_DECL_NSIGEOLOCATIONUPDATE
   NS_DECL_NSIOBSERVER
 
   nsGeolocationService() {
       mHigherAccuracy = false;
   }