[Bluetooth2] DispatchReplyError() should reply |BluetoothStatus| instead of |BluetoothValue(nsString)| in LE scan result handle.

RESOLVED FIXED in Firefox 41

Status

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jaliu, Assigned: jaliu)

Tracking

unspecified
2.2 S14 (12june)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

3 years ago
DispatchReplyError() shouldn't take |BluetoothValue| as its parameter.
It should only replay |nsAString| or |BluetoothStatus|.
(Assignee)

Updated

3 years ago
Assignee: nobody → jaliu
(Assignee)

Comment 1

3 years ago
Created attachment 8613405 [details] [diff] [review]
Fix the parameter type of DispatchReplyError() in startLeScan result handler. (v1)
Attachment #8613405 - Flags: review?(joliu)
(Assignee)

Comment 2

3 years ago
Thank Bruce Sun for helping me found this bug.
Comment on attachment 8613405 [details] [diff] [review]
Fix the parameter type of DispatchReplyError() in startLeScan result handler. (v1)

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

::: dom/bluetooth/bluedroid/BluetoothGattManager.cpp
@@ +521,5 @@
>          new BluetoothVoidReplyRunnable(nullptr);
>        gattManager->UnregisterClient(mClient->mClientIf, result);
>      }
>  
> +    DispatchReplyError(mClient->mStartLeScanRunnable, mClient->mAppUuid);

Hi Jamin,

This error string isn't descriptive.
I would suggest to reject with a error status here.

Thanks,
Jocelyn
Attachment #8613405 - Flags: review?(joliu)
(Assignee)

Updated

3 years ago
Summary: [Bluetooth2] DispatchReplyError() should reply |nsString| instead of |BluetoothValue(nsString)| in LE scan result handle. → [Bluetooth2] DispatchReplyError() should reply |BluetoothStatus| instead of |BluetoothValue(nsString)| in LE scan result handle.
(Assignee)

Comment 4

3 years ago
Created attachment 8613474 [details] [diff] [review]
Fix the parameter type of DispatchReplyError() in startLeScan result handler. (v2)

- revise previous patch based on #comment 3
Attachment #8613405 - Attachment is obsolete: true
Attachment #8613474 - Flags: review?(joliu)
Comment on attachment 8613474 [details] [diff] [review]
Fix the parameter type of DispatchReplyError() in startLeScan result handler. (v2)

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

r=me, thanks!
Attachment #8613474 - Flags: review?(joliu) → review+
(Assignee)

Comment 6

3 years ago
Created attachment 8613876 [details] [diff] [review]
Fix the parameter type of DispatchReplyError() in startLeScan result handler. (v2.1), r=joliu

- add reviewer's name to commit message
- convert to hg format.

Thank Jocelyn for reviewing the patch.
Attachment #8613474 - Attachment is obsolete: true
(Assignee)

Comment 7

3 years ago
Skip the "Pushing to Try" procedure since Bluetooth API v2 hasn't be covered by tinderbox.
https://hg.mozilla.org/mozilla-central/rev/69bc92321873
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S14 (12june)
You need to log in before you can comment on or make changes to this bug.