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

Status: VERIFIED FIXED

Description

[Alison Shiue]

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

Comment 1

[Alison Shiue]

[Blocking Requested - why for this release]:
functional issue

Comment 2

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

Alphan, please take a look or mentor Eden to work on this.

Comment 3

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

Adding qawanted for branch checks.

Comment 4

[Alphan Chen [:alchen]]

No problem. I will check this bug first.

Comment 5

[howie [:howie]]

Triage: not blocking

Comment 6

[Alphan Chen [:alchen]]

Attachment: (1003) notify mtpserver for new added folder

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.

Comment 7

[Alphan Chen [:alchen]]

Attachment: (1003-update) notify mtpserver for new added folder

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

Comment 8

[Alphan Chen [:alchen]]

Attachment: (1006) Bug 1074593 - Add AddEntryAndNotify and RemoveEntryAndNotify for file/folder change when MTP in use

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.

Comment 9

[Dave Hylands [: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.

Comment 10

[Alphan Chen [:alchen]]

Attachment: (1009) Bug 1074593 - Add AddEntryAndNotify and RemoveEntryAndNotify for file/folder change when MTP in use

Hi Dave,
Here is the patch included the suggestion in comment 9.
Thanks.

Comment 11

[Dave Hylands [: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

Comment 12

[Alphan Chen [:alchen]]

Attachment: Add AddEntryAndNotify and RemoveEntryAndNotify for file/folder change when MTP in use. r=dhylands

Add reviewer.

Try server result is as below. They looks fine.
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=526622281969

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/b2g-inbound/rev/1ec76d3b14be

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1ec76d3b14be

Comment 15

[Yeojin Chung [:YeojinC]]

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

Comment 17

[Ryan VanderMeulen [:RyanVM]]

This was approved for uplift in bug 1107146.

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/4413baee2e08

Comment 18

[Shally Li（leave from Mozilla）]

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

Comment 19

[Shally Li（leave from Mozilla）]

Attachment: verified_v2.1.MP4

Comment 20

[Vincent Liu[:vliu]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/4413baee2e08

Comment 21

[Shally Li（leave from Mozilla）]

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.