Closed
Bug 821186
Opened 10 years ago
Closed 10 years ago
[Gaia::Bluetooth] Remove workaround of using deviceStorage in bluetooth transfer after Bug 811615 is fixed
Categories
(Firefox OS Graveyard :: Gaia::Bluetooth, defect, P3)
Tracking
(blocking-b2g:koi+, b2g18+ fixed, b2g-v1.2 fixed)
People
(Reporter: dkuo, Assigned: iliu)
References
Details
Attachments
(2 files)
No description provided.
Reporter | ||
Comment 1•10 years ago
|
||
Since bug 811615 is fixed, we can pass File by web activities normally, it will not turn into Blob again. So as title, bluetooth transfer should remove the workaround of using deviceStorage, now we can directly receive the File instead of retrieving the File again from deviceStorage. This will effect bluetooth transfer in System app and Bluetooth app.
Assignee | ||
Comment 2•10 years ago
|
||
We should remove the useless workaround of using deviceStorage to query the file again. It may wastes time and memory. Could we let the issue be bb+?
blocking-basecamp: --- → ?
Comment 3•10 years ago
|
||
Triage: BB+, P3 - to remove unnecessary code
blocking-basecamp: ? → +
Priority: -- → P3
Target Milestone: --- → B2G C3 (12dec-1jan)
Comment 4•10 years ago
|
||
I'm happy to review this when you have a patch ready.
Comment 5•10 years ago
|
||
Ian what is the performance impact to the user of leaving this code in? Slow transfer? Slow initialization time, etc?
Comment 6•10 years ago
|
||
Given the lack of concrete user impact noted here ("may wastes time and memory"), we'll track for v1.x
blocking-basecamp: + → -
tracking-b2g18:
--- → +
Assignee | ||
Comment 7•10 years ago
|
||
Hi Eric, I try to remove the workaround. Then, we can get the filename information from the blob. It works on sending file from unagi to SS galaxy tab. But it don't work on sending file from unagi to unagi. I provide the patch with debug mode on. We could need to investigate the problem together.
Attachment #707978 -
Flags: feedback?(echou)
Comment 8•10 years ago
|
||
Comment on attachment 707978 [details]
Remove workaround of using deviceStorage in bluetooth sending file
For the part of passing a blob directly into function sendFile(), looks good. I've cherry-picked your patch and tested, seems to work fine.
Attachment #707978 -
Flags: feedback?(echou) → feedback+
Assignee | ||
Comment 9•10 years ago
|
||
Eric, It doesn't work for me. What gecko version did you test?
Comment 10•10 years ago
|
||
I was using the latest mozilla-b2g18
Assignee | ||
Comment 11•10 years ago
|
||
I just test the latest mozilla-b2g18 by version 2013-02-21-23-02-03. The sending process is able to run on defaultAdapter.sendFile(). But the file could not be sent. There are two different status for following test case: Unagi --> Unagi Two of the Unagi receive message with "File could not be sent" immediately. Unagi --> GalaxyTab GT-P6800 Sometimes Galaxy won't receive transferring file request. Sometimes it works normally.
Comment 12•10 years ago
|
||
The nightly build 2013-02-23-23-02-04 of b2g-18-unagi works for me. Moreover, just tried latest b2g-18/gecko with latest v1-train/gaia and it worked well, too.
Comment 13•10 years ago
|
||
Forgot to mention, I used Samsung GS2.
Assignee | ||
Comment 14•10 years ago
|
||
Leave me experiment.. I find out a strange experiment to fail the test case. Unagi --> GalaxyTab GT-P6800 ** Plug-in power for GalaxyTab ** After we send out a file from Unagi to GalaxyTab, GalaxyTab will turn on the screen without any message about the transferring request. Unlock the lock screen for GalaxyTab, there is no any prompt or status icon for out-coming transferring request. But we unplug the power from GalaxyTab, it will show a prompt and status icon for out-coming transferring request. We can following these message to receive transferring request successfully. I'm not sure that it's a SS' issue. I also use Nexus 7 and Nexus S to try the reproduces steps.(Nexus 7 --> GalaxyTab, Nexus S --> GalaxyTab) They work normally...(so sad~~) We should keep to track the strange issue.
Comment 15•10 years ago
|
||
(In reply to Ian Liu [:ianliu] from comment #14) > Leave me experiment.. > I find out a strange experiment to fail the test case. > > Unagi --> GalaxyTab GT-P6800 > ** Plug-in power for GalaxyTab ** > > After we send out a file from Unagi to GalaxyTab, GalaxyTab will turn on the > screen without any message about the transferring request. Unlock the lock > screen for GalaxyTab, there is no any prompt or status icon for out-coming > transferring request. > > But we unplug the power from GalaxyTab, it will show a prompt and status > icon for out-coming transferring request. We can following these message to > receive transferring request successfully. I'm not sure that it's a SS' > issue. > > I also use Nexus 7 and Nexus S to try the reproduces steps.(Nexus 7 --> > GalaxyTab, Nexus S --> GalaxyTab) They work normally...(so sad~~) > We should keep to track the strange issue. Just investigated more about this. When we apply Ian's patch, the class that target blob belongs to would change to class nsDOMMultipartFile, which differs from the original one, nsDOMFileFile. So, the scenario is: 1. Cheery-pick Ian's patch to current Gaia/v1-train 2. In BluetoothOppManager.cpp, we try to get an nsIInputStream instance from GetInternalStream(). 3. nsDOMMultipartFile::GetInternalStream() would be called. In the for-loop of function GetInternalStream(), a blob instance is retrieved from mBlobs and its GetInternalStream() would then be called. 4. The GetInternalStream() of class RemoteBlob in dom/ipc/Blob.cpp would be called. The beginning of the function looks like: ActorType* actor = RemoteBlobBase<ActorFlavor>::Actor(); if (!actor) { return NS_ERROR_UNEXPECTED; } Unfortunately, the actor has no value so NS_ERROR_UNEXPECTED is returned. That's why the operation would fail.
Assignee | ||
Comment 16•10 years ago
|
||
Just verify the issue is fixed or not without the workaround. The WIP patch is updated. Build version: Gaia::master, Gecko::mozilla-central, ID: 20130826190748 Noted the log as following: ======================================================================================== E/GeckoConsole( 1821): Content JS LOG at app://bluetooth.gaiamobile.org/js/transfer.js:280 in bt_connSuccess/<: filepath = /sdcard/DCIM/100MZLLA/IMG_0001.jpg E/GeckoConsole( 1821): Content JS LOG at app://bluetooth.gaiamobile.org/js/transfer.js:294 in bt_connSuccess/<: blob.name = /sdcard/DCIM/100MZLLA/IMG_0001.jpg E/GeckoConsole( 1821): Content JS LOG at app://bluetooth.gaiamobile.org/js/transfer.js:295 in bt_connSuccess/<: blob.size = 335330 E/GeckoConsole( 1821): Content JS LOG at app://bluetooth.gaiamobile.org/js/transfer.js:296 in bt_connSuccess/<: blob = [object File] I/nsVolumeMountLock( 1678): nsVolumeMountLock created for 'sdcard' I/nsVolume( 1678): nsVolume: sdcard state Mounted @ '/mnt/sdcard' gen 1 locked 1 fake 0 media 1 sharing 0 I/AutoMounter( 1678): UpdateState: umsAvail:1 umsEnabled:1 mode:0 usbCablePluggedIn:1 tryToShare:0 I/AutoMounter( 1678): UpdateState: Volume sdcard is Mounted and inserted @ /mnt/sdcard gen 1 locked 1 sharing y I/AutoMounter( 1678): UpdateState: umsAvail:1 umsEnabled:1 mode:0 usbCablePluggedIn:1 tryToShare:0 I/AutoMounter( 1678): UpdateState: Volume sdcard is Mounted and inserted @ /mnt/sdcard gen 1 locked 1 sharing y I/nsVolumeMountLock( 1678): nsVolumeMountLock acquired for 'sdcard' gen 1 E/GeckoConsole( 1821): Content JS LOG at app://bluetooth.gaiamobile.org/js/transfer.js:284 in bt_connSuccess/</getRequest.onsuccess: result = [object File] ======================================================================================== There is no any error in Gaia side. It will need Gecko Bluetooth for investigating the issue. And it might be other issue about passing blob via activity. https://bugzilla.mozilla.org/show_bug.cgi?id=896176
Comment 17•10 years ago
|
||
koi+ as it blocks koi+ bug
blocking-b2g: --- → koi+
blocking-basecamp: - → ---
Assignee | ||
Comment 18•10 years ago
|
||
Since bug 908432 is landed, passing blob via bluetooth is working for me. My test case: * photo, music, video * single/multiple file(s). Dominic, Could you please help to review the patch? Thanks.
Attachment #800633 -
Flags: review?(dkuo)
Reporter | ||
Comment 19•10 years ago
|
||
Comment on attachment 800633 [details]
Pointer to Github pull request 11983.html
Ian, this a trivial patch that simply remove the workaround, so r+ of course, and please also address the minor issue I commented on github before you land this, thanks.
Attachment #800633 -
Flags: review?(dkuo) → review+
Assignee | ||
Comment 20•10 years ago
|
||
Dominic, I have added and concerned to the empty array issue. Thanks for your reviewing effort. Since the patch is updated and landed, we're able to close the issue now. gaia/master: a3397464f8c73e650018e88cd0ae8e1b16a9ceb3
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
status-b2g18:
--- → fixed
Updated•10 years ago
|
status-b2g-v1.2:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•