Closed
Bug 778235
Opened 9 years ago
Closed 8 years ago
Add support for Gonk to NetworkGeolocationProvider.js
Categories
(Core :: DOM: Geolocation, defect)
Tracking
()
People
(Reporter: mwu, Assigned: dougt)
References
Details
Attachments
(1 file, 2 obsolete files)
10.19 KB,
patch
|
jdm
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Comment 1•9 years ago
|
||
WIP - apparently DOMWifiManager can't be used from a js component context so this doesn't work.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → doug.turner
Comment 2•9 years ago
|
||
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.
Assignee | ||
Comment 3•8 years ago
|
||
Attachment #652985 -
Attachment is obsolete: true
Attachment #669583 -
Flags: review?(josh)
Assignee | ||
Comment 4•8 years ago
|
||
http://tbpl.mozilla.org/?tree=Try&rev=c07ef53519f7
Comment 5•8 years ago
|
||
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-
Comment 6•8 years ago
|
||
Also I suspect you've got interfaces missing from this, like nsIWifiScanResultsReady.
Comment 7•8 years ago
|
||
(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
Assignee | ||
Comment 8•8 years ago
|
||
> 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...
Assignee | ||
Comment 9•8 years ago
|
||
Attachment #669583 -
Attachment is obsolete: true
Attachment #669712 -
Flags: review?(josh)
Updated•8 years ago
|
Attachment #669712 -
Flags: review?(josh) → review+
Assignee | ||
Comment 10•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e76aae8e80c9
Comment 11•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e76aae8e80c9
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Updated•8 years ago
|
blocking-basecamp: ? → +
You need to log in
before you can comment on or make changes to this bug.
Description
•