[Gaia::Bluetooth] Remove workaround of using deviceStorage in bluetooth transfer after Bug 811615 is fixed

RESOLVED FIXED in B2G C3 (12dec-1jan)

Status

defect
P3
normal
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: dkuo, Assigned: iliu)

Tracking

unspecified
B2G C3 (12dec-1jan)
x86_64
Linux
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:koi+, b2g18+ fixed, b2g-v1.2 fixed)

Details

Attachments

(2 attachments)

No description provided.
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.
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: --- → ?
Triage: BB+, P3 - to remove unnecessary code
blocking-basecamp: ? → +
Priority: -- → P3
Target Milestone: --- → B2G C3 (12dec-1jan)
I'm happy to review this when you have a patch ready.
Ian what is the performance impact to the user of leaving this code in? Slow transfer? Slow initialization time, etc?
Given the lack of concrete user impact noted here ("may wastes time and memory"), we'll track for v1.x
blocking-basecamp: + → -
tracking-b2g18: --- → +
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 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+
Eric,

It doesn't work for me. What gecko version did you test?
I was using the latest mozilla-b2g18
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.
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.
Forgot to mention, I used Samsung GS2.
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.
(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.
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
koi+ as it blocks koi+ bug
blocking-b2g: --- → koi+
blocking-basecamp: - → ---
Depends on: 913294
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)
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+
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: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.