Closed Bug 809006 Opened 12 years ago Closed 12 years ago

B2G Network Manager: Unable to set default route and DNS when required system properties not available

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-basecamp:+, firefox18 fixed, firefox19 fixed)

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

People

(Reporter: swu, Assigned: swu)

Details

Attachments

(1 file, 1 obsolete file)

The currently network manager relies on the following system properties to set default route and DNS: net.<ifname>.gw net.<ifname>.dns1 net.<ifname>.dns2 On some partner devices, they don't provide these required system properties, and caused network failure for data call or wifi, because no default route and DNS were configured. As we already have these information in nsINetworkInterface, it's more reasonable to use gw and dns information here instead of system property.
Attached patch Patch V1 (obsolete) — Splinter Review
Assignee: nobody → swu
Status: NEW → ASSIGNED
Attachment #678711 - Flags: review?(vyang)
Comment on attachment 678711 [details] [diff] [review] Patch V1 Review of attachment 678711 [details] [diff] [review]: ----------------------------------------------------------------- Thank you :) I guess it's not easy to add a test case for this :( ::: dom/system/gonk/net_worker.js @@ +163,3 @@ > if (!options.gateway || !options.dns1_str) { > options = getIFProperties(options.ifname); > } 'getIFProperties()' could override all of the three properties. I'd rather: let ifprops = getIFProperties(options.ifname); let gateway_str = options.gateway_str || ifprops.gateway_str; let dns1_str = options.dns1_str || ifprops.dns1_str; let dns2_str = options.dns2_str || ifprops.dns2_str; let gateway = netHelpers.stringToIP(gateway_str); And remove `gateway` assignment in getIFProperties() as well. Just prevent any other possible null value problems in the future.
Attachment #678711 - Flags: review?(vyang) → review+
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #2) > Comment on attachment 678711 [details] [diff] [review] > Patch V1 > > Review of attachment 678711 [details] [diff] [review]: > ----------------------------------------------------------------- > > Thank you :) I guess it's not easy to add a test case for this :( I did manual test, that's what we can do now. > > ::: dom/system/gonk/net_worker.js > @@ +163,3 @@ > > if (!options.gateway || !options.dns1_str) { > > options = getIFProperties(options.ifname); > > } > > 'getIFProperties()' could override all of the three properties. I'd rather: > > let ifprops = getIFProperties(options.ifname); > let gateway_str = options.gateway_str || ifprops.gateway_str; > let dns1_str = options.dns1_str || ifprops.dns1_str; > let dns2_str = options.dns2_str || ifprops.dns2_str; > let gateway = netHelpers.stringToIP(gateway_str); > > And remove `gateway` assignment in getIFProperties() as well. Just prevent > any other possible null value problems in the future. That's a good idea, thank you.
Addressed previous review comments.
Attachment #678711 - Attachment is obsolete: true
blocking-kilimanjaro: --- → ?
Noming basecamp because the issue blocks internet access on some devices.
blocking-basecamp: --- → ?
blocking-kilimanjaro: ? → ---
blocking-basecamp: ? → +
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: