diff --git a/dom/geolocation/nsGeolocation.cpp b/dom/geolocation/nsGeolocation.cpp index 3919a94e366..dace3d3b3fd 100644 --- a/dom/geolocation/nsGeolocation.cpp +++ b/dom/geolocation/nsGeolocation.cpp @@ -31,6 +31,7 @@ #include "mozilla/Preferences.h" #include "mozilla/ClearOnShutdown.h" #include "mozilla/dom/Event.h" +#include "mozilla/WeakPtr.h" #include "mozilla/dom/PermissionMessageUtils.h" #include "mozilla/dom/SettingChangeNotificationBinding.h" #include "mozilla/dom/WakeLock.h" @@ -75,13 +76,12 @@ using namespace mozilla::hal; class nsGeolocationRequest final : public nsIContentPermissionRequest - , public nsITimerCallback , public nsIGeolocationUpdate + , public SupportsWeakPtr { public: NS_DECL_CYCLE_COLLECTING_ISUPPORTS NS_DECL_NSICONTENTPERMISSIONREQUEST - NS_DECL_NSITIMERCALLBACK NS_DECL_NSIGEOLOCATIONUPDATE NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(nsGeolocationRequest, nsIContentPermissionRequest) @@ -93,6 +93,9 @@ class nsGeolocationRequest final uint8_t aProtocolType, bool aWatchPositionRequest = false, int32_t aWatchId = 0); + + MOZ_DECLARE_WEAKREFERENCE_TYPENAME(nsGeolocationRequest) + void Shutdown(); void SendLocation(nsIDOMGeoPosition* aLocation); @@ -107,6 +110,23 @@ class nsGeolocationRequest final private: virtual ~nsGeolocationRequest(); + class TimerCallbackHolder final : public nsITimerCallback + { + public: + NS_DECL_ISUPPORTS + NS_DECL_NSITIMERCALLBACK + + explicit TimerCallbackHolder(nsGeolocationRequest* aRequest) + : mRequest(aRequest) + {} + + private: + ~TimerCallbackHolder() {} + WeakPtr mRequest; + }; + + void Notify(); + already_AddRefed AdjustedLocation(nsIDOMGeoPosition*); bool mIsWatchPositionRequest; @@ -379,12 +399,12 @@ nsGeolocationRequest::nsGeolocationRequest(Geolocation* aLocator, nsGeolocationRequest::~nsGeolocationRequest() { + StopTimeoutTimer(); } NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsGeolocationRequest) NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIContentPermissionRequest) NS_INTERFACE_MAP_ENTRY(nsIContentPermissionRequest) - NS_INTERFACE_MAP_ENTRY(nsITimerCallback) NS_INTERFACE_MAP_ENTRY(nsIGeolocationUpdate) NS_INTERFACE_MAP_END @@ -393,12 +413,11 @@ NS_IMPL_CYCLE_COLLECTING_RELEASE(nsGeolocationRequest) NS_IMPL_CYCLE_COLLECTION(nsGeolocationRequest, mCallback, mErrorCallback, mLocator) -NS_IMETHODIMP -nsGeolocationRequest::Notify(nsITimer* aTimer) +void +nsGeolocationRequest::Notify() { StopTimeoutTimer(); NotifyErrorAndShutdown(nsIDOMGeoPositionError::TIMEOUT); - return NS_OK; } void @@ -561,7 +580,8 @@ nsGeolocationRequest::SetTimeoutTimer() } mTimeoutTimer = do_CreateInstance("@mozilla.org/timer;1"); - mTimeoutTimer->InitWithCallback(this, timeout, nsITimer::TYPE_ONE_SHOT); + RefPtr holder = new TimerCallbackHolder(this); + mTimeoutTimer->InitWithCallback(holder, timeout, nsITimer::TYPE_ONE_SHOT); } } @@ -741,10 +761,7 @@ nsGeolocationRequest::Shutdown() MOZ_ASSERT(!mShutdown, "request shutdown twice"); mShutdown = true; - if (mTimeoutTimer) { - mTimeoutTimer->Cancel(); - mTimeoutTimer = nullptr; - } + StopTimeoutTimer(); // If there are no other high accuracy requests, the geolocation service will // notify the provider to switch to the default accuracy. @@ -756,6 +773,23 @@ nsGeolocationRequest::Shutdown() } } + +//////////////////////////////////////////////////// +// nsGeolocationRequest::TimerCallbackHolder +//////////////////////////////////////////////////// + +NS_IMPL_ISUPPORTS(nsGeolocationRequest::TimerCallbackHolder, nsISupports, nsITimerCallback) + +NS_IMETHODIMP +nsGeolocationRequest::TimerCallbackHolder::Notify(nsITimer*) +{ + if (mRequest) { + mRequest->Notify(); + } + return NS_OK; +} + + //////////////////////////////////////////////////// // nsGeolocationService //////////////////////////////////////////////////// diff --git a/dom/tests/browser/browser.ini b/dom/tests/browser/browser.ini index 66d21a780a0..363758ed7bd 100644 --- a/dom/tests/browser/browser.ini +++ b/dom/tests/browser/browser.ini @@ -7,6 +7,7 @@ support-files = test-console-api.html test_bug1004814.html worker_bug1004814.js + geo_leak_test.html [browser_test_toolbars_visibility.js] support-files = @@ -20,6 +21,7 @@ skip-if = buildapp == 'mulet' [browser_autofocus_background.js] skip-if= buildapp == 'mulet' [browser_autofocus_preference.js] +[browser_bug1238427.js] [browser_bug396843.js] [browser_focus_steal_from_chrome.js] [browser_focus_steal_from_chrome_during_mousedown.js] diff --git a/dom/tests/browser/browser_bug1238427.js b/dom/tests/browser/browser_bug1238427.js new file mode 100644 index 00000000000..18e39ad950c --- /dev/null +++ b/dom/tests/browser/browser_bug1238427.js @@ -0,0 +1,31 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +"use strict"; + +const TEST_URI = "http://example.com/" + + "browser/dom/tests/browser/geo_leak_test.html"; + +const BASE_GEO_URL = "http://mochi.test:8888/tests/dom/tests/mochitest/geolocation/network_geolocation.sjs"; + +add_task(function* () { + Services.prefs.setBoolPref("geo.prompt.testing", true); + Services.prefs.setBoolPref("geo.prompt.testing.allow", true); + + // Make the geolocation provider responder very slowly to ensure that + // it does not reply before we close the tab. + Services.prefs.setCharPref("geo.wifi.uri", BASE_GEO_URL + "?delay=100000"); + + // Open the test URI and close it. The test harness will make sure that the + // page is cleaned up after some GCs. If geolocation is not shut down properly, + // it will show up as a non-shutdown leak. + yield BrowserTestUtils.withNewTab({ + gBrowser, + url: TEST_URI + }, function* (browser) { /* ... */ }); + + ok(true, "Need to do something in this test"); +}); + diff --git a/dom/tests/browser/geo_leak_test.html b/dom/tests/browser/geo_leak_test.html new file mode 100644 index 00000000000..fb3fabac404 --- /dev/null +++ b/dom/tests/browser/geo_leak_test.html @@ -0,0 +1,17 @@ + + + +Geolocation incomplete position leak test + + + + +