778235 - Add support for Gonk to NetworkGeolocationProvider.js

Status: RESOLVED FIXED

Comment 1

[Michael Wu [:mwu]]

Attachment: WIP

WIP - apparently DOMWifiManager can't be used from a js component context so this doesn't work.

Comment 2

[Blake Kaplan (:mrbkap) (inactive, use needinfo)]

Here's a write-up of what I think needs to happen to be able to access the necessary information from C++:

In order to get at wifi information from chrome, things should be
relatively simple.

The relevant source code is in dom/wifi. You probably want to look at
WifiWorker.js (the chrome implementation) and nsIWifi.idl. The other
main file is DOMWifiManager.js (the implementation of the DOM API).

We already have an API, nsIWifi, which is accessible via
SystemWorkerManager::GetInterface(nsIWifi). In turn, the
SystemWorkerManager is available via its contract ID
(@mozilla.org/telephony/system-worker-manager;1). Because we're
guaranteed to only ever have one chrome process, we can simply add the
APIs needed to nsIWifi. I would suggest using callbacks for things that
are asynchronous in the DOM API (for example, getNetworks) instead of
trying do anything fancy. My understanding is that geolocation only
needs the available networks (including information like signal
strength). In order to do this, I would add:

interface nsIWifiScanResult : nsISupports {
  readonly attribute DOMString SSID;
  readonly attribute DOMString BSSID;

  const int32_t WPA2_PSK = ...;
  const int32_t WEP = ...;
  readonly attribute uint32_t capabilities;

  readonly attribute uint32_t signalStrength;
  readonly attribute uint32_t relSignalStrength;
  readonly attribute boolean connected;
};

interface nsIWifiScanResultsReady : nsISupports {
  void onready(in int32_t count, [array, size_is(count)] in
nsIWifiScanResult results);
  void onfailure();
};

and then:

interface nsIWifi : nsISupports {
  // ...
  void getScanResults(in nsIWifiScanResultsReady callback);
};

The implementation can then use a similar implementation as the
getNetworks
(http://hg.mozilla.org/mozilla-central/file/fd4d9c386f97/dom/wifi/WifiWorker.js#l2014)
method, but with the proper conversion to call the
nsIWifiScanResultsReady functions instead of the DOMRequest stuff when
the results come back.

Comment 3

[Doug Turner (:dougt)]

Attachment: patch v.1

Comment 4

[Doug Turner (:dougt)]

http://tbpl.mozilla.org/?tree=Try&rev=c07ef53519f7

Comment 5

[Josh Matthews [:jdm]]

Comment on attachment 669583 [details] [diff] [review]
patch v.1

Review of attachment 669583 [details] [diff] [review]:
-----------------------------------------------------------------

I'd like some answers before this gets r+.

::: netwerk/wifi/nsWifiAccessPoint.h
@@ +32,5 @@
>    }
>  
> +  void setMacRaw(const char* aString)
> +  {
> +    memcpy(mMac, aString, 18);

mozilla::ArrayLength?

@@ +56,5 @@
>    }
>  
> +  void setSSIDRaw(const char* aSSID, unsigned long len) {
> +    memcpy(mSsid, aSSID, 33);
> +    mSsidLen = PR_MIN(len, 33);

mozilla::ArrayLength for these as well?

::: netwerk/wifi/nsWifiMonitor.cpp
@@ +25,5 @@
>  #if defined(PR_LOGGING)
>  PRLogModuleInfo *gWifiMonitorLog;
>  #endif
>  
> +#ifdef MOZ_WIDGET_GONK

There's enough code here that I would really like to see this implementation in a separate file.

@@ +83,5 @@
> +      break;
> +    }
> +  }
> +
> +  if(mListeners.Length() == 0) {

nit: space before (

@@ +158,5 @@
> +  nsTArray<nsIWifiAccessPoint*> ac;
> +  uint32_t resultCount = accessPoints.Count();
> +  for (uint32_t i = 0; i < resultCount; i++) {
> +    ac.AppendElement(accessPoints[i]);
> +  }

Why are we a) duplicating the contents of accessPoints, b) not addrefing them?

::: netwerk/wifi/nsWifiMonitor.h
@@ +70,5 @@
>  
>  };
> +#else
> +
> +#include "nsIWifi.h"

Similarly, other file please.

Comment 6

[Josh Matthews [:jdm]]

Also I suspect you've got interfaces missing from this, like nsIWifiScanResultsReady.

Comment 7

[Blake Kaplan (:mrbkap) (inactive, use needinfo)]

(In reply to Josh Matthews [:jdm] from comment #6)
> Also I suspect you've got interfaces missing from this, like
> nsIWifiScanResultsReady.

Those were added in bug 792952.

Comment 8

[Doug Turner (:dougt)]

> There's enough code here that I would really like to see this implementation in a separate file.

I was on the fence about this.  I wasn't convinced.

I'll split off the gonk implementation into its own source file, but the header file is pretty clean.

> Why are we a) duplicating the contents of accessPoints, b) not addrefing them?

'ac' is a local variable and owns a weak reference back to the actual access points held in 'accessPoints'.  Were doing this because the 'OnChange' listener requires a nsIWifiAccessPoint** and count.

patching coming up...

Comment 9

[Doug Turner (:dougt)]

Attachment: patch v.2

Comment 10

[Doug Turner (:dougt)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e76aae8e80c9

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e76aae8e80c9

Comment 12

[Doug Turner (:dougt)]

We probably want this on b2g v.1.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/34a039c14957