1058376 - [Devices][Storage] Could not detect internal storage

Status: VERIFIED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Alison Shiue]

Attachment: open_media_storage.log

Device    Flame
Gaia      1934a2297ffc0d90424cd9cd3294c4a8c74a7333
Gecko     https://hg.mozilla.org/mozilla-central/rev/18901d4f3edd
BuildID   20140825160203
Version   34.0a1

After taking some pictures, record video, etc.(nothing special steps) and found the device could not detect internal storage, also could not open Gallery, Music, and Video app since no memory card found.

But all files exist when check /storage/sdcard folder.

Comment 1

[Alison Shiue]

[Blocking Requested - why for this release]:

Comment 2

[Alison Shiue]

Attachment: detect_no_storages.jpg

Comment 3

[Jason Smith [:jsmith]]

QA Wanted for branch checks.

Comment 4

[Cody Roesch [:croesch]]

This bug repro's on: Flame 2.1, OpenC 2.1

Actual Results: Loss of SDcard storage and Internal storage options in Settings after taking photos and video.

Repro Rate: 5/5

Environmental Variables:
Device: Flame Master
BuildID: 20140825172052
Gaia: 4d1d0ea5a82cddeeab497774cfa1703639e3c7d9
Gecko: dc352a7bf234
Version: 34.0a1 (Master) 
Firmware Version: v123
------------------------------------------------
Environmental Variables:
Device: Open_C Master
BuildID: 20140825172052
Gaia: 4d1d0ea5a82cddeeab497774cfa1703639e3c7d9
Gecko: dc352a7bf234
Version: 34.0a1 (Master) 
Firmware Version: P821A10V1.0.0B06_LOG_DL

------------------------------------------------
------------------------------------------------

This bug does NOT repro on: Flame 2.0, Flame 1.4

Actual Result: No problems losing Storage options.

Repro Rate: 0/12 attempts

Environmental Variables:
Device: Flame 2.0
BuildID: 20140825185056
Gaia: a51633e29a7826b6bf07cb1c5ad81b3217b9820a
Gecko: fdac649a65ac
Version: 32.0 (2.0) 
Firmware Version: v123
--------------------------------------------------
Environmental Variables:
Device: Flame 1.4
BuildID: 20140825062151
Gaia: cf9d74da6653efeb43d9653e81c61aa00e693a67
Gecko: cdcb73d0febc
Version: 30.0 (1.4) 
Firmware Version: v123

Comment 5

[Pi Wei Cheng [:piwei] (inactive)]

mozilla-inbound regression window:

Last Working Environmental Variables:
Device: Flame
BuildID: 20140821223018
Gaia: afcdd36f13e75adcdebe57d842a277fd587faf28
Gecko: c365e93b8e42
Version: 34.0a1 (2.1 Master)
Firmware: V123
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

First Broken Environmental Variables:
Device: Flame
BuildID: 20140821224417
Gaia: afcdd36f13e75adcdebe57d842a277fd587faf28
Gecko: a9c2b0b7adac
Version: 34.0a1 (2.1 Master)
Firmware: V123
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

Gaia is the same again so it's a Gecko issue.

Gecko pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c365e93b8e42&tochange=a9c2b0b7adac

Caused by bug 1040017.

Comment 6

[Joshua Mitchell (Inactive)]

Caused by bug 1040017 ? Patrick, can you take a look?

Comment 9

[Johnny Stenback (:jst)]

Need to block on this IMO, Dave, can you look into this, or redirect if we've got someone better suited to dig in here? Thanks!

Comment 10

[Dave Hylands [:dhylands]]

This looks like a dup of 1058425 which I'm already investigating.

So far, I haven't been able to reproduce, but jdarcangelo can, so I sent him a patch which adds some more logging to see if I can figure out what's going on.

Right now it looks like navigator.getDeviceStorages is returning a zero length array when it should be returning a array of 2 entries (on the flame).

Comment 11

[Dave Hylands [:dhylands]]

Attachment: Logging patch. DO NOT LAND.

Logging patch.

If anybody can reproduce with this logging, I would really like to get some logcat output.

Comment 12

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Sorry for late response. In comment 5 my patch was found possibly to be a cause of this bug, while I am currently on vacation and don't have a device around. I will look into this next week when I back to office. Keeping the ni as reminder.

Comment 13

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

The symptom described in comment 0 looks quite like SendFilePathUpdate is not broadcasted. However, I can't reproduce this with the version which bug 1040017 landed or current master. And I can see SendFilePathUpdate messages are sent to every non-Nuwa processes, including preallocated process.

Comment 14

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Talked with Alison, we found that this bug is not always reproducible, but it still happens sometimes in recent build.

Comment 15

[Pi Wei Cheng [:piwei] (inactive)]

This bug is causing a lot of other bugs regarding utilizing internal storage and can be constantly hit. Here's an updated STR I used for finding the window at comment 5. I've checked this STR against latest Flame 2.2, and the repro rate is 3/3.

STR:
0) Flash a phone to any of the affect build, or simply Factory Reset the phone via Settings
1) Progress past FTU without changing anything.
2) Launch Camera > take a picture
3) Go back to Homescreen and tap on Settings app
4) Long-press Home button and kill the Settings process
5) Enter Settings again > Media storage

Observe that the Media Storage section doesn't show any storage available. At this point all internal storage can't be utilized.

Tested on:
Device: Flame
BuildID: 20140910060915
Gaia: f108c706fae43cd61628babdd9463e7695b2496e
Gecko: 843332cc69af
Version: 35.0a1 (2.2 Master)
Firmware: V123
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Comment 16

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

I can now reproduce this bug with v123 base image + shallow flash latest gecko. It looks I shouldn't resend the SendFilePathUpdate messages which are observed by Nuwa's content parent after Nuwa forked a new process.

Comment 17

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

I think we should probably backout bug 1040017, I'd like to find a better way to get it fixed.

Comment 18

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

(In reply to Patrick Wang (Chih-Kai Wang) (:kk1fff) from comment #17)
> I think we should probably backout bug 1040017, I'd like to find a better
> way to get it fixed.

Wait. I can reproduce this only on V123 base image and shallow flashed gecko. If I build and flash whole image, it can't be reproduce anymore.

Comment 19

[Dave Hylands [:dhylands]]

The bug is a race condition. It will probably come and go as it sees fit and which exact combination trigger the problem over time will vary.

Bug 1063877 completely eliminates the Broadcast Volume request and the SendFileSystemUpdate calls that were occuring during early process startup.

For the BroadcatVolume/SendFileSystemUpdate, what should happen is that the Broadcast from the child to the parent is the thing that should be redone after a child is started from a Nuwa process.

What happens under normal (Nuwa not in the picture) is this:

1 - parent launches child
2 - child issues BroadcastVolume
3 - parent issues SendFileSystemUpdate containing volume state - this is the "initial" state
4 - parent later issues SendFileSystemUpdate calls as the state changes - child tracks updated state

If step 2 & 3 happen before the Nuwa process is "frozen" then the Nuwa process gets a bad "initial" state which can have a variety of issues.

If the Nuwa process is frozen between steps 1&2 then everything is good.

If the Nuwa process is frozen between steps 2&3 then the child has the wrong initial state (i.e. no volumes)

If the Nuwa process is frozen between steps 3&4 then the chilsd has a stale initial state (i.e. knows about the volumes, but the volumes may have changed state in the meantime).

I didn't think that SendFilePathUpdate should be sent prior to the child executing the app code. If it is, then we should investigate that and see if we can understand what's going on.

Comment 20

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

One of what bug 1040017 do is resending the last SendFilePathUpdate that Nuwa's ContentParent observed. What I see when reproducing this bug is that Nuwa resends SendFilePathUpdate message (which means Nuwa registered the observer), does this imply Nuwa's child start to track SendFilePathUpdate and something went wrong before Nuwa freeze? Or we can just delay resending of SendFilePathUpdate?

Comment 22

[No-Jun Park [:njpark]]

Just to mention that this happens in KK base image (v180) as well, per Bug 1066128.

Comment 23

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Attachment: Patch

Ignoring file updates notifications in Nuwa.

Comment 24

[Dave Hylands [:dhylands]]

Comment on attachment 8489229 [details] [diff] [review]
Patch

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

Looks good to me. Just a couple minor suggestions to consider...

::: dom/ipc/ContentParent.cpp
@@ -2519,5 @@
> -                                                  mReinitializeData->mFilePathUpdateStorageName,
> -                                                  mReinitializeData->mFilePathUpdatePath,
> -                                                  mReinitializeData->mFilePathUpdateReason);
> -        }
> -        if (mReinitializeData->mReceivedFilePathUpdate) {

This is using mReceivedFilePathUpdate rather than mReceivedFileSystemUpdate - which may have contributed to the original problem.

It doesn't matter now, I just happened to notice and thought I'd point it out.

@@ +2633,3 @@
>  
>  #ifdef MOZ_NUWA_PROCESS
>          if (!(IsNuwaReady() && IsNuwaProcess())) {

If you put the opening curly brace after the #endif, then the closing curly brace doesn't need an #ifdef around it.

@@ +2670,2 @@
>          if (!(IsNuwaReady() && IsNuwaProcess())) {
>  #endif

same comment about opening curly brace here as well.

Comment 25

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

(In reply to Dave Hylands [:dhylands] from comment #24)
> ::: dom/ipc/ContentParent.cpp
> @@ -2519,5 @@
> > -                                                  mReinitializeData->mFilePathUpdateStorageName,
> > -                                                  mReinitializeData->mFilePathUpdatePath,
> > -                                                  mReinitializeData->mFilePathUpdateReason);
> > -        }
> > -        if (mReinitializeData->mReceivedFilePathUpdate) {
> 
> This is using mReceivedFilePathUpdate rather than mReceivedFileSystemUpdate
> - which may have contributed to the original problem.
> 
> It doesn't matter now, I just happened to notice and thought I'd point it
> out.

Ah yes. I realized this after reading this comment. It is going to be removed anyway.

Comment 27

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/330a92fcc3a7

Comment 28

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/330a92fcc3a7

Comment 29

[Julien Wajsberg [:julienw]]

Hey Patrick, don't forget that you need to ask approval-mozilla-aurora on the patch so that it's uplifted to 2.1 :) Thanks!

Comment 30

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Attachment: Patch for uplift

Approval Request Comment

[Feature/regressing bug #]: 1040017
[User impact if declined]: user will not be able to access sd storage in some case.
[Describe test coverage new/current, TBPL]: manual test, and it happens in daily use.
[Risks and why]: Low, this just prevents unnecessary updates.
[String/UUID change made/needed]: no

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/55408c8c8d7a

Comment 33

[Roland Kunkel [:RolandK]]

I am still seeing this issue after peforming a full flash to the latest Flame KK build:

Enviromental Variables:
----------------------------------------
Device:  Flame 2.1
BuildID: 20140922063004
Gaia: 689c4ad4d8c3e4aa95805a2e49ae6cf786a1ae91
Gecko: 185fc54d29c1
Version: 34.0a2 (2.1)
Firmware: v180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

My internal storage is detected, but my SD card is not similar to bug 1068162 which was marked as a dupilicate to this one.

Comment 34

[Dave Hylands [:dhylands]]

(In reply to Roland Kunkel [:RolandK] from comment #33)
> I am still seeing this issue after peforming a full flash to the latest
> Flame KK build:
> 
> Enviromental Variables:
> ----------------------------------------
> Device:  Flame 2.1
> BuildID: 20140922063004
> Gaia: 689c4ad4d8c3e4aa95805a2e49ae6cf786a1ae91
> Gecko: 185fc54d29c1
> Version: 34.0a2 (2.1)
> Firmware: v180
> User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0
> 
> My internal storage is detected, but my SD card is not similar to bug
> 1068162 which was marked as a dupilicate to this one.

It would be extremely useful to get a copy of your logcat when this is happening.

Comment 35

[Roland Kunkel [:RolandK]]

Attachment: logcat

(In reply to Dave Hylands [:dhylands] from comment #34)

> It would be extremely useful to get a copy of your logcat when this is
> happening.

I have attached a logcat. I enounter this issue after a fresh flash. Enabling USB and checking Media Storage shows my SD card as not present even though I have an SD card in my device.

Comment 36

[Martin Shuman [:Marty]]

On both 2.1 and 2.2 Full Flashes with the KK base, I am still seeing the behaviour from bug 1066128, which was duped to this bug.
Plugging a device with USB Storage enabled into a computer results in Internal memory being visible, but not the SD card. 

Environmental Variables:
Device: Flame 2.1
BuildID: 20140923003005
Gaia: 3a2947df41a480de1457a6dcdbf46ad0af70d8e0
Gecko: df42b05782aa
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0


Environmental Variables:
Device: Flame 2.2 (Master)
BuildID: 20140922193018
Gaia: fe92ddd450e03b38edb2d465de7897971d68ac68
Gecko: 790f41c631cc
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 37

[Dave Hylands [:dhylands]]

The attached logcat shows "No Media" at startup (for the external sdcard).

The flame hardware doesn't support hot-swapping the sdcard (there is no card-detected signal - the sdcard socket doesn't support hot swapping).

This means that you have to power off the phone when inserting/removing the sdcard.

If you do remove/insert the sdcard while the phone is powered on, you'll get a wide variety of indeterminate behaviour.

Comment 38

[Roland Kunkel [:RolandK]]

(In reply to Dave Hylands [:dhylands] from comment #37)

I am not performing a hot swap during testing, I see this behaviour everytime I perform a full flash with my Flame device.

Comment 39

[Dave Hylands [:dhylands]]

I just got my flame-kk image built, and what I see is that the internal sd card is shared, but the external sdcard is not.

Both sdcards were being shared with my JB build. My gut tells me that this is related bug 1054131 which has something to do with the configuration files.

I've just started testing UMS and MTP for the flame-kk and will make sure we get things working :)

Comment 40

[Ryan VanderMeulen [:RyanVM]]

Should we take comments 33+ to a follow-up bug?

Comment 41

[Dave Hylands [:dhylands]]

We unduped bug 1066128 and have moved this over there.

Comment 42

[Tony Chung [:tchung]]

Please retest this is fixed on 2.1 and master, verifying it against comment 33 (external SD).

Comment 43

[Rachel Pribble [:rpribble]]

Verified as fixed on the Flame v2.1.

Environmental Variables:
Device: Flame 2.1
BuildID: 20140929000203
Gaia: 063de64a4ffc606e931ed7b09e93282713c46eca
Gecko: 055d46b81ed1
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Internal storage is detected.

Comment 44

[Rachel Pribble [:rpribble]]

Verified fixed on Flame v2.2.

Environmental Variables:
Device: Flame 2.2 Master
BuildID: 20140929040202
Gaia: 2834baf4c7e34fe6ef335f0469f6d0f593c5922b
Gecko: 9d66436af432
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Internal storage is detected.

Comment 45

[Rachel Pribble [:rpribble]]

Flame v2.1 build's Gonk hash: 5883a99b6528ced9dafaed8d3ca2405fb285537e.

Comment 46

[Rachel Pribble [:rpribble]]

fd2ace405952a06d16eb8a1334129007c3f179fa(In reply to Rachel Pribble [:rpribble] from comment #28)
> Flame v2.1 build's Gonk hash: 5883a99b6528ced9dafaed8d3ca2405fb285537e.

Previous listed is incorrectly listed as Flame v2.1 build's Gonk hash, but it is Flame v2.2 build's Gonk hash.
Restatement for verification:

Flame v2.1 build's Gonk hash: fd2ace405952a06d16eb8a1334129007c3f179fa
Flame v2.2 build's Gonk hash: 5883a99b6528ced9dafaed8d3ca2405fb285537e

Comment 47

[Rachel Pribble [:rpribble]]

Flame v2.1 & v2.2 device-flame hash: 52c909e821d107d414f851e267dedcd7aae2cebf