Closed Bug 1074590 Opened 5 years ago Closed 5 years ago

[MTP][KK] PC could not access the storage which has been formatted while MTP enable and USB plugged

Categories

(Firefox OS Graveyard :: MTP/UMS, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:2.1+, firefox34 wontfix, firefox35 wontfix, firefox36 fixed, b2g-v2.1 verified, b2g-v2.2 fixed)

VERIFIED FIXED
2.1 S8 (7Nov)
blocking-b2g 2.1+
Tracking Status
firefox34 --- wontfix
firefox35 --- wontfix
firefox36 --- fixed
b2g-v2.1 --- verified
b2g-v2.2 --- fixed

People

(Reporter: ashiue, Assigned: edenchuang)

References

Details

Attachments

(4 files, 7 obsolete files)

88.00 KB, image/png
Details
90.62 KB, text/x-log
Details
6.94 KB, patch
Details | Diff | Splinter Review
5.37 MB, video/3gpp
Details
Attached image format_storage.png
Gaia-Rev        13973ab50760d1e8bb773082163f0dff19d35a44
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-aurora/rev/6e317e075d04
Build-ID        20140928160204
Version         34.0a2

STR:
1. USB plugged in and enable MTP
2. Go to Settings -> Media storage, format a storage
3. Check the formatted storage on PC

Expect result:
1. PC can access the storage

Actual result:
1. PC cannot access the storage
QA Whiteboard: [COM=Storage]
[Blocking Requested - why for this release]:
functional issue
blocking-b2g: --- → 2.1?
Adding qawanted for branch checks.
Keywords: qawanted
Hi Eden,
I think this issue is similar to bug 1069744.
Hi Alison,
It is normal when Eden and I try to format sdcard with MTP enable and USB plugged.
However, it is abnormal when formatting internal storage.

For internal storage case,
we need to wait the solution of Bug 1073335 from vendor for KK cannot format internal storage problem.
Depends on: 1073335
QA Whiteboard: [COM=Storage] → [COM=MTP/UMS]
Component: General → MTP/UMS
QA Contact: ddixon
Branch Check

Issue DOES occur in Flame 2.2, 2.1. 

Device: Flame Master
Build ID: 20141010060203
Gaia: cc5da7b055e2b06fdeb46fa94970550392ee571d
Gecko: 097821fd89ed
Version: 35.0a1 (Master)
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0
-----------------------------------------------------
Device: Flame 2.1
Build ID: 20141010000201
Gaia: bc8eb493311c58f1f311a56b8b645b52bfbd2f71
Gecko: 72c13d8631ff
Version: 34.0a2 
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
-----------------------------------------------------
-----------------------------------------------------
Flame 2.0 Not Applicable 
(MTP is not implemented in Flame 2.0)
QA Whiteboard: [COM=MTP/UMS] → [COM=MTP/UMS][QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: qawanted
QA Whiteboard: [COM=MTP/UMS][QAnalyst-Triage?] → [COM=MTP/UMS][QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
Attached file mtp.log
This issue happens on windows for both internal storage and SD card.
Keywords: regression
The triage group decided this regression warrants blocking.
blocking-b2g: 2.1? → 2.1+
Assignee: nobody → echuang
Hi Andrew, this is not regression as there was no MTP before 2.1. Also Peripherals team owns the triage now, I wonder what triage group you mentioned.
Flags: needinfo?(overholt)
Hi Howie,

The triages-all-blocker-noms-3-times-a-week triage group :)  It's been going on for probably almost 2 years now.  Yesterday it was me, Dietrich, and No-Jun.  It's unfortunately at times that aren't friendly to Taiwan.

I don't know why we thought it was a regression so feel free to change the nom if you disagree.

Thanks,

Andrew
Flags: needinfo?(overholt)
I think the root cause is the mounted volume is not ready to be accessed.

During my testing, I found that when the bug is reproduced, I always get the sdcard1 total storage size is 0. 

following is logcat for this case.

1488 10-20 23:18:48.575   322   322 E QCALOG  : [MessageQ] ProcessNewMessage: [XTWiFi-PE] unknown deliver target [OS-Agent]
1489 10-20 23:18:48.595   190   295 I Vold    : Filesystem check completed OK
1490 10-20 23:18:48.635   190   295 D Vold    : blkid identified as /dev/block/vold/179:65: UUID="C2B0-1EF0" TYPE="vfat"
1491 10-20 23:18:48.635 31541 31556 I VolumeManager: Volume sdcard1 (2): changing state from Checking to Mounted @ '/storage/sdcard1' (6 observers) mountGeneration = 5, locked = 0
1492 10-20 23:18:48.635 31541 31556 I MozMtp  : Notify: Volume sdcard1 mStorageID 0x00020001 state changed to Mounted SharingEnabled: 1
1493 10-20 23:18:48.635 31541 31556 I MozMtp  : StorageAvailable: Adding Volume sdcard1 mStorageID 0x00020001 mountPoint /storage/sdcard1 to MozMtpDatabase
1494 10-20 23:18:48.635 31541 31556 I MozMtp  : AddStorage: create new storage for 'sdcard1'('/storage/sdcard1')
1495 10-20 23:18:48.635 31541 31556 I MozMtp  : AddStorage: Append the new storage to MozMtpDatabase's StorageArray
1496 10-20 23:18:48.635 31541 31556 I MozMtp  : AddStorage: added 3 items from tree '/storage/sdcard1'
1497 10-20 23:18:48.635 31541 31556 I MozMtp  : StorageAvailable: Adding Volume sdcard1 mStorageID 0x00020001 mountPoint /storage/sdcard1 to MtpServer
1498 10-20 23:18:48.635 31541 31556 I MozMtp  : StorageAvailable: volume 'sdcard1(/storage/sdcard1)' size informaiton
1499 10-20 23:18:48.635 31541 31556 I MozMtp  : StorageAvailable: f_bsize (block size): 4096
1500 10-20 23:18:48.635 31541 31556 I MozMtp  : StorageAvailable: f_frsize (fragment size): 4096
1501 10-20 23:18:48.635 31541 31556 I MozMtp  : StorageAvailable: f_blocks (size of fs in f_frsize units): 0
1502 10-20 23:18:48.635 31541 31556 I MozMtp  : StorageAvailable: f_bfree (free blocks): 0
1503 10-20 23:18:48.635 31541 31556 I MozMtp  : StorageAvailable: f_bavail (free blocks for unprivileged users): 0
1504 10-20 23:18:48.635 31541 31556 I MozMtp  : StorageAvailable: f_files (inodes): 0
1505 10-20 23:18:48.635 31541 31556 I MozMtp  : StorageAvailable: f_ffree (free inodes): 0
1506 10-20 23:18:48.635 31541 31556 I MozMtp  : StorageAvailable: f_favail (free inodes for unprivileged users): 0
1507 10-20 23:18:48.635 31541 31556 I MozMtp  : StorageAvailable: f_fsid (file system ID): 0
1508 10-20 23:18:48.635 31541 31556 I MozMtp  : StorageAvailable: f_flag (mount flags): 4097
1509 10-20 23:18:48.635 31541 31556 I MozMtp  : StorageAvailable: f_namemax (maximum filename length)255
1510 10-20 23:18:48.635   190   295 D Vold    : Volume sdcard1 state changing 3 (Checking) -> 4 (Mounted)
1511 10-20 23:18:48.645 31541 31556 I AutoMounter: UpdateState: ums:A1C0E0 mtp:A1C1E1 mode:3 usb:1 tryToShare:0 state:MTP_STARTED

At line 1492, MozMtpStorage of sdcard1 get the notification from volume's state change to be "Mounted". The state change is caused by vold, and MozMtpStorage::Notify() is called.
In the MozMtpStorage::Notify(), it will call MozMtpStorage::StorageAvailable() to add MtpStorage back to MtpDatabase and MtpServer under the proper situation. Once the MtpStorage is added into MtpServer, MtpServer will send event to PC to show the corresponding storage.

I add some code in MozMtpStorage::StorageAvailable() to get the volume size information before the time point we add storage back into MtpServer.
See the line 1497 to 1509 in logcat, it is very interesting that we get the number of free blocks is 0 at line 1502. At the time point, I am sure that vold finished the "Mount" work, but it does not mean the "Mounted" volume can be accessed. In fact, we always assume that once the volume is "Mounted", it also can be accessed, so we have no any access checking code in the flow. 

I think we need to add the volume access checking before we share it. 

Bug 1069744 is a dup of this.
Attachment #8508467 - Flags: feedback?(dhylands)
Duplicate of this bug: 1069744
Adding a time counter to escape infinite loop in volume access checking function.
Attachment #8508467 - Attachment is obsolete: true
Attachment #8508467 - Flags: feedback?(dhylands)
Attachment #8508566 - Flags: feedback?(dhylands)
Hi Dave,
could you take a look about this bug?
We can reproduce the symptom by formatting sdcard.
(The problem of cannot format internal storage still exist on Flame KK base image v188) 

Eden have a patch which will check the volume size before adding storage back into MtpServer.
If the volume size is 0, we should not add this storage into MtpServer.
There are re-try mechanism in this patch as well.

We would like to get your feedback.
Thanks.
Flags: needinfo?(dhylands)
Remove "regression" keyword per comment 8.
Keywords: regression
Comment on attachment 8508566 [details] [diff] [review]
Bug1074590: adding_storage_access_checking_before_sharing_through_MTP_v2

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

OK - I was able to reproduce the issue. Nothing concrete for a solution yet.

I think that MozMtpStorage is the wrong place to implement this.

We definitely should NOT be using sleep commands (I'd give this r-/f- just for the presence of the sleep command). We should instead post a deferred task.

I think that the correct place for this code to go is into the AutoMounter/Volume classes.

In particular, we should detect STATE_MOUNTED here:
http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/Volume.cpp#411

and rather than calling SetState with STATE_MOUNTED, we should introduce a new state called something like STATE_CHECK_MOUNTED (assign it a larger number like 100, 0 to 8 come from vold).

The AutoMounter should then detect STATE_CHECK_MOUNTED (in this loop: http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/AutoMounter.cpp#901) and if the state is STATE_CHECK_MOUNTED then it should do whatever checks are needed (so call statvfs like you're doing).

If the AutoMounter detects that a volume is mounted,then it should change the state to STATE_MOUNTED, and fall through to the existing logic. If the AutoMounter detects that the volume isn't yet ready, then it should leave the volume in the STATE_CHECK_MOUNTED state and post a deferred action to check again in an appropriate delay time.

This can be done similar to this: http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/AutoMounter.cpp#991-994 (except that we would use a delay of 1000 instead of 5000). I would actually try smaller delays, like 100 to see how long it really takes, and then tune from there. My feeling is that 1 second is probably too large. I would expect something like 250 or 500 may be more realistic.

By having the AutoMounter do the checking, then all clients of the volumes get the benefit of the checks, and we don't have to duplicate the checks in all of the client code (MozMtp is a client in this scenario). The meida apps are other clients.
Attachment #8508566 - Flags: feedback?(dhylands) → feedback-
So when I said nothing concrete for a solution, I meant to say that I don't understand why the delay is required. I think that your idea of checking statvfs is best solution that we have today, until such time as we understand why a mounted volume isn't really ready yet.

I also noticed that the mount flags change from 0x1001 to 0x1006

0x1001 = MS_BIND | MS_RDONLY
0x1006 = MS_BIND | MS_NODEV | MS_NOSUID

So obviously there is some deferred processing happening.
Flags: needinfo?(dhylands)
Hello Dave

I attached a patch according to your suggestion in comment 15. 
A new state "nsIVolume::STATE_CHECKMNT (100)" is created for volume accessibility checking. 

In Volume::HandleVoldResponse(), once the volume receives changing state to "Mounted" form Vold, we set it as "nsIVolume::STATE_CHECKMNT". This will involve AutoMounter::UpdateState(), and we checking the volume accessibility by statvfs() in its flow. If the volume can be accessed, we set it as "nsIVolume::STATE_MOUNTED" by calling Volume::SetState(), otherwise, we post a 250ms delay task of AutoMounter::UpdateState() to check the volume state again.
Attachment #8508566 - Attachment is obsolete: true
Attachment #8512451 - Flags: review?(dhylands)
Comment on attachment 8512480 [details] [diff] [review]
Bug1074590 adding_volume_accessibility_checking_before_set_it_as_Mounted_v4

remove the unnecessary space in the patch.

Hello Dave

Need we add a counter to avoid calling the volume accessibility checking infinitely?
Attachment #8512480 - Attachment description: Bug1074590 → Bug1074590 adding_volume_accessibility_checking_before_set_it_as_Mounted_v4
Attachment #8512480 - Flags: review?(dhylands)
Comment on attachment 8512451 [details] [diff] [review]
Bug1074590: adding_storage_accessibility_checking_before_setting_it_as_Mounted_v3

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

Needs lots of polish, and a few other improvements.

::: dom/system/gonk/AutoMounter.cpp
@@ +919,5 @@
>          // sharing -> mounted), and mIsSharing never gets set back to false.
>          // So we clear mIsSharing here.
>          vol->SetIsSharing(false);
>        }
> +    } else if (vol->State() == nsIVolume::STATE_CHECKMNT) {

I don't think that this should be an else.

I also think that this check should go after the MediaPresent() check

@@ +920,5 @@
>          // So we clear mIsSharing here.
>          vol->SetIsSharing(false);
>        }
> +    } else if (vol->State() == nsIVolume::STATE_CHECKMNT) {
> +      // Bug1074590, to make sure the mounted volume is accessible before we 

nit: trailing space

@@ +923,5 @@
> +    } else if (vol->State() == nsIVolume::STATE_CHECKMNT) {
> +      // Bug1074590, to make sure the mounted volume is accessible before we 
> +      // mark it as "Mounted", we use statvfs() to check the volume's status.
> +      // If the volume status is invalid, posting a delay task of this method
> +      // to check it again later. 

nit: trailing space

@@ +925,5 @@
> +      // mark it as "Mounted", we use statvfs() to check the volume's status.
> +      // If the volume status is invalid, posting a delay task of this method
> +      // to check it again later. 
> +      LOG("UpdateState: Volume %s is %s and %s", vol->NameStr(), vol->StateStr(),
> +          vol->MediaPresent() ? "inserted" : "missing");

If you move the the STATE_CHECKMNT logic, the this log won't be needed.

@@ +927,5 @@
> +      // to check it again later. 
> +      LOG("UpdateState: Volume %s is %s and %s", vol->NameStr(), vol->StateStr(),
> +          vol->MediaPresent() ? "inserted" : "missing");
> +      struct statvfs vfsbuf;
> +      if (-1 == statvfs(vol->MountPoint().get(), &vfsbuf)) {

The call to statvfs can return -1 with errno = EINTR, which means that it was interrupted and we should try again.

There is a macro MOZ_TEMP_FAILURE_RETRY which does this automatically.
http://dxr.mozilla.org/mozilla-central/source/xpcom/glue/FileUtils.h#173-179

@@ +930,5 @@
> +      struct statvfs vfsbuf;
> +      if (-1 == statvfs(vol->MountPoint().get(), &vfsbuf)) {
> +        ERR("fail to get '%s' file system information", vol->NameStr());
> +      }
> +      if (0 == vfsbuf.f_blocks) {

this is invalid test if the call statvfs failed above. (i.e. if statvfs fails, then you're testing some random value left on the stack).

@@ +936,5 @@
> +        int delay = 250;
> +        MessageLoopForIO::current()->
> +          PostDelayedTask(FROM_HERE,
> +                          NewRunnableMethod(this, &AutoMounter::UpdateState),
> +                          delay);

You should put a continue statment here.

::: dom/system/gonk/Volume.cpp
@@ +250,5 @@
>       case nsIVolume::STATE_IDLE:
>         break;
> +
> +     case nsIVolume::STATE_CHECKMNT:
> +       break;

Let's just make STATE_IDLE and STATE_CHECKMNT fall through:

    case nsIVolume::STATE_IDLE: // Fall through
    case nsIVolume::STATE_CHECKMNT: // Fall through
    default:
      break;

@@ +412,5 @@
>          if (token.EqualsLiteral("to")) {
>            nsresult errCode;
>            token = aTokenizer.nextToken();
> +          STATE newState = (STATE)(token.ToInteger(&errCode));
> +          if (nsIVolume::STATE_MOUNTED == newState) {

not: Personally I hate this style of comparison. Just use

if (newState == nsIVolume::STATE_MOUNTED)

If you accidentally do:

if (newState = nsIVolume::STATE_MOUNTED)

then the compiler complains.

@@ +415,5 @@
> +          STATE newState = (STATE)(token.ToInteger(&errCode));
> +          if (nsIVolume::STATE_MOUNTED == newState) {
> +	    // We set the volume as "STATE_CHECKMNT" here, and checking the its
> +	    // accessibility in AutoMounter. Once the volume can be accessed, 
> +	    // AutoMounter set the volume as "STATE_MOUNTED".

Let's reword this entire comment block to be:

// We set the state to STATE_CHECKMNT here, and the once the
// AutoMounter detects that the volume is actually accessible
// then the AutoMounter will change the state to STATE_MOUNTED.

::: dom/system/gonk/nsIVolume.idl
@@ +18,5 @@
>    const long STATE_UNMOUNTING  = 5;
>    const long STATE_FORMATTING  = 6;
>    const long STATE_SHARED      = 7;
>    const long STATE_SHAREDMNT   = 8;
> +  const long STATE_CHECKMNT    = 100;

Since we're changing the idl, we need to also change the uuid as well.

This change should be split out from the rest, since it needs to be reviewed by a DOM peer, otherwise your checkin will be rejected.
Attachment #8512451 - Flags: review?(dhylands)
Comment on attachment 8512480 [details] [diff] [review]
Bug1074590 adding_volume_accessibility_checking_before_set_it_as_Mounted_v4

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

This seems to be the same patch as the other one.

I'm not sure I would use a count. I think I would increase the delay, perhaps doubling it each time, until it gets to 4 seconds (250 -> 500 -> 1000 -> 2000 -> 4000) and then if it still isn't ready don't bother retrying any more.
Attachment #8512480 - Flags: review?(dhylands)
Hello Dave

I don't know who is the Dom peer can review the change on idl.
Could you help me forward this review to the proper one? Thanks.
Attachment #8512451 - Attachment is obsolete: true
Attachment #8512480 - Attachment is obsolete: true
Attachment #8514185 - Flags: review?(dhylands)
Hello Dave

This patch is including the suggestion in comment 20 and 21. 
Please help to review it. Thanks.
Attachment #8514188 - Flags: review?(dhylands)
Comment on attachment 8514185 [details] [diff] [review]
Bug1074590: Adding a new state STATE_CHECKMNT in nsIVolume.idl for volume accessibility checking

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

Redirecting to a DOM peer
Attachment #8514185 - Flags: review?(dhylands) → review?(bzbarsky)
Comment on attachment 8514188 [details] [diff] [review]
Bug1074590: adding_storage_accessibility_checking_before_setting_it_as_Mounted_v5

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

This is starting to look good.

All of my remaining comments are pretty minor.

::: dom/system/gonk/AutoMounter.cpp
@@ +937,5 @@
> +      struct statvfs vfsbuf;
> +      int result = MOZ_TEMP_FAILURE_RETRY(statvfs(vol->MountPoint().get(), &vfsbuf));
> +      if (result == -1) {
> +        // statvfs() failed. Keeping the volume being STEAT_CHECKMNT, since most
> +        // of failed cases cannot be recovered. Detailed in statvfs() error code

I'd reword this whole comment block as:

// statvfs() failed. Stay in STATE_CHECKMNT. Any failures here
// are probably non-recoverable, so we need to wait until
// something else changes the state back to IDLE/UNMOUNTED, etc.

@@ +938,5 @@
> +      int result = MOZ_TEMP_FAILURE_RETRY(statvfs(vol->MountPoint().get(), &vfsbuf));
> +      if (result == -1) {
> +        // statvfs() failed. Keeping the volume being STEAT_CHECKMNT, since most
> +        // of failed cases cannot be recovered. Detailed in statvfs() error code
> +        ERR("fail to get %s file system information", vol->NameStr());

nit: Print errno() (which then at least gives us some indication of why it failed).
nit: Reword error message to be:

ERR("statvfs failed for '%s': errno = %d", vol->NameStr(), errno());

@@ +943,5 @@
> +        continue;
> +      }
> +      static int delay = 250;
> +      if (vfsbuf.f_blocks == 0) {
> +        LOG("UpdateState: Volume %s is inaccessible", vol->NameStr());

Let's change this to:

LOG("UpdateState: Volume '%s' is inaccessible, checking again in %d msec", vol->NameStr(), delay);

and put it inside the if (delay <= 4000)

and then put an else that says:

LOG("UpdateState: Volume '%s' is inaccessible, giving up", vol->NameStr());

@@ +949,5 @@
> +          MessageLoopForIO::current()->
> +            PostDelayedTask(FROM_HERE,
> +                            NewRunnableMethod(this, &AutoMounter::UpdateState),
> +                            delay);
> +          delay << 1;

This is a no-op (same thing as if you did: delay + 1;

You probably meant delay <<= 1, but please use delay *= 2; instead.

It's more obvious what you're doing and the compiler will optimize it into delay <<=1

::: dom/system/gonk/Volume.cpp
@@ +411,5 @@
> +          STATE newState = (STATE)(token.ToInteger(&errCode));
> +          if (newState == nsIVolume::STATE_MOUNTED) {
> +            // We set the state to STATE_CHECKMNT here, and the once the
> +            // AutoMounter detects that the volume is actually accessible
> +            // then the AutoMounter set the volume as STATE_MOUNTED.

nit: s/AutoMounter set/AutoMounter will set/
Attachment #8514188 - Flags: review?(dhylands)
Hello Dave 

The attached patch is including the suggestion in comment 25.
Please help to review it. 

While statvfs fail I print the error message as following

ERR("statvfs failed for '%s': errno = %d (%s)", vol->NameStr(), errno, strerror(errno));

By printing strerror(errno), it is much easier to realize the fail reason. 

Thanks
Attachment #8514188 - Attachment is obsolete: true
Attachment #8514746 - Flags: review?(dhylands)
Comment on attachment 8514746 [details] [diff] [review]
Bug1074590: adding_storage_accessibility_checking_before_setting_it_as_Mounted_v6

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

Excellent.
Attachment #8514746 - Flags: review?(dhylands) → review+
Comment on attachment 8514185 [details] [diff] [review]
Bug1074590: Adding a new state STATE_CHECKMNT in nsIVolume.idl for volume accessibility checking

This doesn't need DOM peer review.  It just needs review from someone who knows something about this header, which is not me.
Attachment #8514185 - Flags: review?(bzbarsky) → review?(dhylands)
Comment on attachment 8514185 [details] [diff] [review]
Bug1074590: Adding a new state STATE_CHECKMNT in nsIVolume.idl for volume accessibility checking

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

Ahh - ok it's webidl that needs a dom peer. My bad.
Attachment #8514185 - Flags: review?(dhylands) → review+
Keywords: checkin-needed
Comment on attachment 8516510 [details] [diff] [review]
Bug1074590: adding_storage_accessibility_checking_before_setting_it_as_Mounted. r=dhylands

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): It is a bug of vold
User impact if declined: User cannot access sdcard through MTP after the sdcard is formatted by device.
Testing completed: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=d928d25d8459
Risk to taking this patch (and alternatives if risky): (Low risk) it is a correction
String or UUID changes made by this patch: none
Attachment #8516510 - Flags: approval-mozilla-b2g34?
https://hg.mozilla.org/integration/b2g-inbound/rev/b355b68ef47c

FYI, I tweaked the author name in the commit information to better fit our usual conventions. Please update your config accordingly :)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b355b68ef47c
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S8 (7Nov)
Attachment #8516510 - Flags: approval-mozilla-b2g34? → approval-mozilla-b2g34+
Verified on
[2.1]
Gaia-Rev        9658b93b412bdcc0f953d668e8c8e68318c99fb8
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/76880403db44
Build-ID        20141105161202
Version         34.0

[2.2]
Gaia-Rev        7918024c737c4570cacd784f267e28737ae05dea
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/2114ef80f6ae
Build-ID        20141105160209
Version         36.0a1
Status: RESOLVED → VERIFIED
This issue has been verified successfully on Flame 2.1
See attachment: Verify_video.3gp
Reproducing rate: 0/5

Flame2.1 build:
Gaia-Rev        afdfa629be209dd53a1b7b6d6c95eab7077ffcd9
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/dc3018cbdbe6
Build-ID        20141123001201
Version         34.0
Attached video Verify_video.3gp
You need to log in before you can comment on or make changes to this bug.