819832 - [b2g-bluetooth] Sending files to Mac would fail at the end of sending process

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

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

Although sending files to laptop installed with Ubuntu 12.04/12.10 and several Android devices(HTC Sensation XL, HTC One S, Samsung Galaxy S2) worked, sharing file with Mac would fail at the end of sending process. 

This is a Gecko issue, nominated as bb+.

Comment 1

[Gina Yeh [:gyeh] [:ginayeh]]

Attachment: Patch 1(v1): Sending files to Mac would fail at the end of sending process

Send a PutFinal packet with End-of-Body header at the end of transferring.

Comment 2

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

Comment on attachment 690305 [details] [diff] [review]
Patch 1(v1): Sending files to Mac would fail at the end of sending process

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

There is a description in chapter 3 "Session Protocol" of OBEX spec 1.2, "Once a request is issued, the client waits for a response from the server before issuing another request. The request/response pair is referred to as an operation." So the patch has to be revised, otherwise we may send a [Put(f)/EndOfBody] request before receiving the response of the last [Put(c)/Body].

::: dom/bluetooth/BluetoothOppManager.cpp
@@ +909,5 @@
>    UnixSocketRawData* s = new UnixSocketRawData(index);
>    memcpy(s->mData, req, s->mSize);
>    SendSocketData(s);
>  
> +  if (aFinal) {

Here we are going to send a packet with header [End-Of-Body] without content. I suggest to write comments to describe the reason why we do so.

@@ +913,5 @@
> +  if (aFinal) {
> +    // Section 2.2.9 "End-of-Body", IrObex 1.2
> +    // [opcode:1][length:2]
> +    index = 3;
> +    index += AppendHeaderEndOfBody(&req[index]);

I think we don't have function AppendHeaderEndOfBody in ObexBase.cpp yet. Miss something?

@@ +914,5 @@
> +    // Section 2.2.9 "End-of-Body", IrObex 1.2
> +    // [opcode:1][length:2]
> +    index = 3;
> +    index += AppendHeaderEndOfBody(&req[index]);
> +    SetObexPacketInfo(req, ObexRequestCode::PutFinal, 6);

Please use variable "index" instead of magic number 6.

@@ +917,5 @@
> +    index += AppendHeaderEndOfBody(&req[index]);
> +    SetObexPacketInfo(req, ObexRequestCode::PutFinal, 6);
> +    mLastCommand = ObexRequestCode::PutFinal;
> +
> +    s = new UnixSocketRawData(6);

Please use variable "index" instead of magic number 6.

Comment 3

[Gina Yeh [:gyeh] [:ginayeh]]

Attachment: Patch 1(v2): Sending files to Mac would fail at the end of sending process

Send PutFinal packet with a zero length End-of-Body header after we get response of last Put packet from the remote.

Comment 4

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

Comment on attachment 690334 [details] [diff] [review]
Patch 1(v2): Sending files to Mac would fail at the end of sending process

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

r=me with nit addressed.

::: dom/bluetooth/BluetoothOppManager.cpp
@@ +920,5 @@
> +
> +void
> +BluetoothOppManager::SendPutFinalRequest()
> +{
> +  LOG("[O] %s", __FUNCTION__); 

I guess this should be removed? :)

::: dom/bluetooth/ObexBase.cpp
@@ +45,5 @@
> +
> +  return 3;
> +}
> +
> +

Nit: remove an additional line

Comment 5

[Gina Yeh [:gyeh] [:ginayeh]]

Attachment: Patch 1: Sending files to Mac would fail at the end of sending process, r=echou

try:
https://tbpl.mozilla.org/?tree=Try&rev=6e0ffceeb69d
https://tbpl.mozilla.org/?tree=Try&rev=6334c3354b96

Comment 6

[Gina Yeh [:gyeh] [:ginayeh]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5908a3ce5507

This patch still have conflicts on mozilla-beta and mozilla-aurora, so mark approval-mozilla-beta and approval-mozilla-aurora in Bug 802288. Please land this patch after Bug 802288. Thanks.

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/5908a3ce5507

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ec9d0613daef
https://hg.mozilla.org/releases/mozilla-b2g18/rev/664cfda52bce