813817 - [BLUETOOTH] Cancelling file transferring doesn't clear the notifications

Status: VERIFIED FIXED

(Core :: DOM: Device Interfaces, defect, P1)

Description

[Walter Chen[:ypwalter][:wachen]]

Attachment: screenshot

*Environment:
Phone - Unagi
gaia master(git): e212eeeb490b559ad9214faf71870d9c8b8c2ebd
gecko aurora(hg): 114289:8e00ce1788c7

*How to reproduce:
  1. Turn on bluetooth
  2. Pair with a computer (I am testing in Ubuntu 12.04)
  3. Send a file from PC to device
  4. Before the file transferring completed, cancel it.
  5. Redo 3 & 4 again (same file)

*Expected Result:
  There should be 2 notifications on file transferring failure. Also, 2 file transferring progress bar should be cleared from notification center.

*Actual Result:
  there is 2 file transferring progress bar with only 1 notification of file transferring failed.

*Sidenote:
  Even clear all in notification center doesn't clear everything.

Comment 1

[David Scravaglieri [:scravag]]

Does this bug is on Notification or on File transfert

Comment 2

[Walter Chen[:ypwalter][:wachen]]

It's on the notification.
As shown in the screen, those two file transferring are stopped after they got cancelled. 
But: 
  1. The progress bar freezed there. And, it can't be cleared.
     (I didn't take remember if it will continue to wait for files or not.)
  2. The failure notification only came out for one time (should have 2 alerts)

Comment 3

[Evelyn Hung]

I believe Ian can help on this. re-assign this issue to him

Comment 4

[Walter Chen[:ypwalter][:wachen]]

Note:

   Please cancel the file transferring in Laptop/PC (not in phone)

Comment 5

[[e-mail], [e-mail]]

(In reply to Walter Chen from comment #4)
> Note:
> 
>    Please cancel the file transferring in Laptop/PC (not in phone)

Walter,
Thanks for your help to verify the reproduced steps.

Eric,
I don't receive the "bluetooth-opp-transfer-complete" message in this case.
I think that platform need to handle this case.
So, I re-assigned the issue to you.

Comment 6

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

This should be a known issue of Gecko Bluetooth. Gina will take a look on it.

Comment 7

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

Attachment: Patch 1(v1): Cancelling file transferring doesn't clear the notifications

Comment 8

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

Attachment: Patch 1(v2): Cancelling file transferring doesn't clear the notifications

Comment 9

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

Comment on attachment 685499 [details] [diff] [review]
Patch 1(v2): Cancelling file transferring doesn't clear the notifications

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

r=me with nits addressed and questions answered. Thanks for making BluetoothOppManager more decent. :)

::: dom/bluetooth/BluetoothOppManager.cpp
@@ +355,2 @@
>    if (!aConfirm) {
> +    success = false;

Nit(?): let /success/ be initialized to false at the initialization so that we can check "if (aConfirm)" only

@@ +1091,5 @@
> +    if (!mSuccessFlag && mMode == Mode::RECEIVE) {
> +      DeleteReceivedFile();
> +    }
> +    FileTransferComplete();
> +    Listen();

Question: Is it possible that "OnDisconnect() is invoked while mSocketStatus is not SOCKET_CONNECTED but we still want to call Listen() again"? To be more specific, do we need to handle the condition such as "mSocketStatus == SocketConnectionStatus::SOCKET_CONNECTING"?

::: dom/bluetooth/BluetoothOppManager.h
@@ +79,5 @@
>    virtual void OnConnectSuccess() MOZ_OVERRIDE;
>    virtual void OnConnectError() MOZ_OVERRIDE;
>    virtual void OnDisconnect() MOZ_OVERRIDE;
>  
> +  enum Mode

I think maybe we could have another name to make it more clear. How about 'TransferMode' or 'TransferDirection'? :)

@@ +83,5 @@
> +  enum Mode
> +  {
> +    SEND = 0,
> +    RECEIVE = 1
> +  } mMode;

It seems that we would only have two /modes/ for file sharing. If so, can we just use a boolean value? Then we don't have to cast mMode to boolean every time while sending system message.

Comment 10

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

Attachment: Patch 1: Cancelling file transferring doesn't clear the notifications, r=echou

Final patch with nit picked. Also, error handling in onDisconnect() is added for socket status of "listening".

try:
https://tbpl.mozilla.org/?tree=Try&rev=c33f177e7401
https://tbpl.mozilla.org/?tree=Try&rev=cdb011e2ff3f

Comment 11

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/17683784d36b

Comment 12

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

dom/bluetooth, does not seems like a Gaia bug. Changing the component.

Comment 13

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/17683784d36b

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/87e8c81bcec4
https://hg.mozilla.org/releases/mozilla-beta/rev/46c6945f4c63

Comment 15

[Walter Chen[:ypwalter][:wachen]]

verified in 20121212 build http://releases.mozilla.com/b2g/