Closed Bug 1168021 Opened 7 years ago Closed 7 years ago

[NetworkManager] consider expanding the nsINetworkInterface parameter in NetworkService interface

Categories

(Firefox OS Graveyard :: RIL, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(firefox41 fixed)

RESOLVED FIXED
2.2 S14 (12june)
Tracking Status
firefox41 --- fixed

People

(Reporter: jessica, Assigned: jessica)

References

Details

Attachments

(2 files, 2 obsolete files)

Blocks: 904542
I'd like to fix this first, it will make bug 1167132 easier and cleaner.
Assignee: nobody → jjong
Attached patch Part 1: interface changes, v1. (obsolete) — Splinter Review
Edgar, may I have you review on the interface changes? Thanks.
Attachment #8610964 - Flags: review?(echen)
Attached patch Part 2: implementation, v1. (obsolete) — Splinter Review
Comment on attachment 8610964 [details] [diff] [review]
Part 1: interface changes, v1.

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

Looks good, thank you.
Attachment #8610964 - Flags: review?(echen) → review+
Comment on attachment 8611038 [details] [diff] [review]
Part 2: implementation, v1.

Edgar, here is the implementation part, mind taking a look at it? Thanks.

BTW, I've changed all arguments in NetworkService to start with 'a' prefix.
Attachment #8611038 - Flags: review?(echen)
Comment on attachment 8611038 [details] [diff] [review]
Part 2: implementation, v1.

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

Looks good, thank you.

::: dom/system/gonk/NetworkService.js
@@ +528,2 @@
>  
> +    this.controlMessage(aConfig, function setDhcpServerResult(response) {

Nit: s/function setDhcpServerResult(response)/function(aResponse)/
Attachment #8611038 - Flags: review?(echen) → review+
(In reply to Edgar Chen [:edgar][:echen] from comment #6)
> Comment on attachment 8611038 [details] [diff] [review]
> Part 2: implementation, v1.
> 
> Review of attachment 8611038 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good, thank you.
> 
> ::: dom/system/gonk/NetworkService.js
> @@ +528,2 @@
> >  
> > +    this.controlMessage(aConfig, function setDhcpServerResult(response) {
> 
> Nit: s/function setDhcpServerResult(response)/function(aResponse)/

Thanks Edgar, you are really good at catching things! :D
Add r=echen.
Attachment #8610964 - Attachment is obsolete: true
Attachment #8614587 - Flags: review+
Address nit in review comment 6 and add r=echen.
Attachment #8611038 - Attachment is obsolete: true
Attachment #8614589 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/d87eeedbc93c
https://hg.mozilla.org/mozilla-central/rev/2ec6ba38c4c6
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S14 (12june)
Alphan, this has been fixed. Note that you'll still get unknown values for cell identity if there is a valid signal strength, since we filter out cell info when all values are unknown. Please let us know if you find any other issue. Thanks.
Please ignore last comment, wrong bug. :/
You need to log in before you can comment on or make changes to this bug.