Closed Bug 795162 Opened 7 years ago Closed 7 years ago

B2G Wifi: Reuse systemlib's property_get

Categories

(Core :: DOM: Device Interfaces, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: hub, Assigned: hub)

References

Details

Attachments

(1 file)

WifiWorker import ctype and reimplement get_property from libcutils

Instead it should just do 

XPCOMUtils.defineLazyGetter(this, "libcutils", function () {
  Cu.import("resource://gre/modules/systemlibs.js");
  return libcutils;
});

(need bug 795161)
Depends on: 795161
Summary: WifiWorker import ctype and reimplement get_property from libcutils → B2G Wifi: Reuse systemlib's property_get
QA Contact: hub
Assignee: nobody → hub
QA Contact: hub
Comment on attachment 665695 [details] [diff] [review]
Bug 795162 - Use systemlibs.js for WifiWorker.

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

::: dom/wifi/WifiWorker.js
@@ +41,4 @@
>  // expected results).
>  var WifiManager = (function() {
>    function getSdkVersionAndDevice() {
> +    Cu.import("resource://gre/modules/systemlibs.js");

I thought you were going to do a lazy getter? :)

I guess it doesn't matter so much since Cu.import() is nilpotent for subsequent calls, plus we're only calling this function once.

@@ +42,5 @@
>  var WifiManager = (function() {
>    function getSdkVersionAndDevice() {
> +    Cu.import("resource://gre/modules/systemlibs.js");
> +    return { sdkVersion: parseInt(libcutils
> +                                  .property_get("ro.build.version.sdk")),

Always pass the base to parseInt(). IOW, parseInt(x, 10);

Also maybe a more elegant way to get around this overlong line would be to define sdkVersion as a local variable above the `return` statement.

r=me with that.
Attachment #665695 - Flags: review?(philipp) → review+
https://hg.mozilla.org/mozilla-central/rev/22df14239cfb
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.