Bug 645325 - Part 2: Remove the two-second startup delay for the MLS fallback provider on Windows and macOS. r=garvank r?jdm draft
authorChris Peterson <cpeterson@mozilla.com>
Wed, 28 Feb 2018 23:59:03 -0800
changeset 791839 c49aae50f4ae12883e8b7392239a444ce2c45869
parent 791838 2d9fa5d2cc86c3c0774bee476163a3a9791c982b
child 791840 82066999ffe10b90eea6d38dea9e255cdb597c19
push id108905
push usercpeterson@mozilla.com
push dateSun, 06 May 2018 06:27:00 +0000
reviewersgarvank, jdm
bugs645325
milestone61.0a1
Bug 645325 - Part 2: Remove the two-second startup delay for the MLS fallback provider on Windows and macOS. r=garvank r?jdm The Windows and macOS location providers used to start the MLS provider (with a two-second delay) and then call the operating system's location provider, letting them race. Currently, however, we only start the MLS fallback provider after the system provider returns an error, so the two-second startup delay is just wasted time. I left the starup delay option in MLSFallback because the Linux gpsd provider still uses it to race MLS with the system's GPS provider. MozReview-Commit-ID: 3ZFeF1g6PoG
dom/geolocation/MLSFallback.cpp
dom/system/mac/CoreLocationLocationProvider.mm
dom/system/windows/WindowsLocationProvider.cpp
--- a/dom/geolocation/MLSFallback.cpp
+++ b/dom/geolocation/MLSFallback.cpp
@@ -23,16 +23,22 @@ MLSFallback::~MLSFallback()
 nsresult
 MLSFallback::Startup(nsIGeolocationUpdate* aWatcher)
 {
   if (mHandoffTimer || mMLSFallbackProvider) {
     return NS_OK;
   }
 
   mUpdateWatcher = aWatcher;
+
+  // No need to schedule a timer callback if there is no startup delay.
+  if (mDelayMs == 0) {
+    return CreateMLSFallbackProvider();
+  }
+
   return NS_NewTimerWithCallback(getter_AddRefs(mHandoffTimer),
                                  this, mDelayMs, nsITimer::TYPE_ONE_SHOT);
 }
 
 nsresult
 MLSFallback::Shutdown()
 {
   mUpdateWatcher = nullptr;
--- a/dom/system/mac/CoreLocationLocationProvider.mm
+++ b/dom/system/mac/CoreLocationLocationProvider.mm
@@ -69,22 +69,17 @@ static const CLLocationAccuracy kDEFAULT
 
   if ([aError code] == kCLErrorDenied) {
     mProvider->NotifyError(nsIDOMGeoPositionError::PERMISSION_DENIED);
     return;
   }
 
   // The CL provider does not fallback to GeoIP, so use NetworkGeolocationProvider for this.
   // The concept here is: on error, hand off geolocation to MLS, which will then report
-  // back a location or error. We can't call this with no delay however, as this method
-  // is called with an error code of 0 in both failed geolocation cases, and also when
-  // geolocation is not immediately available.
-  // The 2 sec delay is arbitrarily large enough that CL has a reasonable head start and
-  // if it is likely to succeed, it should complete before the MLS provider.
-  // Take note that in locationManager:didUpdateLocations: the handoff to MLS is stopped.
+  // back a location or error.
   mProvider->CreateMLSFallbackProvider();
 }
 
 - (void)locationManager:(CLLocationManager*)aManager didUpdateLocations:(NSArray*)aLocations
 {
   if (aLocations.count < 1) {
     return;
   }
@@ -144,22 +139,24 @@ CoreLocationLocationProvider::MLSUpdate:
   position->GetCoords(getter_AddRefs(coords));
   if (!coords) {
     return NS_ERROR_FAILURE;
   }
   mParentLocationProvider.Update(position);
   Telemetry::Accumulate(Telemetry::GEOLOCATION_OSX_SOURCE_IS_MLS, true);
   return NS_OK;
 }
+
 NS_IMETHODIMP
 CoreLocationLocationProvider::MLSUpdate::NotifyError(uint16_t error)
 {
   mParentLocationProvider.NotifyError(error);
   return NS_OK;
 }
+
 class CoreLocationObjects {
 public:
   nsresult Init(CoreLocationLocationProvider* aProvider) {
     mLocationManager = [[CLLocationManager alloc] init];
     NS_ENSURE_TRUE(mLocationManager, NS_ERROR_NOT_AVAILABLE);
 
     mLocationDelegate = [[LocationDelegate alloc] init:aProvider];
     NS_ENSURE_TRUE(mLocationDelegate, NS_ERROR_NOT_AVAILABLE);
@@ -251,31 +248,29 @@ CoreLocationLocationProvider::SetHighAcc
 
 void
 CoreLocationLocationProvider::Update(nsIDOMGeoPosition* aSomewhere)
 {
   if (aSomewhere && mCallback) {
     mCallback->Update(aSomewhere);
   }
 }
-
 void
 CoreLocationLocationProvider::NotifyError(uint16_t aErrorCode)
 {
   mCallback->NotifyError(aErrorCode);
 }
-
 void
 CoreLocationLocationProvider::CreateMLSFallbackProvider()
 {
   if (mMLSFallbackProvider) {
     return;
   }
 
-  mMLSFallbackProvider = new MLSFallback();
+  mMLSFallbackProvider = new MLSFallback(0);
   mMLSFallbackProvider->Startup(new MLSUpdate(*this));
 }
 
 void
 CoreLocationLocationProvider::CancelMLSFallbackProvider()
 {
   if (!mMLSFallbackProvider) {
     return;
--- a/dom/system/windows/WindowsLocationProvider.cpp
+++ b/dom/system/windows/WindowsLocationProvider.cpp
@@ -97,17 +97,16 @@ LocationEvent::QueryInterface(REFIID iid
     *ppv = static_cast<ILocationEvents*>(this);
   } else {
     return E_NOINTERFACE;
   }
   AddRef();
   return S_OK;
 }
 
-
 STDMETHODIMP
 LocationEvent::OnStatusChanged(REFIID aReportType,
                                LOCATION_REPORT_STATUS aStatus)
 {
   if (aReportType != IID_ILatLongReport) {
     return S_OK;
   }
 
@@ -271,17 +270,17 @@ WindowsLocationProvider::SetHighAccuracy
 nsresult
 WindowsLocationProvider::CreateAndWatchMLSProvider(
   nsIGeolocationUpdate* aCallback)
 {
   if (mMLSProvider) {
     return NS_OK;
   }
 
-  mMLSProvider = new MLSFallback();
+  mMLSProvider = new MLSFallback(0);
   return mMLSProvider->Startup(new MLSUpdate(aCallback));
 }
 
 void
 WindowsLocationProvider::CancelMLSProvider()
 {
   if (!mMLSProvider) {
     return;