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)
Tracking
(blocking-basecamp:+, firefox18 fixed, firefox19 fixed)
RESOLVED
FIXED
blocking-basecamp | + |
People
(Reporter: swu, Assigned: swu)
Details
Attachments
(1 file, 1 obsolete file)
3.34 KB,
patch
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•12 years ago
|
||
Comment 2•12 years ago
|
||
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+
Assignee | ||
Comment 3•12 years ago
|
||
(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.
Assignee | ||
Comment 4•12 years ago
|
||
Addressed previous review comments.
Attachment #678711 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
blocking-kilimanjaro: --- → ?
Assignee | ||
Comment 5•12 years ago
|
||
Noming basecamp because the issue blocks internet access on some devices.
blocking-basecamp: --- → ?
blocking-kilimanjaro: ? → ---
Assignee | ||
Comment 6•12 years ago
|
||
Updated•12 years ago
|
blocking-basecamp: ? → +
Comment 7•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 8•12 years ago
|
||
status-firefox18:
--- → fixed
status-firefox19:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•