1072576 - MTP should add storages before starting MtpServer

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Dave Hylands [:dhylands]]

When enabling MTP, we should wait for the MozMtpStorage objects to be registered before actually starting the MTP server.

Here's a logcat with debugging turned on;

> 209/ 476        AutoMounter  I   Changing mode from 'Disable' to 'Enable-MTP'
> 209/ 476        AutoMounter  I   AutoMounter state changed from UMS_CONFIGURED to MTP_CONFIGURING
> 209/ 476        AutoMounter  I   SetUsbFunction(mtp) sys.usb.config to 'mtp,adb'
> 209/ 476        AutoMounter  I   UpdateState: ums:A1C1E0 mtp:A1C0E1 mode:3 usb:1 tryToShare:0 state:MTP_CONFIGURING
> 209/ 476        AutoMounter  I   UpdateState: Volume sdcard is Mounted and inserted @ /storage/sdcard gen 1 locked 0 sharing en-n
> 209/ 476        AutoMounter  I   UpdateState: Volume sdcard1 is Mounted and inserted @ /storage/sdcard1 gen 2 locked 0 sharing en-n
> 209/ 476 AutoMounterSetting  I   Changing status from 'Disabled' to 'Enabled'
> 209/ 476        AutoMounter  I   UpdateState: ums:A1C0E0 mtp:A1C0E1 mode:3 usb:0 tryToShare:0 state:MTP_CONFIGURING
> 209/ 476        AutoMounter  I   UpdateState: Volume sdcard is Mounted and inserted @ /storage/sdcard gen 1 locked 0 sharing en-n
> 209/ 476        AutoMounter  I   UpdateState: Volume sdcard1 is Mounted and inserted @ /storage/sdcard1 gen 2 locked 0 sharing en-n
> 209/ 476        AutoMounter  I   Starting MtpServer
> 209/ 476             MozMtp  I   Run: Opened '/dev/mtp_usb' fd 131
> 209/ 476             MozMtp  I   MozMtpStorage: Storage constructed for Volume sdcard mStorageID 0x00010001
> 209/ 476             MozMtp  I   Notify: Volume sdcard mStorageID 0x00010001 state changed to Mounted SharingEnabled: 1
> 209/ 476             MozMtp  I   StorageAvailable: Adding Volume sdcard mStorageID 0x00010001 mountPoint /storage/sdcard to MozMtpDatabase
> 209/1166             MozMtp  I   Run: MozMtpServer started
> 209/1166             MozMtp  I   sessionStarted: 
> 209/1166             MozMtp  I   getSupportedPlaybackFormats: returning MTP_FORMAT_UNDEFINED, MTP_FORMAT_ASSOCIATION, MTP_FORMAT_PNG
> 209/1166             MozMtp  I   getSupportedCaptureFormats: returning MTP_FORMAT_ASSOCIATION, MTP_FORMAT_PNG
> 209/1166             MozMtp  I   getSupportedDeviceProperties: 
> 209/1166             MozMtp  I   getObjectPropertyDesc: Property: MTP_PROPERTY_OBJECT_SIZE 0x0000dc04
> 209/ 476             MozMtp  D   AddEntry: Handle: 0x00000001 Parent: 0xffffffff Path:'/storage/sdcard/LOST.DIR'
> 209/1166             MozMtp  I   getObjectPropertyDesc: Property: MTP_PROPERTY_OBJECT_SIZE 0x0000dc04
> 209/1166             MozMtp  I   getObjectPropertyDesc: Property: MTP_PROPERTY_OBJECT_SIZE 0x0000dc04
> 209/ 476             MozMtp  D   AddEntry: Handle: 0x00000002 Parent: 0xffffffff Path:'/storage/sdcard/sdcard0'
> 209/ 476             MozMtp  I   AddStorage: added 3 items from tree '/storage/sdcard'
> 209/ 476             MozMtp  I   StorageAvailable: Adding Volume sdcard mStorageID 0x00010001 mountPoint /storage/sdcard to MtpServer
> 209/ 476             MozMtp  I   Notify: Volume sdcard mStorageID 0x00010001 state changed to Mounted SharingEnabled: 1
> 209/ 476             MozMtp  I   MozMtpStorage: Storage constructed for Volume sdcard1 mStorageID 0x00020001
> 209/ 476             MozMtp  I   Notify: Volume sdcard1 mStorageID 0x00020001 state changed to Mounted SharingEnabled: 1
> 209/ 476             MozMtp  I   StorageAvailable: Adding Volume sdcard1 mStorageID 0x00020001 mountPoint /storage/sdcard1 to MozMtpDatabase
> 209/ 476             MozMtp  D   AddEntry: Handle: 0x00000003 Parent: 0xffffffff Path:'/storage/sdcard1/boot.scr'
> 209/ 476             MozMtp  D   AddEntry: Handle: 0x00000004 Parent: 0xffffffff Path:'/storage/sdcard1/boot.script'
> 209/ 476             MozMtp  D   AddEntry: Handle: 0x00000005 Parent: 0xffffffff Path:'/storage/sdcard1/MLO'
> 209/ 476             MozMtp  D   AddEntry: Handle: 0x00000006 Parent: 0xffffffff Path:'/storage/sdcard1/u-boot.bin'
> 209/ 476             MozMtp  D   AddEntry: Handle: 0x00000007 Parent: 0xffffffff Path:'/storage/sdcard1/uImage'
> 209/ 476             MozMtp  D   AddEntry: Handle: 0x00000008 Parent: 0xffffffff Path:'/storage/sdcard1/uInitrd'
> 209/ 476             MozMtp  D   AddEntry: Handle: 0x00000009 Parent: 0xffffffff Path:'/storage/sdcard1/LOST.DIR'
> 209/ 476             MozMtp  D   AddEntry: Handle: 0x0000000a Parent: 0xffffffff Path:'/storage/sdcard1/sdcard1'
> 209/ 476             MozMtp  I   AddStorage: added 11 items from tree '/storage/sdcard1'
> 209/ 476             MozMtp  I   StorageAvailable: Adding Volume sdcard1 mStorageID 0x00020001 mountPoint /storage/sdcard1 to MtpServer
> 209/ 476             MozMtp  I   Notify: Volume sdcard1 mStorageID 0x00020001 state changed to Mounted SharingEnabled: 1
> 209/ 476        AutoMounter  I   AutoMounter state changed from MTP_CONFIGURING to MTP_STARTED
> 209/ 476        AutoMounter  I   UpdateState: ums:A1C0E0 mtp:A1C1E1 mode:3 usb:1 tryToShare:0 state:MTP_STARTED
> 209/ 476        AutoMounter  I   UpdateState: Volume sdcard is Mounted and inserted @ /storage/sdcard gen 1 locked 0 sharing en-n
> 209/ 476        AutoMounter  I   UpdateState: Volume sdcard1 is Mounted and inserted @ /storage/sdcard1 gen 2 locked 0 sharing en-n

The way it is now, sometimes when I start MTP, I just see a window with "Firefox OS" but I don't see sdcard or sdcard1. Pressing F5 in Nautilus causes them to show up. This tells me that the PC is asking about the storages before we've registered them.

Comment 1

[Dave Hylands [:dhylands]]

Attachment: Register storages before running server

I noticed a bunch of warnings (related to unified build) and fixed those as well (The #undef's of LOG/ERR/DBG, etc).

Comment 2

[Dave Hylands [:dhylands]]

Attachment: Register storages before running server. v2

Comment 3

[Eric Chou [:ericchou] [:echou]]

Hey Dave,

Thanks for your patch. I'll find some time this week to review.

Comment 4

[Eric Chou [:ericchou] [:echou]]

Comment on attachment 8497214 [details] [diff] [review]
Register storages before running server. v2

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

Thanks for your patch. r+ but I have a suggestion. Please see it below.

::: dom/system/gonk/AutoMounter.cpp
@@ +611,3 @@
>    sMozMtpServer = new MozMtpServer();
> +  if (!sMozMtpServer->Init()) {
> +    return;

suggestion: for this case, I think we should change the type of return value to bool to reflect the failure of initialization. So that we could do something to recover at the point that StartMtpServer() gets called in AutoMounter::UpdateState().

Comment 5

[Dave Hylands [:dhylands]]

https://hg.mozilla.org/integration/b2g-inbound/rev/869eba5ab4a0

Comment 6

[Dave Hylands [:dhylands]]

Fix bustage in unified build:
 https://hg.mozilla.org/integration/b2g-inbound/rev/d45d5b25b63f

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/869eba5ab4a0

Comment 8

[Ryan VanderMeulen [:RyanVM]]

(In reply to Dave Hylands [:dhylands] from comment #6)
> Fix bustage in unified build:
>  https://hg.mozilla.org/integration/b2g-inbound/rev/d45d5b25b63f

Landed with the wrong bug number in the commit message.

https://hg.mozilla.org/mozilla-central/rev/d45d5b25b63f