Closed Bug 1031369 Opened 6 years ago Closed 5 years ago

[B2G][General][OTA] If SIM PIN is set no SIM card or data connection is present after OTA from 1.4 to 2.0

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:2.0+, b2g-v2.0 fixed, b2g-v2.1 fixed)

RESOLVED FIXED
2.0 S6 (18july)
blocking-b2g 2.0+
Tracking Status
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: bzumwalt, Assigned: qdot)

Details

(Whiteboard: [2.0-flame-test-run-2][systemsfe])

Attachments

(3 files)

Attached file Logcat
Description:
If user sets SIM pin on 1.4 then preforms OTA update to 2.0 no data connection (WiFi or 3G) is present. FTU Tour displays and user proceeds directly to homescreen without SIM PIN prompt. Restarting phone or accessing SIM PIN prompt manually through an app (e.g. Cost Control, SIM Security, etc.)

Updated from Flame 1.4 20140624000202 to Flame 2.0 20140626000202

Repro Steps:
1) Update a Flame to 20140624000202
2) Change update channel to Aurora
3) Set SIM PIN enabled in SIM security settings
4) Force check for update
5) Select update in notification tray and install after download completes
6) Proceed through new features tour to homescreen

Actual:
No cell data or wifi connection present after ota from 1.4 to 2.0

Expected:
Cell data and Wifi connection persist after OTA update.

Environmental Variables:
Device: Flame 2.0
Build ID: 20140626000202
Gaia: 6a1373340b40fcfe901336bc9e80676e5f2ba979
Gecko: 82ef9bf64d87
Version: 32.0a2 (2.0) 
Firmware Version: v121-2

User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Notes:
Repro frequency: 3/3, 100%
Link to failed test case: https://moztrap.mozilla.org/manage/case/11472/
See attached: screenshot & logcat
Attached image Screenshot
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?]
Unable to test OTA on Buri at this time
Open C blocked from testing by bug bug 1029932
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
(In reply to Brogan Zumwalt from comment #0)
> Created attachment 8447254 [details]
> Logcat
> 
> Description:
> If user sets SIM pin on 1.4 then preforms OTA update to 2.0 no data
> connection (WiFi or 3G) is present. FTU Tour displays and user proceeds
> directly to homescreen without SIM PIN prompt. Restarting phone or accessing
> SIM PIN prompt manually through an app (e.g. Cost Control, SIM Security,
> etc.)

The last sentence here I think needs to be finished - what happens in these two use cases?
QA Whiteboard: [QAnalyst-Triage+]
Keywords: qawanted
Component: General → Gaia::System
In reply to :jsmith - Sentence for two use cases should be: "Restarting phone or accessing SIM PIN prompt manually through an app (e.g. Cost Control, SIM Security,etc.) causes SIM PIN entry screen to appear, after which data connection for existing SIM card should appear."
Summary: [B2G][General][OTA] If SIM PIM is set no SIM card or data connection is present after OTA from 1.4 to 2.0 → [B2G][General][OTA] If SIM PIN is set no SIM card or data connection is present after OTA from 1.4 to 2.0
Can't do a regression check here since we don't have 1.3 --> 1.4 available for Flame. Noming this because I think this is a bad first use of the phone post updating.
blocking-b2g: --- → 2.0?
Keywords: qawanted
QA Whiteboard: [QAnalyst-Triage+][lead-review+]
blocking-b2g: 2.0? → 2.0+
Doh, 2.0+ and nobody caring. Couldn't it be a dupe of bug 1035739 ?
Flags: needinfo?(bzumwalt)
Could we please check whether this still happens ?

And if it does happen:
 - does it work with one or two SIMs ?
 - SIM PIN set on one or two cards ?
 - does the SIM PIN dialog comes when starting Dialer ?
(In reply to Alexandre LISSY :gerard-majax from comment #6)
> Doh, 2.0+ and nobody caring. Couldn't it be a dupe of bug 1035739 ?

I don't think this is related - this is using an OTA flow.

(In reply to Alexandre LISSY :gerard-majax from comment #7)
> Could we please check whether this still happens ?

I don't see the reason the retest here. There's no evidence presented on the bug here that something has been fixed to warrant a need to retest.
Flags: needinfo?(bzumwalt)
Assignee: nobody → kyle
Whiteboard: [2.0-flame-test-run-2] → [2.0-flame-test-run-2][systemsfe]
Target Milestone: --- → 2.0 S6 (18july)
(In reply to Alexandre LISSY :gerard-majax from comment #7)
> Could we please check whether this still happens ?

I just checked. Still has issue as of today's builds (loses wifi info, SIM doesn't come up).

> And if it does happen:
>  - does it work with one or two SIMs ?
>  - SIM PIN set on one or two cards ?

My test was 1 SIM only so far.

>  - does the SIM PIN dialog comes when starting Dialer ?

Nope. Requires phone reboot to bring up PIN dialog at all.

Also, after rebooting, if SIM PIN is entered, it will show there is a cell radio connection/quality. However, SIM Manager/Call Settings still say "SIM card not ready" even though lockscreen says "T-Mobile (Roaming)". 

Additionally, Wifi credentials are /gone/. It won't reconnect to wifi network it was on without resetting wifi password. Once wifi password is reset, wifi works again. 

Not sure if these are actually separate bugs.
Ok, just checked with no SIM in on OTA. Wifi info still disappears. I think these are two separate bugs as I can't find any other bugs on wifi ota issues, but I'm gonna ni? jsmith to see if he's aware and knows of anything.
Flags: needinfo?(jsmith)
(In reply to Kyle Machulis [:kmachulis] [:qdot] from comment #10)
> Ok, just checked with no SIM in on OTA. Wifi info still disappears. I think
> these are two separate bugs as I can't find any other bugs on wifi ota
> issues, but I'm gonna ni? jsmith to see if he's aware and knows of anything.

Don't know of other bugs off the top of my head. Feel free to split this out into two bugs if need be.
Flags: needinfo?(jsmith)
Splitting wifi bug off to Bug 1036564
This bug is caused by the FTU Launcher always sending the 'ftuopen' event when the FTU opens. In the first run case, this is fine because the FTU handles SIM PIN entry itself. However, in the upgrade case, FTU SIM PIN entry is skipped, but the system SIM PIN entry automatically closes when it sees the FTU coming up. The SIM Lock Dialog should check whether we are upgrading or not, and only close the System SIM PIN Dialog if this is a first run situation.
It also appears that the FTU throws the SIM into an odd, unrecoverable state when run after upgrade. My current testing method is to just have VersionHelper.isUpgrade() always return true, so we'll always get the ftu upgrade to come up. Even if I can get the SIM Lock screen to come up after this, the SIM will never unlock, even with a correct PIN. If I restore the old functionality and the FTU doesn't come up, the SIM PIN entry goes back to working fine.
Attachment #8453443 - Flags: review?(sfoster)
Attachment #8453443 - Flags: review?(lissyx+mozillians)
Note that this is currently failing tbpl due to unit tests I forgot to update. Working on that now. Code should still be valid, just have to teach unit tests about the new upgrade stuff.
Kyle, I'm no peer of FTU, are you sure you flagged the proper people for reviewing this? :)
I'm also not a peer, though I did a drive-by and it looks like the right changes to me. I'd suggest fcampo for review.
Attachment #8453443 - Flags: review?(sfoster)
Comment on attachment 8453443 [details] [review]
Patch 1 (v1) - Show System SIM PIN after FTU Upgrade Path

Micro-nit noted in GH. LGTM.
Attachment #8453443 - Flags: feedback+
Attachment #8453443 - Flags: review?(lissyx+mozillians) → review?(fernando.campo)
Ok, was going by blames and who I'd talked to, not peers, heh. Reassigned. :)
Just pushed new version with fixed unit tests.
Comment on attachment 8453443 [details] [review]
Patch 1 (v1) - Show System SIM PIN after FTU Upgrade Path

Left few comments with nits (mainly optional), but so far, FTU part looks good.

Sadly, this modifies system parts, and I'm no peer in there, so asking for review to alive for that part.
Attachment #8453443 - Flags: review?(fernando.campo)
Attachment #8453443 - Flags: review?(alive)
Attachment #8453443 - Flags: review+
Ok, fixed up all the nits from sfoster and fcampo. Now awaiting alive's review. :)
I am concern about the UX - show sim pin dialog before 'upgraded FTU' seems odd.
Could we just show it after ftudone event? Is VersionHelper not reporting 'isUpgrade' after FTU is done?
Comment on attachment 8453443 [details] [review]
Patch 1 (v1) - Show System SIM PIN after FTU Upgrade Path

I'd like UX to look into this. What is expected? Before or after ftu?
My suggestion would be, if ftudone(FTU is closed) is got, no matter we are fresh ftu or upgraded ftu, just show the sim pin unlock dialog if the user does not unlock it during FTU(Normal FTU has its own unlock UI).
Attachment #8453443 - Flags: ui-review?(firefoxos-ux-bugzilla)
Comment on attachment 8453443 [details] [review]
Patch 1 (v1) - Show System SIM PIN after FTU Upgrade Path

Let's wait UX opinion.
And please read mock_dom_request to implement your mock_version_helper :D
Attachment #8453443 - Flags: review?(alive)
So UX confirmed they want to show SIM PIN once phone is rebooted, and before upgraded FTU.
I am fine with that. So the patch works for me, but please fix the test as described above and ask review again.
Following up on comment 27, as per the proposal in bug 1017591, this require the use of a dialog as opposed to displaying the FTU screen. Jenny, do we have a wireframe for this currently? This is for the 2.0 release.

Also, to clarify, this would proceed the tour portion of the FTU when there is no setup flow during an upgrade. The idea is we don't want the user to miss a phone call during the tour.
Flags: needinfo?(jelee)
Flags: needinfo?(hhsu)
(In reply to Rob MacDonald [:robmac] from comment #28)
> Following up on comment 27, as per the proposal in bug 1017591, this require
> the use of a dialog as opposed to displaying the FTU screen. Jenny, do we
> have a wireframe for this currently? This is for the 2.0 release.

Is using the system dialog for this not sufficient?
(In reply to Alive Kuo [:alive][NEEDINFO!][Berlin 7/14-7/18] from comment #26)
> And please read mock_dom_request to implement your mock_version_helper :D

Ok, well, read mock_dom_request, have no idea how that's supposed to help. Is there another instance of simulating promises using mock_dom_request? Do we have to mock promises too?
Flags: needinfo?(alive)
(In reply to Kyle Machulis [:kmachulis] [:qdot] from comment #30)
> (In reply to Alive Kuo [:alive][NEEDINFO!][Berlin 7/14-7/18] from comment
> #26)
> > And please read mock_dom_request to implement your mock_version_helper :D
> 
> Ok, well, read mock_dom_request, have no idea how that's supposed to help.
> Is there another instance of simulating promises using mock_dom_request? Do
> we have to mock promises too?

Grep is your friend.
Searching 229 files for "mock_dom_request"

/Users/alive/Projects/gaia/apps/system/test/unit/bluetooth_transfer_test.js:
    3  require('/shared/test/unit/mocks/mock_navigator_moz_set_message_handler.js');
    4  require('/shared/test/unit/mocks/mock_event_target.js');
    5: require('/shared/test/unit/mocks/mock_dom_request.js');
    6  require('/test/unit/mock_navigator_get_device_storage.js');
    7  require('/test/unit/mock_bluetooth.js');

/Users/alive/Projects/gaia/apps/system/test/unit/mock_dom_request_test.js:
    3  
    4  require('/shared/test/unit/mocks/mock_event_target.js');
    5: require('/shared/test/unit/mocks/mock_dom_request.js');
    6  
    7  suite('MockDOMRequest', function() {

/Users/alive/Projects/gaia/apps/system/test/unit/nfc_handover_manager_test.js:
   14  requireApp('system/test/unit/mock_system_nfc_connect_dialog.js');
   15  requireApp('system/shared/test/unit/mocks/mock_event_target.js');
   16: requireApp('system/shared/test/unit/mocks/mock_dom_request.js');
   17  requireApp('system/test/unit/mock_bluetooth_transfer.js');
   18  requireApp('system/test/unit/mock_bluetooth.js');

/Users/alive/Projects/gaia/apps/system/test/unit/nfc_manager_test.js:
    8  require('/shared/js/nfc_utils.js');
    9  require('/shared/test/unit/mocks/mock_event_target.js');
   10: require('/shared/test/unit/mocks/mock_dom_request.js');
   11  require('/test/unit/mock_screen_manager.js');
   12  requireApp('system/test/unit/mock_activity.js');

/Users/alive/Projects/gaia/apps/system/test/unit/screenshot_test.js:
    5  requireApp('system/js/screenshot.js');
    6  requireApp('system/shared/test/unit/mocks/mock_event_target.js');
    7: requireApp('system/shared/test/unit/mocks/mock_dom_request.js');
    8  requireApp('system/test/unit/mock_navigator_get_device_storage.js');
    9  require('/shared/test/unit/mocks/mock_l10n.js');

/Users/alive/Projects/gaia/apps/system/test/unit/storage_watcher_test.js:
    6  requireApp('system/test/unit/mock_notification_screen.js');
    7  requireApp('system/shared/test/unit/mocks/mock_event_target.js');
    8: requireApp('system/shared/test/unit/mocks/mock_dom_request.js');
    9  requireApp('system/test/unit/mock_navigator_get_device_storage.js');
   10  

6 matches across 6 files

Please read https://groups.google.com/d/msg/mozilla.dev.gaia/cSeNH2arb44/gYBmxhiCFpkJ
Flags: needinfo?(alive)
A "yes, you need to write mock_promises" would've sufficed.

Off to continue completely reimplementing our browser in mocks.
Comment on attachment 8453443 [details] [review]
Patch 1 (v1) - Show System SIM PIN after FTU Upgrade Path

Ok. Fixed up mock_version_helper to be more instrumentable and not rely on throwing to the event loop. Could possibly try extracting that out to mock_promise but don't feel like I'm super clear on the scope of that, not to mention, blocker due soon, so willing to file followup for that.
Attachment #8453443 - Flags: review?(alive)
(In reply to Rob MacDonald [:robmac] from comment #28)

Hello Rob, I think using the system PIN screen would be sufficient here. Latest spec will be released shortly within 2 weeks. Thanks!
Flags: needinfo?(jelee)
Flags: needinfo?(hhsu)
Attachment #8453443 - Flags: review?(alive) → review+
Whiteboard: [2.0-flame-test-run-2][systemsfe] → [2.0-flame-test-run-2][systemsfe][ETA=7/16]
Passed gaia-try outside of a burn on integration that seems to be happening on every build, so calling it good.

https://github.com/mozilla-b2g/gaia/commit/24b0d6d1e9f68540161e3d3e76cfd39a5dae2727
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
v2.0: https://github.com/mozilla-b2g/gaia/commit/c02f9666687de954b408797b633f8b63809bd36d
Whiteboard: [2.0-flame-test-run-2][systemsfe][ETA=7/16] → [2.0-flame-test-run-2][systemsfe]
You need to log in before you can comment on or make changes to this bug.