Bug 1283563 - Don't reset timeout for oneshot geolocation request. r=jdm
authorKan-Ru Chen <kanru@kanru.info>
Fri, 23 Sep 2016 16:49:23 +0800
changeset 315816 f54d698d7de7c83e65d365eed3d10dc775be9884
parent 315815 b6f0cf0441ddd6b527ca8fae504c4af055ea7040
child 315817 955340c5cf9eff6f6aa79c88f656fa27428fb12f
push id20634
push usercbook@mozilla.com
push dateFri, 30 Sep 2016 10:10:13 +0000
treeherderfx-team@afe79b010d13 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdm
bugs1283563
milestone52.0a1
Bug 1283563 - Don't reset timeout for oneshot geolocation request. r=jdm MozReview-Commit-ID: 4EkPD54Xu7f
dom/geolocation/nsGeolocation.cpp
dom/tests/mochitest/geolocation/chrome.ini
dom/tests/mochitest/geolocation/mochitest.ini
dom/tests/mochitest/geolocation/test_handlerSpinsEventLoop.html
--- a/dom/geolocation/nsGeolocation.cpp
+++ b/dom/geolocation/nsGeolocation.cpp
@@ -511,16 +511,18 @@ nsGeolocationRequest::GetRequester(nsICo
   requester.forget(aRequester);
 
   return NS_OK;
 }
 
 void
 nsGeolocationRequest::SetTimeoutTimer()
 {
+  MOZ_ASSERT(!mShutdown, "set timeout after shutdown");
+
   StopTimeoutTimer();
 
   if (mOptions && mOptions->mTimeout != 0 && mOptions->mTimeout != 0x7fffffff) {
     mTimeoutTimer = do_CreateInstance("@mozilla.org/timer;1");
     RefPtr<TimerCallbackHolder> holder = new TimerCallbackHolder(this);
     mTimeoutTimer->InitWithCallback(holder, mOptions->mTimeout, nsITimer::TYPE_ONE_SHOT);
   }
 }
@@ -580,17 +582,20 @@ nsGeolocationRequest::SendLocation(nsIDO
 
     MOZ_ASSERT(callback);
     callback->Call(*wrapped);
   } else {
     nsIDOMGeoPositionCallback* callback = mCallback.GetXPCOMCallback();
     MOZ_ASSERT(callback);
     callback->HandleEvent(aPosition);
   }
-  SetTimeoutTimer();
+
+  if (mIsWatchPositionRequest && !mShutdown) {
+    SetTimeoutTimer();
+  }
   MOZ_ASSERT(mShutdown || mIsWatchPositionRequest,
              "non-shutdown getCurrentPosition request after callback!");
 }
 
 nsIPrincipal*
 nsGeolocationRequest::GetPrincipal()
 {
   if (!mLocator) {
new file mode 100644
--- /dev/null
+++ b/dom/tests/mochitest/geolocation/chrome.ini
@@ -0,0 +1,6 @@
+[DEFAULT]
+support-files =
+  geolocation_common.js
+  network_geolocation.sjs
+
+[test_handlerSpinsEventLoop.html]
--- a/dom/tests/mochitest/geolocation/mochitest.ini
+++ b/dom/tests/mochitest/geolocation/mochitest.ini
@@ -10,17 +10,16 @@ support-files =
 [test_cachedPosition.html]
 [test_cancelCurrent.html]
 [test_cancelWatch.html]
 [test_clearWatch.html]
 [test_clearWatchBeforeAllowing.html]
 [test_clearWatch_invalid.html]
 [test_errorcheck.html]
 [test_geolocation_is_undefined_when_pref_is_off.html]
-[test_handlerSpinsEventLoop.html]
 [test_manyCurrentConcurrent.html]
 [test_manyCurrentSerial.html]
 [test_manyWatchConcurrent.html]
 [test_manyWatchSerial.html]
 [test_manyWindows.html]
 [test_optional_api_params.html]
 [test_shutdown.html]
 [test_timeoutCurrent.html]
--- a/dom/tests/mochitest/geolocation/test_handlerSpinsEventLoop.html
+++ b/dom/tests/mochitest/geolocation/test_handlerSpinsEventLoop.html
@@ -1,67 +1,65 @@
 <!DOCTYPE HTML>
 <html>
 <!--
 https://bugzilla.mozilla.org/show_bug.cgi?id=911595
 -->
 <head>
   <title>Test for spinning the event loop inside position handlers</title>
-  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="chrome://mochikit/content/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" />
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css" />
 </head>
 <body>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=911595 ">Mozilla Bug 911595</a>
 <p id="display"></p>
 <div id="content" style="display: none">
   
 </div>
 <pre id="test">
 <script class="testbody" type="text/javascript">
 
 /*
  * In bug 911595 , spinning the event loop from inside position
  * handlers could cause both success and error callbacks to be
  * fired for the same request if that request has a small timeout.
  */
 
+var { classes: Cc, interfaces: Ci, utils: Cu }  = Components;
+
 SimpleTest.waitForExplicitFinish();
-SimpleTest.requestFlakyTimeout("untriaged");
 
 resume_geolocationProvider(function() {
   force_prompt(true, test1);
 });
 
-function spinEventLoopAndSetTimeout() {
-  if (successCallbackCalled || errorCallbackCalled) {
-    // this should only be called once from either callback
-    return;
-  }
-
-  SpecialPowers.spinEventLoop(window);
-
-  setTimeout(function() {
-    ok(successCallbackCalled != errorCallbackCalled, "Ensure only one callback is called");
-    SimpleTest.finish();
-  }, 5);
-}
-
 var successCallbackCalled = false;
 function successCallback(position) {
-  spinEventLoopAndSetTimeout();
   successCallbackCalled = true;
+  check_geolocation(position);
+  while (!timeoutPassed) {
+    SpecialPowers.spinEventLoop(window);
+  }
+  ok(successCallbackCalled != errorCallbackCalled, "Ensure only one callback is called");
+  SimpleTest.finish();
 }
 
 var errorCallbackCalled = false;
 function errorCallback(error) {
-  spinEventLoopAndSetTimeout();
   errorCallbackCalled = true;
 }
 
+var timeoutPassed = false;
+var timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
 function test1() {
-  navigator.geolocation.getCurrentPosition(successCallback, errorCallback, {timeout: 1});
+  SpecialPowers.pushPrefEnv({"set": [["geo.wifi.timeToWaitBeforeSending", 10]]}, function() {
+    navigator.geolocation.getCurrentPosition(successCallback, errorCallback, {timeout: 500});
+    timer.initWithCallback(timer => {
+      timeoutPassed = true;
+    }, 600, Ci.nsITimer.TYPE_ONE_SHOT);
+  });
 }
 </script>
 </pre>
 </body>
 </html>