Closed Bug 1074593 Opened 10 years ago Closed 10 years ago

[MTP][KK] PC could not sync files which were created from device when the folder for storing files does not exist

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(firefox35 wontfix, firefox36 fixed, b2g-v2.1 verified, b2g-v2.1S verified, b2g-v2.2 verified)

VERIFIED FIXED
2.1 S7 (24Oct)
Tracking Status
firefox35 --- wontfix
firefox36 --- fixed
b2g-v2.1 --- verified
b2g-v2.1S --- verified
b2g-v2.2 --- verified

People

(Reporter: ashiue, Assigned: alchen)

References

Details

(Whiteboard: [ft:Peripherals])

Attachments

(2 files, 4 obsolete files)

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. Set current default media location as internal storage
3. Delete screenshot folder on internal storage from PC
4. Device take a screenshot

Expect result:
1. PC could see the screenshot

Actual result:
1. PC could not see the screenshot unless reconnect to the device
[Blocking Requested - why for this release]:
functional issue
blocking-b2g: --- → 2.1?
QA Whiteboard: [COM=Storage]
Alphan, please take a look or mentor Eden to work on this.
Flags: needinfo?(alchen)
Adding qawanted for branch checks.
Keywords: qawanted
No problem. I will check this bug first.
Flags: needinfo?(alchen)
Whiteboard: [ft:Peripherals]
Triage: not blocking
Assignee: nobody → alchen
blocking-b2g: 2.1? → ---
Hi Dave,
for this bug, we need to notify all new added files (include folder and file) to mtp server. MtpServer will notify initiator(ex:PC) by "sendObjectAdded".
This behavior is the same as Nexus 4.

In my testing,
The mtp driver on Ubuntu didn't care about "MtpServer: sendObjectAdded $FILEHANDLE".
However, the mtp on windows will use several commands to get latest status after receiving "MtpServer: sendObjectAdded $FILEHANDLE".

[On Windows]
Without this patch, the file explorer of windows can only react file create immediately.
However, if we delete "screenshot" folder first and do a screenshot, the screenshot won't appear in the end.
With this patch, the file explorer of windows will pop "screenshot" folder immediately. (PS: Bug 1074600 is another known issue for new added file)

[On Ubuntu]
There is no change with or without this patch.
If there is new added file, file explorer of Ubuntu won't appear this file(or folder) immediately. However, we can refresh the status after re-entering this folder.

For example,
1. we delete "screenshot" folder in /sdcard.
2. Do a screenshot.
3. Go to another folder and get back to /sdcard again.
4. screenshot will appear.
Attachment #8499467 - Flags: feedback?(dhylands)
Update the checking.


Example:
1. delete "DCIM" folder in sdcard1
2. Take a picture

(LOG)
10-03 18:16:43.590  6782  9211 I MozMtp  : FileWatcherUpdate: file /storage/sdcard1/DCIM/100MZLLA/IMG_0031.jpg created
10-03 18:16:43.590  6782  9211 I MozMtp  : FileWatcherUpdate: entryHandle = 0
10-03 18:16:43.590  6782  9209 D MozMtp  : AddEntry: Handle: 0x00000010 Parent: 0xffffffff Path:'/storage/sdcard1/DCIM'
10-03 18:16:43.590  6782  9209 V MtpServer: sendObjectAdded 16
10-03 18:16:43.590  6782  9209 V MtpServer: mEvent.write returned 0
10-03 18:16:43.590  6782  9209 D MozMtp  : AddEntry: Handle: 0x00000011 Parent: 0x00000010 Path:'/storage/sdcard1/DCIM/100MZLLA'
10-03 18:16:43.590  6782  9209 V MtpServer: sendObjectAdded 17
10-03 18:16:43.590  6782  9209 V MtpServer: mEvent.write returned 0
10-03 18:16:43.590  6782  9209 D MozMtp  : AddEntry: Handle: 0x00000012 Parent: 0x00000011 Path:'/storage/sdcard1/DCIM/100MZLLA/IMG_0031.jpg'
10-03 18:16:43.590  6782  9209 I MozMtp  : FileWatcherUpdate: About to call sendObjectAdded Handle 0x00000012 file /storage/sdcard1/DCIM/100MZLLA/IMG_0031.jpg
10-03 18:16:43.590  6782  9209 V MtpServer: sendObjectAdded 18
10-03 18:16:43.590  6782  9209 V MtpServer: mEvent.write returned 0
Attachment #8499467 - Attachment is obsolete: true
Attachment #8499467 - Flags: feedback?(dhylands)
Attachment #8499480 - Flags: feedback?(dhylands)
Hi Dave,
in this patch I add two functions (AddEntryAndNotify, RemoveEntryAndNotify)  for file/folder change when MTP in use.

They will do AddEntry(or RemoveEntry) and tell mtp server about this change.
Attachment #8499480 - Attachment is obsolete: true
Attachment #8499480 - Flags: feedback?(dhylands)
Attachment #8500298 - Flags: feedback?(dhylands)
Comment on attachment 8500298 [details] [diff] [review]
(1006) Bug 1074593 - Add AddEntryAndNotify and RemoveEntryAndNotify for file/folder change when MTP in use

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

Excellent - good catch.

Just a few minor nits to cleanup.

::: dom/system/gonk/MozMtpDatabase.cpp
@@ +112,5 @@
>            entry->mHandle, entry->mParent, entry->mPath.get());
>  }
>  
>  void
> +MozMtpDatabase::AddEntryAndNotify(RefCountedMtpServer* aMtpServer, DbEntry *entry)

nit: I would swap the arguments. We're adding the dbEntry and notifying the mtpServer, so the arguments should be in that order.

@@ +177,5 @@
>    }
>  }
>  
> +void
> +MozMtpDatabase::RemoveEntryAndNotify(RefCountedMtpServer* aMtpServer, MtpObjectHandle aHandle)

nit: swap arguments here as well.

@@ +289,5 @@
>        return;
>      }
> +
> +    // Create entry for this file and notify MTP if there is new added entry
> +    entryHandle = CreateEntryForFile(aMtpServer, filePath, aFile);

Lets rename this as CreateEntryForFileAndNotify and put the mtp server argument at the end.
Attachment #8500298 - Flags: feedback?(dhylands) → feedback+
QA Whiteboard: [COM=Storage] → [COM=MTP/UMS]
Component: General → MTP/UMS
Hi Dave,
Here is the patch included the suggestion in comment 9.
Thanks.
Attachment #8500298 - Attachment is obsolete: true
Attachment #8502338 - Flags: review?(dhylands)
Comment on attachment 8502338 [details] [diff] [review]
(1009) Bug 1074593 - Add AddEntryAndNotify and RemoveEntryAndNotify for file/folder change when MTP in use

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

Thanks
Attachment #8502338 - Flags: review?(dhylands) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1ec76d3b14be
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Blocks: 1084180
QA Whiteboard: [COM=MTP/UMS]
This issue is verified fixed on Flame 2.2.

Result: The "screenshot" folder is created immediately on the PC (Window).

Device: Flame 2.2 (319mb, KK, Full Flash)
BuildID: 20141107040206
Gaia: 779f05fead3d009f6e7fe713ad0fea16b6f2fb31
Gecko: 64f4392d0bdc
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 36.0a1 (2.2 Master)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0
==================================================

This issue reproduces on Flame 2.1.

Result: The "screenshot" folder is not created. The device has to be reconnected to see the newly taken screenshot.

Device: Flame 2.1 (319mb, KK, Shallow Flash)
BuildID: 20141107001205
Gaia: 6295f6acfe91c6ae659712747dd2b9c8f51d0339
Gecko: 8c23b4f2ba29
Version: 34.0 (2.1) 
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Leaving verifyme for 2.1 uplift.
===============================================

MTP feature does not exist on Flame 2.0.

Device: Flame 2.0 (319mb, KK, Shallow Flash)
BuildID: 20141107000206
Gaia: d3e4da377ee448f9c25f908159480e867dfb13f3
Gecko: 9836e9d81357 
Version: 32.0 (2.0)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Keywords: qawantedverifyme
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
This bug has been successfully verified on latest Flame v2.1.
See attachment: verified_v2.1.mp4
Reproduce rate: 0/5

STR:
1. Plug in USB cable and enable MTP on device.
2. Set current default media location as internal storage
3. Delete screenshot folder on internal storage from PC
4. Device take a screenshot and don't plug out USB.
5. Go to PC and see the screenshot folder.
**PC can see the screenshot folder after refreshing.

Flame 2.1 build:
Build ID               20150205001711
Gaia Revision          17bf14f12e43043654498330d610d469b8b55e64
Gaia Date              2015-02-03 05:19:41
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/bdebcc47ec7a
Gecko Version          34.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150205.035050
Firmware Date          Thu Feb  5 03:51:01 EST 2015
Bootloader             L1TC000118D0
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][MGSEI-Triage+]
This bug has been successfully verified on latest v2.1S.
Reproduce rate: 0/5

2.1S (256m) build:
Build ID               20150210002201
Gaia Revision          7dd130a312f12c89b2d41948f8557effa56afbf6
Gaia Date              2015-02-10 06:09:14
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/5017c32553c7
Gecko Version          34.0
Device Name            scx15_sp7715ga
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150210.040821
Firmware Date          Tue Feb 10 04:08:33 EST 2015

2.1S (512m) build:
Build ID               20150210161202
Gaia Revision          7dd130a312f12c89b2d41948f8557effa56afbf6
Gaia Date              2015-02-10 06:09:14
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/df6f80e36912
Gecko Version          34.0
Device Name            scx15_sp7715ea
Firmware(Release)      4.4.2
Firmware(Incremental)  122
Firmware Date          Thu Feb  5 12:42:58 CST 2015

Note:
On 2.1S (256m):
I have verified this bug successfully on SD storage because the device's internal storage can't be recognized by PC.
You need to log in before you can comment on or make changes to this bug.