Use enums for MobileRadioState in nsIMobileConnectionProvider.idl

RESOLVED FIXED in 2.1 S7 (24Oct)

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: anshulj, Assigned: jessica)

Tracking

unspecified
2.1 S7 (24Oct)
x86_64
Windows 7

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 4 obsolete attachments)

(Reporter)

Description

4 years ago
The bug 898445 introduced the  MobileRadioState enum in the MozMobileConnection.webidl but no corresponding constants were added in nsIMobileConnectionProvider.idl. 

Please consider adding enums for radio state for getRadioState API in nsIMobileConnectionProvider.idl to strictly define the IPDL interface.

Updated

4 years ago
Blocks: 1047203
(Assignee)

Updated

4 years ago
Assignee: nobody → jjong
(Assignee)

Comment 1

4 years ago
Created attachment 8509328 [details] [diff] [review]
Part 1: idl changes, v1.
(Assignee)

Comment 2

4 years ago
Created attachment 8509331 [details] [diff] [review]
Part 2: dom/ipc changes, v1.
(Assignee)

Comment 3

4 years ago
Created attachment 8509333 [details] [diff] [review]
Part 3: gonk changes, v1.
(Assignee)

Comment 4

4 years ago
Created attachment 8510033 [details] [diff] [review]
Part 2: dom/ipc changes, v2.

Added assertion stuff.
Attachment #8509331 - Attachment is obsolete: true
(Assignee)

Comment 5

4 years ago
Comment on attachment 8509328 [details] [diff] [review]
Part 1: idl changes, v1.

Hsinyi, may I have your review? Thanks.
Attachment #8509328 - Flags: review?(htsai)
(Assignee)

Comment 6

4 years ago
Comment on attachment 8510033 [details] [diff] [review]
Part 2: dom/ipc changes, v2.

Edgar, may I have your review? Thanks.
Attachment #8510033 - Flags: review?(echen)
(Assignee)

Updated

4 years ago
Attachment #8509333 - Flags: review?(echen)
Comment on attachment 8509328 [details] [diff] [review]
Part 1: idl changes, v1.

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

r=me with comment addressed.

::: dom/mobileconnection/gonk/nsIGonkMobileConnectionService.idl
@@ +39,5 @@
>  
>    void notifyIccChanged(in unsigned long clientId, in DOMString iccId);
>  
>    void notifyNetworkSelectModeChanged(in unsigned long clientId,
> +                                      in long mode);

This shouldn't be here ;)
Attachment #8509328 - Flags: review?(htsai) → review+

Comment 8

4 years ago
Comment on attachment 8509333 [details] [diff] [review]
Part 3: gonk changes, v1.

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

Looks good to me, thank you.
Attachment #8509333 - Flags: review?(echen) → review+

Updated

4 years ago
Attachment #8510033 - Flags: review?(echen) → review+

Comment 9

4 years ago
The marionette tests in try server is broken now. Please remember to run the tests manually in your local environment before pushing into inbound. Thank you. :)
(Assignee)

Comment 10

4 years ago
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #7)
> Comment on attachment 8509328 [details] [diff] [review]
> Part 1: idl changes, v1.
> 
> Review of attachment 8509328 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me with comment addressed.
> 
> ::: dom/mobileconnection/gonk/nsIGonkMobileConnectionService.idl
> @@ +39,5 @@
> >  
> >    void notifyIccChanged(in unsigned long clientId, in DOMString iccId);
> >  
> >    void notifyNetworkSelectModeChanged(in unsigned long clientId,
> > +                                      in long mode);
> 
> This shouldn't be here ;)

oh, this was a miss in bug 1052842, I though to fix it here, or should I file a follow-up bug to bug 1052842?
(Assignee)

Comment 11

4 years ago
(In reply to Edgar Chen [:edgar][:echen] from comment #9)
> The marionette tests in try server is broken now. Please remember to run the
> tests manually in your local environment before pushing into inbound. Thank
> you. :)

I have ran the tests locally, tests looks good but I did find some test_wifi_* failures and noticed Henry about them. Thanks for the remind. :)
(Assignee)

Comment 12

4 years ago
Created attachment 8510881 [details] [diff] [review]
[final] Part 1: idl changes. r=hsinyi

- Per comment 7, revert changes for notifyNetworkSelectModeChanged(), this will be done in Bug 1052842.
- Add r=hsinyi.
Attachment #8509328 - Attachment is obsolete: true
Attachment #8510881 - Flags: review+
(Assignee)

Comment 13

4 years ago
Created attachment 8510882 [details] [diff] [review]
[final] Part 2: dom/ipc changes. r=echen

Add r=echen
Attachment #8510033 - Attachment is obsolete: true
Attachment #8510882 - Flags: review+
(Assignee)

Comment 14

4 years ago
Created attachment 8510884 [details] [diff] [review]
[final] Part 3: gonk changes. r=echen

Add r=echen
Attachment #8509333 - Attachment is obsolete: true
Attachment #8510884 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/ae49c3d47718
https://hg.mozilla.org/mozilla-central/rev/70b014471e1b
https://hg.mozilla.org/mozilla-central/rev/6bfe180d72e9
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S7 (24Oct)
You need to log in before you can comment on or make changes to this bug.