1123567 - [Media Storage] Media Storage does not show "Format SD card" button when inserted a unreadable SD card

Status: VERIFIED FIXED

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

Description

[Alison Shiue]

Gaia-Rev        f5b3d1b6cfa3e702033f613915ae637cb735cbfb
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/7b3fa8dff424
Build-ID        20150119162505
Version         37.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150119.195851
FW-Date         Mon Jan 19 19:59:02 EST 2015
Bootloader      L1TC000118D0

STR:
1. Inserted an unreadable SD card
2. Flash 2.2 build
3. Go to Settings -> Media storage

Expected result:
"Format SD card" button exists

Actual result:
(1) "Format SD card" button does not exist
(2) After enable UMS and plug->unplug USB cable, "SD Card" option added on Default Media Location

Comment 1

[howie [:howie]]

Triage: regression, blocking, regression window wanted, thanks

Comment 2

[Alison Shiue]

> Actual result:
> (2) After enable UMS and plug->unplug USB cable, "SD Card" option added on
> Default Media Location
This problem not only happened when inserted unreadable SD card but also without SD card, tracking this issue on bug 1123602

Comment 3

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Alison,
I'm noticing that in 2.1 the "Format SD card" button does not appear until after UMS has been enabled with a plug->unplug as well.  Is that the desired function for this?  That broke somewhere in the last couple of weeks for master and I can find that window, but it seems odd to me that is required at all to access this button.

Comment 4

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Bug 1091544  seems to have caused this issue.

B2g-inbound Regression Window

Last Working 
Environmental Variables:
Device: Flame 2.2
BuildID: 20150111231732
Gaia: 737ef4d7a8494bc6ece4bc3f37b4de977f01333c
Gecko: adfe0cd4c631
Version: 37.0a1 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

First Broken 
Environmental Variables:
Device: Flame 2.2
BuildID: 20150112002115
Gaia: 737ef4d7a8494bc6ece4bc3f37b4de977f01333c
Gecko: f3f06c6800fc
Version: 37.0a1 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Last Working gaia / First Broken gecko - Issue DOES occur
Gaia: 737ef4d7a8494bc6ece4bc3f37b4de977f01333c
Gecko: f3f06c6800fc

First Broken gaia / Last Working gecko - Issue does NOT occur
Gaia: 737ef4d7a8494bc6ece4bc3f37b4de977f01333c
Gecko: adfe0cd4c631

Gecko Pushlog: http://hg.mozilla.org/integration/b2g-inbound/pushloghtml?fromchange=adfe0cd4c631&tochange=f3f06c6800fc

Comment 5

[KTucker [:KTucker][Inactive 3/4/2016]]

Eden, can you take a look at this please? This could have been a result of the work that was done on bug 1091544

Comment 6

[Alison Shiue]

(In reply to Jayme Mercado [:JMercado] from comment #3)
> Alison,
> I'm noticing that in 2.1 the "Format SD card" button does not appear until
> after UMS has been enabled with a plug->unplug as well.  Is that the desired
In fact, just only plug and unplug USB cable, the SD format button will show up.
> function for this?  That broke somewhere in the last couple of weeks for
> master and I can find that window, but it seems odd to me that is required
> at all to access this button.

Yes, this is an issue. Since the symptom is different from 2.2, I fired another bug (bug 1123566)to track this 2.1 issue.

Comment 7

[Eden Chuang[:edenchuang]]

System app judges sdcard unrecognized status by tracking the volume's status changing with following patterns

 actionUnrecognised: nomedia->idle->checking->idle
 actionUnrecognisedAfterRechecked: idle->checking->idle
 actionUnrecognisedAfterFormatted: idle->formaing->idle
 actionUnrecognisedStorageRemoved: idle->nomedia

To resolve the problem of bug 1091544, gecko introduced a new status "mount-fail". So, the following patterns need to be modified as

 actionUnrecognised: nomedia->idle->checking->mount-fail
 actionUnrecognisedAfterRechecked: idle->checking->mount-fail
 
This modification could resolve the problem caused by the bug 1091544's patch. However, it can not resolve the bug 1123566.

Comment 8

[Eden Chuang[:edenchuang]]

Hi Ian

According to our previous discussion, please help to take the new volume status into the finite state machine design in system app. 

If you need any gecko's help, please feel free to info me.

And we might plan transferring the finite state machine into gecko in the future. Thanks.

Comment 9

[GitHub Autolander Bot]

Attachment: [PullReq] crh0716:1123567 to mozilla-b2g:master

Comment 10

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8561316 [details] [review]
[PullReq] crh0716:1123567 to mozilla-b2g:master

Ian, I've added the "Mount-Fail" state to the state machine and also fixed one of the major issue in which the code did not get the current status of the storage. As there are high chances that the status changes of the storage happen before `ExternalStorageMonitor` observes the event, so `volume.external.unrecognised` won't be set correctly until there are further status changes (like enable usb storage and pull the usb cable).

In this patch I set the value of `volume.external.unrecognised` to true only when the current status of the storage is `Mount-Fail`. The other logic remains almost the same except for replacing "Idle" in some cases with "Mount-Fail". Please help take a look at the patch, thanks!

Comment 11

[[e-mail], [e-mail]]

(In reply to Eden Chuang[:edenchuang] from comment #7)
> System app judges sdcard unrecognized status by tracking the volume's status
> changing with following patterns
> 
>  actionUnrecognised: nomedia->idle->checking->idle
>  actionUnrecognisedAfterRechecked: idle->checking->idle
>  actionUnrecognisedAfterFormatted: idle->formaing->idle
>  actionUnrecognisedStorageRemoved: idle->nomedia
> 
> To resolve the problem of bug 1091544, gecko introduced a new status
> "mount-fail". So, the following patterns need to be modified as
> 
>  actionUnrecognised: nomedia->idle->checking->mount-fail
>  actionUnrecognisedAfterRechecked: idle->checking->mount-fail
>  
> This modification could resolve the problem caused by the bug 1091544's
> patch. However, it can not resolve the bug 1123566.

Eden, I have two questions for the new status 'Mount-Fail'.

1. Is it possible that the state machine go through the rules? 

Idle(Unmounted) --> Formatting --> Mount-Fail(Unmounted)
(Go into 'Mount-Fail' state after formated the storage.)

Mount-Fail(Unmounted) --> NoMedia
(Leave from 'Mount-Fail' state, then go into 'NoMedia' state.)


2. What's the possible state(Previous_Status --> Mount-Fail --> Next_Status) nearby the new 'Mount-Fail' state?

I would like to check it again carefully for those patterns which were defined in Gaia state machine before. Thanks.

Comment 12

[Eden Chuang[:edenchuang]]

> 1. Is it possible that the state machine go through the rules? 
> 
> Idle(Unmounted) --> Formatting --> Mount-Fail(Unmounted)
> (Go into 'Mount-Fail' state after formated the storage.)

  for this case, I think the status sequence after Formatting would be
  
  Formatting->Idle->Checking->Mount-Fail

> Mount-Fail(Unmounted) --> NoMedia
> (Leave from 'Mount-Fail' state, then go into 'NoMedia' state.) 

  Yes, it cloud be. 
  Once the device supports hot-swappable sdcard, and vold detects the sdcard is removed, then the status will be set as NoMedia.

> 2. What's the possible state(Previous_Status --> Mount-Fail --> Next_Status)
> nearby the new 'Mount-Fail' state?

For now, the Previous_Status only can be "Checking". and the "Next_Status" can be "Formatting" or "NoMedia".

Comment 13

[[e-mail], [e-mail]]

Thanks for Eden's explanation. It's pretty clear for me!

Comment 14

[[e-mail], [e-mail]]

Comment on attachment 8561316 [details] [review]
[PullReq] crh0716:1123567 to mozilla-b2g:master

Arthur, I have addressed some comment on GitHub. We can revise the pattern of state flow more simple. Let's only check 'Checking --> Mount-Fail(Unmounted)' to identify the unrecognised SD card. And revise pattern 'actionUnrecognisedStorageRemoved' with the new state. Thanks.

Comment 15

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8561316 [details] [review]
[PullReq] crh0716:1123567 to mozilla-b2g:master

Ian, I've addressed the comments as you suggested. Basically I removed 'actionUnrecognisedAfterRechecked' and 'actionUnrecognisedAfterFormatted' because they are covered by 'actionUnrecognised'. Can you take a look at it again? Thanks.

Comment 16

[[e-mail], [e-mail]]

Comment on attachment 8561316 [details] [review]
[PullReq] crh0716:1123567 to mozilla-b2g:master

Thanks for Arthur's updated patch. Since we have the new status ''Mount-Fail', the state machine is more simply than before. r+ with me.

Comment 17

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8561316 [details] [review]
[PullReq] crh0716:1123567 to mozilla-b2g:master

Alive, could you help review the patch when you have a chance? Thanks.

Comment 18

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

Comment on attachment 8561316 [details] [review]
[PullReq] crh0716:1123567 to mozilla-b2g:master

LGTM

Comment 19

[Arthur Chen [:arthurcc][inactive after 6/30]]

master: 9e2e3726ee5c0d96dadb97aee4607375aa274613

Comment 20

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8561316 [details] [review]
[PullReq] crh0716:1123567 to mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Bug 1091544
[User impact] if declined: Users are not able to see the format button when inserting a unrecognized sd card.
[Testing completed]: Testing on the device. Unit tests were updated.
[Risk to taking this patch] (and alternatives if risky): Low because the modified code is more deterministic for the unrecognized states.
[String changes made]: None

Comment 21

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/ca64f2fe145909f31af266b1730874051ba76c78

Comment 22

[Alison Shiue]

Verified on
[2.2]
Gaia-Rev        77609916ca5ab721150fab2b7bc5c37f43ee3a5a
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/27ab8aa34201
Build-ID        20150301162504
Version         37.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150301.195931
FW-Date         Sun Mar  1 19:59:42 EST 2015
Bootloader      L1TC000118D0

[3.0]
Gaia-Rev        f34ce82a840ad3c0aed3bfff18517b3f6a0eb37f
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/eea6188b9b05
Build-ID        20150301160222
Version         39.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150301.193134
FW-Date         Sun Mar  1 19:31:45 EST 2015
Bootloader      L1TC100118D0