Created attachment 388135 [details] Testcase (safe to open, a confirmation is asked before anything) Steps to reproduce: see attachment Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090711 Minefield/3.6a1pre (.NET CLR 3.5.30729) ID:20090711042715 http://crash-stats.mozilla.com/report/index/91e308c9-88a8-4fb2-89c8-51b4b2090712 With a clean profile: http://crash-stats.mozilla.com/report/index/f3b9a1b3-2b65-4002-9714-e64bc2090712 http://crash-stats.mozilla.com/report/index/aca035c8-0441-41fa-ac62-e45fc2090712
Crashes on Linux too. Also crashes in 3.5 release, not just trunk. bp-82cb127c-5e49-4d13-bbb1-1346b2090712
OS: Windows Vista → All
Hardware: x86 → All
Version: Trunk → 1.9.1 Branch
http://www.w3.org/TR/2009/WD-geolocation-API-20090707/ The getCurrentPosition() takes one, two or three arguments. When called, it must immediately return and then asynchronously acquire a new Position object. If successful, this method must invoke its associated successCallback argument with a Position object as an argument. If the attempt fails, and the method was invoked with a non-null errorCallback argument, this method must invoke the errorCallback with a PositionError object as an argument. The spec doesn't say what to do if the first argument is null.
Created attachment 388203 [details] [diff] [review] patch note that we don't simply bail out early, we need to give a chance to pass to the error callback, and we need to record that we didn't time out.
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #388203 - Flags: review?(doug.turner)
I tried to narrow down the problem (without success): I was thinking about http://hg.mozilla.org/mozilla-central/annotate/47bfcd275ede/dom/src/geolocation/nsGeolocation.cpp#l328 (mCallback being null) I tried to patch here with a if not null but with a debug build I saw the problem seems to be in NetworkGeolocationProvider.js when WifiGeoPositionProvider.timer is fired and WifiGeoPositionProvider.notify is called (then the crash occurs)
I would rather assert if all of the parameters are null/invalid. timeless, do you want to update, or do you want me to roll a new patch?
Created attachment 388278 [details] [diff] [review] patch v.1 asserts if null is passed as the success callback.
NS_ENSURE_ARG_POINTER ? Isn't this what it was created for?
Attachment #388278 - Flags: review? → review?(jst)
note that i claim dougt's approach is invalid. nothing in the spec says i can't do: stupid.getCurrentPosition(null, errorCallback);
Attachment #388278 - Flags: review?(jst) → review-
(marked - because I wanted to incorporate natch's feedback) timeless - passing null is stupid as you suggest. at the end of the day, do you really think supporting such a thing is better than alerting the developer that they are making a programmatic error? Furthermore, the spec doesn't say alot of things you can and can't do. I assert that this can be a implementation detail. We will assert if you pass null.
Created attachment 388304 [details] [diff] [review] patch v.2
asserts in gecko debug only builds don't help because we're talking about web applications. you need to use the error console. I could vaguely imagine writing something which only has an error callback. if you don't do that, you aren't actually reporting the error, you're just wasting time pretending you are.
not in any useful way, no. http://mxr.mozilla.org/mozilla-central/ident?i=ReportUseOfDeprecatedMethod and if you're going to do it, someone needs to tell the working group.... note that web apps generally do not expect exceptions, so what you're probably doing is causing a number of apps to randomly break. don't do this without standardizing it.
status1.9.1: --- → wanted
Comment on attachment 388304 [details] [diff] [review] patch v.2 fwiw, i notified the wg about this and waiting feedback. we can change the behavior if any different decision is reached. http://lists.w3.org/Archives/Public/public-geolocation/2009Jul/0010.html
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
no more crash with testcase and Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090717 Minefield/3.6a1pre (.NET CLR 3.5.30729) ID:20090717045648 only an error in JS console: Error: uncaught exception: [Exception... "Component returned failure code: 0x80004003 (NS_ERROR_INVALID_POINTER) [nsIDOMGeoGeolocation.getCurrentPosition]" nsresult: "0x80004003 (NS_ERROR_INVALID_POINTER)" location: "JS frame :: https://bug503750.bugzilla.mozilla.org/attachment.cgi?id=388135 :: anonymous :: line 19" data: no] Thanks. => VERIFIED
Status: RESOLVED → VERIFIED
Doug, if this patch applies to 1.9.1, please request approval184.108.40.206 on it.
Attachment #388304 - Flags: approval220.127.116.11? → approval18.104.22.168+
Comment on attachment 388304 [details] [diff] [review] patch v.2 Approved for 22.214.171.124. a=ss for release-drivers Please land on mozilla-1.9.1 and use the ".2-fixed" option of the "status1.9.1" flag.
status1.9.1: wanted → .2-fixed
Verified with testcase in comment #0 using: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:126.96.36.199) Gecko/20090729 Firefox/3.5.2 (.NET CLR 3.5.30729) 3.5 crashes. 3.5.2 does not.
Crash Signature: [@ nsGeolocationRequest::SendLocation(nsIDOMGeoPosition*) ]
You need to log in before you can comment on or make changes to this bug.