Geolocation Crashes Fennec

RESOLVED FIXED

Status

()

Core
Geolocation
P2
critical
RESOLVED FIXED
9 years ago
3 years ago

People

(Reporter: richard, Assigned: dougt)

Tracking

Trunk
x86
Mac OS X
Points:
---
Bug Flags:
blocking1.9.2 +

Firefox Tracking Flags

(status1.9.2 beta1-fixed)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

9 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.3) Gecko/20090824 Firefox/3.5.3 FirePHP/0.3
Build Identifier: 1.0b4

Attempting to get geolocation crashes Fennec.   Same site works fine w/Firefox Desktop.

Reproducible: Always

Steps to Reproduce:
1.Browse to http://avwx.net
2.Log-in.  Username: Mozilla  Password: Fennec
3.Choose "Find Nearby Airports" from main menu.
4.Click the "Share" button in response to avwx.net wants your location.

Actual Results:  
Fennec Crashes

Expected Results:  
Display the main menu with 3 nearby airports at the top of the menu.

MacOS Crash Report attached.
(Reporter)

Comment 1

9 years ago
Created attachment 405116 [details]
Crash Report from MacOS

Comment 2

9 years ago
It doesn't crash on a Linux desktop build, but it didn't find any airports.

I guess that the fact it didn't find any airport is a different issue.
(Assignee)

Updated

9 years ago
Status: UNCONFIRMED → NEW
Component: General → Geolocation
Ever confirmed: true
Product: Fennec → Core
QA Contact: general → geolocation
(Assignee)

Comment 4

9 years ago
Created attachment 405386 [details] [diff] [review]
patch v.1

patch is a bit mess, but basically this does two things:

it uses the bundle magic as described in http://dougt.org/wordpress/2009/09/usingcorewlan/#comments.

it also wraps the calls in a try block.
Assignee: nobody → doug.turner
Attachment #405386 - Flags: review?(joshmoz)

Updated

9 years ago
Attachment #405386 - Flags: review?(joshmoz) → review?(mstange)
Comment on attachment 405386 [details] [diff] [review]
patch v.1

> nsresult
> GetAccessPointsFromWLAN(nsCOMArray<nsWifiAccessPoint> &accessPoints)
> {
>+  NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
>+
>+  accessPoints.Clear();
>+
>   if (!UsingSnowLeopard())
>     return NS_ERROR_NOT_AVAILABLE;
> 
>-  NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];

Why did you move the early return under the autorelease pool? We don't need the pool when !UsingSnowLeopard(), do we?

The rest of the patch looks good, except for style issues.

>+    if (!CWI_class)
>+      return NS_ERROR_NOT_AVAILABLE;
>+    
>+

We only need one blank line here. Also, the first one has trailing whitespace.

>+    NSDictionary *params = nil;
>+    NSError *err = nil;
>+    id scanResult = [[CWI_class interface] scanForNetworksWithParameters: params error: err];
>+    

Trailing whitespace.

>+    if (!scanResult)
>+      return NS_ERROR_NOT_AVAILABLE;
>+    

Again.

>+    NSArray* scan = [NSMutableArray arrayWithArray:scanResult];
>+    NSEnumerator *enumerator = [scan objectEnumerator];
>   

Please fix this line too.

>-  accessPoints.Clear();
>-  
>-  id anObject;
>-  NSError *err = nil;
>-  NSDictionary *params = nil;
>+    while (id anObject = [enumerator nextObject]) {
>+      nsWifiAccessPoint* ap = new nsWifiAccessPoint();
>+      if (!ap)
>+        continue;

I know this already was in the old code, but what sense does it make to continue when we're out of memory? I think a "return NS_ERROR_OUT_OF_MEMORY;" would be more appropriate.
(Assignee)

Comment 6

9 years ago
ugh.  emacs doesn't love objective-c files.  sorry.

yeah, return NS_ERROR_OUT_OF_MEMORY does make some sense....
(Assignee)

Comment 7

9 years ago
Created attachment 405396 [details] [diff] [review]
patch v.2
Attachment #405386 - Attachment is obsolete: true
Attachment #405396 - Flags: review?(mstange)
Attachment #405386 - Flags: review?(mstange)
Attachment #405396 - Flags: review?(mstange) → review+
(Assignee)

Comment 8

9 years ago
http://hg.mozilla.org/mozilla-central/rev/a85d4ac1a17c
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Assignee)

Updated

9 years ago
Attachment #405396 - Flags: approval1.9.2?
(Assignee)

Updated

9 years ago
tracking-fennec: --- → ?
Blocking, doesn't require explicit approval, but heck, a192=beltzner as well!
Flags: blocking1.9.2+
Priority: -- → P2
Attachment #405396 - Flags: approval1.9.2? → approval1.9.2+
(Assignee)

Comment 10

9 years ago
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/e09404afd24b
tracking-fennec: ? → ---
status1.9.2: --- → beta1-fixed
(Assignee)

Updated

9 years ago
Attachment #405396 - Flags: approval1.9.1.5?
Comment on attachment 405396 [details] [diff] [review]
patch v.2

Doesn't seem like the kind of fix we have to take on the stable branch. 1.9.1 approval denied.
Attachment #405396 - Flags: approval1.9.1.6? → approval1.9.1.6-
Depends on: 1142860
You need to log in before you can comment on or make changes to this bug.