Closed Bug 778235 Opened 8 years ago Closed 8 years ago

Add support for Gonk to NetworkGeolocationProvider.js

Categories

(Core :: DOM: Geolocation, defect)

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla19
blocking-basecamp +
Tracking Status
firefox18 --- fixed
firefox19 --- fixed

People

(Reporter: mwu, Assigned: dougt)

References

Details

Attachments

(1 file, 2 obsolete files)

No description provided.
Attached patch WIP (obsolete) — Splinter Review
WIP - apparently DOMWifiManager can't be used from a js component context so this doesn't work.
Assignee: nobody → doug.turner
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.
Attached patch patch v.1 (obsolete) — Splinter Review
Attachment #652985 - Attachment is obsolete: true
Attachment #669583 - Flags: review?(josh)
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.
Attachment #669583 - Flags: review?(josh) → review-
Also I suspect you've got interfaces missing from this, like nsIWifiScanResultsReady.
(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.
Depends on: 792952
> 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...
Attached patch patch v.2Splinter Review
Attachment #669583 - Attachment is obsolete: true
Attachment #669712 - Flags: review?(josh)
Attachment #669712 - Flags: review?(josh) → review+
https://hg.mozilla.org/mozilla-central/rev/e76aae8e80c9
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
We probably want this on b2g v.1.
blocking-basecamp: --- → ?
blocking-basecamp: ? → +
Depends on: 799391
No longer depends on: 799391
You need to log in before you can comment on or make changes to this bug.