Bug 886026 - Make ClearWatch work for pending request. r=jdm
authorVinay G Shetty <shettyvinayg@gmail.com>
Fri, 07 Nov 2014 10:42:00 -0500
changeset 215619 980c6c2b694c1d7b601307c6da5543ca2fce1770
parent 215618 2cdf3d16397f7bf4497d31a2a8bc5f6462a7a2cf
child 215620 2cb38f5097315374b5f924e97b44f967881f7404
push id51796
push userryanvm@gmail.com
push dateThu, 13 Nov 2014 20:47:14 +0000
treeherdermozilla-inbound@a05b5362429f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdm
bugs886026
milestone36.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 886026 - Make ClearWatch work for pending request. r=jdm
dom/geolocation/nsGeolocation.cpp
dom/geolocation/nsGeolocation.h
dom/tests/mochitest/geolocation/mochitest.ini
dom/tests/mochitest/geolocation/test_clearWatchBeforeAllowing.html
--- a/dom/geolocation/nsGeolocation.cpp
+++ b/dom/geolocation/nsGeolocation.cpp
@@ -428,25 +428,33 @@ nsGeolocationRequest::GetElement(nsIDOME
   NS_ENSURE_ARG_POINTER(aRequestingElement);
   *aRequestingElement = nullptr;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsGeolocationRequest::Cancel()
 {
+  if (mLocator->ClearPendingRequest(this)) {
+    return NS_OK;
+  }
+
   NotifyError(nsIDOMGeoPositionError::PERMISSION_DENIED);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsGeolocationRequest::Allow(JS::HandleValue aChoices)
 {
   MOZ_ASSERT(aChoices.isUndefined());
 
+  if (mLocator->ClearPendingRequest(this)) {
+    return NS_OK;
+  }
+
   // Kick off the geo device, if it isn't already running
   nsRefPtr<nsGeolocationService> gs = nsGeolocationService::GetGeolocationService();
   nsresult rv = gs->StartDevice(GetPrincipal());
 
   if (NS_FAILED(rv)) {
     // Location provider error
     NotifyError(nsIDOMGeoPositionError::POSITION_UNAVAILABLE);
     return NS_OK;
@@ -1299,16 +1307,38 @@ Geolocation::NotifyError(uint16_t aError
   // notify everyone that is watching
   for (uint32_t i = 0; i < mWatchingCallbacks.Length(); i++) {
     mWatchingCallbacks[i]->NotifyErrorAndShutdown(aErrorCode);
   }
 
   return NS_OK;
 }
 
+bool
+Geolocation::IsAlreadyCleared(nsGeolocationRequest* aRequest)
+{
+  for (uint32_t i = 0, length = mClearedWatchIDs.Length(); i < length; ++i) {
+    if (mClearedWatchIDs[i] == aRequest->WatchId()) {
+      return true;
+    }
+  }
+  return false;
+}
+
+bool
+Geolocation::ClearPendingRequest(nsGeolocationRequest* aRequest)
+{
+  if (aRequest->IsWatch() && this->IsAlreadyCleared(aRequest)) {
+    this->NotifyAllowedRequest(aRequest);
+    this->ClearWatch(aRequest->WatchId());
+    return true;
+  }
+  return false;
+}
+
 void
 Geolocation::GetCurrentPosition(PositionCallback& aCallback,
                                 PositionErrorCallback* aErrorCallback,
                                 const PositionOptions& aOptions,
                                 ErrorResult& aRv)
 {
   GeoPositionCallback successCallback(&aCallback);
   GeoPositionErrorCallback errorCallback(aErrorCallback);
@@ -1484,20 +1514,25 @@ Geolocation::WatchPositionReady(nsGeoloc
 
 NS_IMETHODIMP
 Geolocation::ClearWatch(int32_t aWatchId)
 {
   if (aWatchId < 0) {
     return NS_OK;
   }
 
+  if (!mClearedWatchIDs.Contains(aWatchId)) {
+    mClearedWatchIDs.AppendElement(aWatchId);
+  }
+
   for (uint32_t i = 0, length = mWatchingCallbacks.Length(); i < length; ++i) {
     if (mWatchingCallbacks[i]->WatchId() == aWatchId) {
       mWatchingCallbacks[i]->Shutdown();
       RemoveRequest(mWatchingCallbacks[i]);
+      mClearedWatchIDs.RemoveElement(aWatchId);
       break;
     }
   }
 
   // make sure we also search through the pending requests lists for
   // watches to clear...
   for (uint32_t i = 0, length = mPendingRequests.Length(); i < length; ++i) {
     if (mPendingRequests[i]->IsWatch() &&
--- a/dom/geolocation/nsGeolocation.h
+++ b/dom/geolocation/nsGeolocation.h
@@ -149,16 +149,20 @@ public:
   bool HasActiveCallbacks();
 
   // Register an allowed request
   void NotifyAllowedRequest(nsGeolocationRequest* aRequest);
 
   // Remove request from all callbacks arrays
   void RemoveRequest(nsGeolocationRequest* request);
 
+  // Check if there is already ClearWatch called for current
+  // request & clear if yes
+  bool ClearPendingRequest(nsGeolocationRequest* aRequest);
+
   // Shutting down.
   void Shutdown();
 
   // Getter for the principal that this Geolocation was loaded from
   nsIPrincipal* GetPrincipal() { return mPrincipal; }
 
   // Getter for the window that this Geolocation is owned by
   nsIWeakReference* GetOwner() { return mOwner; }
@@ -180,16 +184,19 @@ private:
   nsresult WatchPosition(GeoPositionCallback& aCallback, GeoPositionErrorCallback& aErrorCallback, PositionOptions* aOptions, int32_t* aRv);
 
   bool RegisterRequestWithPrompt(nsGeolocationRequest* request);
 
   // Methods for the service when it's ready to process requests:
   nsresult GetCurrentPositionReady(nsGeolocationRequest* aRequest);
   nsresult WatchPositionReady(nsGeolocationRequest* aRequest);
 
+  // Check if clearWatch is already called
+  bool IsAlreadyCleared(nsGeolocationRequest* aRequest);
+
   // Two callback arrays.  The first |mPendingCallbacks| holds objects for only
   // one callback and then they are released/removed from the array.  The second
   // |mWatchingCallbacks| holds objects until the object is explictly removed or
   // there is a page change. All requests held by either array are active, that
   // is, they have been allowed and expect to be fulfilled.
 
   nsTArray<nsRefPtr<nsGeolocationRequest> > mPendingCallbacks;
   nsTArray<nsRefPtr<nsGeolocationRequest> > mWatchingCallbacks;
@@ -203,16 +210,19 @@ private:
   // owning back pointer.
   nsRefPtr<nsGeolocationService> mService;
 
   // Watch ID
   uint32_t mLastWatchId;
 
   // Pending requests are used when the service is not ready
   nsTArray<nsRefPtr<nsGeolocationRequest> > mPendingRequests;
+
+  // Array containing already cleared watch IDs
+  nsTArray<int32_t> mClearedWatchIDs;
 };
 
 class PositionError MOZ_FINAL : public nsIDOMGeoPositionError,
                                 public nsWrapperCache
 {
 public:
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(PositionError)
--- a/dom/tests/mochitest/geolocation/mochitest.ini
+++ b/dom/tests/mochitest/geolocation/mochitest.ini
@@ -12,16 +12,18 @@ skip-if = buildapp == 'b2g' || toolkit =
 [test_cachedPosition.html]
 skip-if = buildapp == 'b2g' || toolkit == 'android' || e10s #TIMED_OUT
 [test_cancelCurrent.html]
 skip-if = buildapp == 'b2g'
 [test_cancelWatch.html]
 skip-if = buildapp == 'b2g'
 [test_clearWatch.html]
 skip-if = buildapp == 'b2g' || toolkit == 'android' #TIMED_OUT
+[test_clearWatchBeforeAllowing.html]
+skip-if = buildapp == 'b2g' || toolkit == 'android' #TIMED_OUT
 [test_clearWatch_invalid.html]
 skip-if = buildapp == 'b2g'
 [test_errorcheck.html]
 skip-if = toolkit=='gonk' || toolkit == 'android' || e10s #TIMED_OUT # b2g-debug(debug-only timeout)
 [test_geolocation_is_undefined_when_pref_is_off.html]
 [test_handlerSpinsEventLoop.html]
 skip-if = buildapp == 'mulet' || buildapp == 'b2g' || toolkit == 'android' || e10s #Don't run modal tests on Android # b2g(showmodaldialog) b2g-debug(showmodaldialog) b2g-desktop(showmodaldialog)
 [test_manyCurrentConcurrent.html]
@@ -42,9 +44,9 @@ skip-if = buildapp == 'b2g' || toolkit =
 skip-if = buildapp == 'b2g'
 [test_shutdown.html]
 skip-if = buildapp == 'b2g' || toolkit == 'android' #TIMED_OUT
 [test_timerRestartWatch.html]
 skip-if = buildapp == 'b2g' || toolkit == 'android' || e10s #TIMED_OUT
 [test_windowClose.html]
 skip-if = buildapp == 'b2g' || toolkit == 'android' #TIMED_OUT
 [test_worseAccuracyDoesNotBlockCallback.html]
-skip-if = buildapp == 'b2g' || toolkit == 'android' #TIMED_OUT
+skip-if = buildapp == 'b2g' || toolkit == 'android' #TIMED_OUT
\ No newline at end of file
new file mode 100644
--- /dev/null
+++ b/dom/tests/mochitest/geolocation/test_clearWatchBeforeAllowing.html
@@ -0,0 +1,59 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=886026
+-->
+<head>
+  <title>Test for getCurrentPosition </title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="geolocation_common.js"></script>
+
+<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<a target="_blank"
+href="https://bugzilla.mozilla.org/show_bug.cgi?id=886026">Mozilla Bug 886026</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+SimpleTest.waitForExplicitFinish();
+resume_geolocationProvider(function() {
+  force_prompt(true, run_test);
+});
+
+function run_test() {
+  var successCallbackCalled = false,
+  errorCallbackCalled = false;
+
+  var watchId = navigator.geolocation.watchPosition(
+    function(pos) {
+      successCallbackCalled = true;
+    }, function(err) {
+         errorCallbackCalled = true;
+       }
+  );
+
+  navigator.geolocation.getCurrentPosition(
+    function(pos) {
+      SimpleTest.executeSoon(function() {
+        ok(successCallbackCalled == false,
+        "getCurrentPosition : Success callback should not have been called");
+
+        ok(errorCallbackCalled == false,
+        "getCurrentPosition : Error callback should not have been called");
+
+        SimpleTest.finish();
+      });
+    }
+  );
+
+  navigator.geolocation.clearWatch(watchId);
+}
+</script>
+</pre>
+</body>
+</html>
\ No newline at end of file