482613 - support for WiFi Scanning on Windows Mobile

Status: RESOLVED FIXED

(Core :: DOM: Geolocation, enhancement)

Description

[Nino D'Aversa (:ndaversa)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b2) Gecko/20081201 Firefox/3.1b2 (.NET CLR 3.5.30729)
Build Identifier: 

@ http://www.ndaversa.com/2009/03/10/07-release-update-windows-mobile-wifiscanner/


Reproducible: Always

Comment 1

[Nino D'Aversa (:ndaversa)]

Attachment: [WIP] patch v1

This is my WIP patch. I'm currently in the process of testing this...

I'll report back when I know more.

Comment 2

[Nino D'Aversa (:ndaversa)]

Attachment: [WIP] patch v2

I'm currently having problems with my nsStringArray being null (more specifcally the impl struct inside nsVoidArray). Does anyone see why? I'm going to seek out some help on IRC tomorrow.

Comment 3

[Nino D'Aversa (:ndaversa)]

Attachment: [WIP] patch v2

I'm currently having problems with my nsStringArray being null (more
specifically the impl struct inside nsVoidArray). Does anyone see why? I'm going
to seek out some help on IRC tomorrow.

Comment 4

[Nino D'Aversa (:ndaversa)]

Attachment: patch v3

Here is a working version of my patch.

You can see some screen shot at my blog, I just popped an alert with the SSID info...

http://www.ndaversa.com/wp-content/uploads/2009/03/08_wifi_scanning_test.png

As always I welcome a thrashing from Doug on all the things that may be wrong with this.

Comment 5

[Doug Turner (:dougt)]

Comment on attachment 369384 [details] [diff] [review]
patch v3

use the auto monitor code similar to what i did in the other ports.

Alot of this is similar to the windows code.  do you think it makes sense to combine the implementations?

Comment 6

[Nino D'Aversa (:ndaversa)]

I'll plug in the auto monitor code. I think I started from an earlier version before you implemented that, that's how I missed it.

With regards to combining implementations I think it makes sense. Once I had a final version I realized it didn't change terribly from what you had done for the win32 version. I think I can merge my changes into your win32 file and IFDEF WINCE the parts that are different. Do you want me to do this?

Comment 7

[Nino D'Aversa (:ndaversa)]

Attachment: patch v4

Doug,

I have combined the WINCE implementation with the Win32 implementation by adding some #ifdef goodness to the win32 impl file.

It works just as well as before and I think this will allow us to maintain the overlapping code more efficiently due to the elimination of code duplication.

Let me know what you think and Happy Easter!

Comment 8

[Doug Turner (:dougt)]

Comment on attachment 372315 [details] [diff] [review]
patch v4

can you work to reduce the difference a bit further by

1) Make the GetNetworkInterfaces the same name between wince and w32.

2) creating a function for the bit of code in the #else block after if (!IsRunningOnVista()) {

3) can we use the same variable names in the PerformQuery() call?

Comment 9

[Nino D'Aversa (:ndaversa)]

Attachment: patch v5

As per our IRC discussion:

1. Fixed up reinterpret casts (they have now gone the way of the dinosaur).
2. GETADAPTERS hash define couldn't be removed, the definition in iphlapi.h does not suffice
3. scary line (&query->Data[0]) given comment, fear no more
4. spaces added to comments where appropriate
5. identation fixed in GetNetworkInterfaces
6. GetNetworkInterfacesFromRegistry renamed to GetNetworkInterfaces
7. PerformQuery cannot have same variables, for reasons discussed
8. moving the block of code after if (!isRunningOnVista()) will break early returns, leaving as is (as per discussion).
9. Removed freelibrary.

Looking forward to seeing this land, let me know if you spot any other things that need fixing before that can happen.

and thanks again Doug for helping me push this through today.

Comment 10

[Doug Turner (:dougt)]

Comment on attachment 372708 [details] [diff] [review]
patch v5

+  if (pGetAdaptersInfo(NULL, &buffer_size) != ERROR_BUFFER_OVERFLOW)      
+    return;//NS_ERROR_FAILURE;


Drop this comment


+  if (pGetAdaptersInfo(adapter_info, &buffer_size) != ERROR_SUCCESS)
+    return;

early return doesn't free adapter_info


+    adapterName.AppendWithConversion(adapter_info->AdapterName);      

Could you add a comment that states:  "AdapterName is ASCII".  Everytime I look at this, i find myself looking at msdn to confirm this fact.


The block after:
   if (!IsRunningOnVista()) {


I think you can pull that out into a seperate function and just make |pGetAdaptersInfo| a static in this function (and don't unload the library!) .  I think i said not to worry about it, but after thinking a bit about, i think it is the right thing.

Comment 11

[Nino D'Aversa (:ndaversa)]

Attachment: patch v6

Doug, Once again thanks for your attention on this bug. I know you got the FF3.5 freeze so I won't expect some love on this for a bit. Anyways...

I addressed your concerns.

However I'm not certain I got the part about "the block after the IsRunningVista" the way you may have intended. Check it out and let me know.

I figured I would just put together what I thought was best and then go from there.

Thanks

Comment 12

[Doug Turner (:dougt)]

http://hg.mozilla.org/mozilla-central/rev/7b213ee86116

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 372991 [details] [diff] [review]
patch v6

a191=beltzner

Comment 14

[Doug Turner (:dougt)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9a45a17fdc7b