1140314 - [B2G][ICC] Improve SIMSlotManager to Publish 'simslotready' at init() stage.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Bevis Tseng[:bevis][:btseng](Exited)]

In bug 1114935, we are trying to refactor IccManager with IPDL to deprecate RILContentHelper in the future.
During refactoring, we found that SIMLockSceen was not shown at device boot-up if SIM PIN is enabled.
(Note: The one in FTU works properly.)
After further analysis, the root cause is addressed in bug 1114935 comment 95.

Hence, we would like to rise our concern to Gaia to see if we can improve this in Gaia before landing the fix of bug 1114935.

The related PR will be uploaded later for reference.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] bevis-tseng:bug_1140314 > mozilla-b2g:master

Comment 2

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: PR v1 - Improve SIMSlotManager to Publish 'simslotready' at init() stage.

We found this impact when refector MozIccMananger with IPDL and 
would like to improve this in SIMSlotManager from the perspective of MozIccManager APIs.
More detailed analysis in addressed in bug 1114935 comment 95.

Hi Alive,

May I have your review for this change?

Thanks!

Comment 3

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8573857 [details] [review]
PR v1 - Improve SIMSlotManager to Publish 'simslotready' at init() stage.

The idea sounds sane; it would be better if you have unit tests added.

Comment 4

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #3)
> Comment on attachment 8573857 [details] [review]
> PR v1 - Improve SIMSlotManager to Publish 'simslotready' at init() stage.
> 
> The idea sounds sane; it would be better if you have unit tests added.

There is one test failure in simslot_manager_test, please fix it before merging. Request review if you think it needs extra review.

Comment 5

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #4)
> (In reply to Alive Kuo [:alive][NEEDINFO!] from comment #3)
> > Comment on attachment 8573857 [details] [review]
> > PR v1 - Improve SIMSlotManager to Publish 'simslotready' at init() stage.
> > 
> > The idea sounds sane; it would be better if you have unit tests added.
> 
> There is one test failure in simslot_manager_test, please fix it before
> merging. Request review if you think it needs extra review.

Thanks! I'll update patch to address this. :)

Comment 6

[GitHub Autolander Bot]

Attachment: [gaia] bevis-tseng:bug_1140314 > mozilla-b2g:master

Comment 7

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: PR v2 - Improve SIMSlotManager to Publish 'simslotready' at init() stage. r=alive

update v2 to address the test case failure.

Comment 8

[Bevis Tseng[:bevis][:btseng](Exited)]

try server result is green. :)
https://treeherder.mozilla.org/#/jobs?repo=gaia-try&revision=fb87b9fe418c

Comment 9

[GitHub Autolander Bot]

Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.

Comment 10

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8573857 [details] [review]
PR v1 - Improve SIMSlotManager to Publish 'simslotready' at init() stage.

https://github.com/mozilla-b2g/gaia/pull/28743

Comment 11

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8575184 [details] [diff] [review]
PR v2 - Improve SIMSlotManager to Publish 'simslotready' at init() stage. r=alive

Hi Alive,

Sorry, I created a new PR instead of updating the original one which was removed from github, and this new PR is rejected by autolander because I marked the r+ with my name.

May I have your review again for this change?
BTW, the try server result of this patch is green now:
https://treeherder.mozilla.org/#/jobs?repo=gaia-try&revision=fb87b9fe418c

Thanks!

Comment 12

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8575184 [details] [diff] [review]
PR v2 - Improve SIMSlotManager to Publish 'simslotready' at init() stage. r=alive

The only question is please also test Dual SIM: one SIM is detected but another is not when init. Please flag review again once addressed, thanks.

Comment 13

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8575184 [details] [diff] [review]
PR v2 - Improve SIMSlotManager to Publish 'simslotready' at init() stage. r=alive

Hi Alive,

I've added one more test case with 1st slot ready and 2nd slot absent.
To achieve, additional modification in MockSIMSlot is required.

May I have your review again for this change?

Thanks!

ps. update tryserver result for reference:
https://treeherder.mozilla.org/#/jobs?repo=gaia-try&revision=33b1175cd8a4

Comment 14

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8575184 [details] [diff] [review]
PR v2 - Improve SIMSlotManager to Publish 'simslotready' at init() stage. r=alive

Thanks!

Comment 15

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/ce4ddf4af16af61866e69edd29b0da06869a3a2c