1006316 - Modify WebAPI for BT on/off

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Ben Tian (inactive)]

Modify dom/webidl/BluetoothAdapter.webidl based on bug 1006308 comment 0. This bug should request a super review for WebAPI change.

Comment 1

[Jocelyn Liu [:jocelyn] [:joliu]]

Attachment: Bug 1006316 - Part1: Change WebAPI of BluetoothAdapter

Comment 2

[Jocelyn Liu [:jocelyn] [:joliu]]

Attachment: Bug 1006316 - Part2: Modify current BluetoothAdapter to support WebAPI change

Hi Ben,

I have done two patches for the BluetoothAdapter WebAPI change.
Please let me know if you have any feedbacks.

Thanks,
Jocelyn

Comment 3

[Jocelyn Liu [:jocelyn] [:joliu]]

Attachment: Patch1: Revise BluetoothAdapter2.webidl for new WebBluetooth API

Revise BluetoothAdapter2.webidl:
    * BluetoothAdapterState enum
    * BluetoothAdapterAttribute enum
    * Modify BluetoothAdapter interface for refined WebBluetooth API

Comment 4

[Jocelyn Liu [:jocelyn] [:joliu]]

Attachment: Patch2: Modify bluetooth2/BluetoothAdapter to support WebAPI change

Revise bluetooth2/BluetoothAdapter based on patch1.

Comment 5

[Ben Tian (inactive)]

Comment on attachment 8428484 [details] [diff] [review]
Patch1: Revise BluetoothAdapter2.webidl for new WebBluetooth API

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

Please remove |discoverableTimeout| related attribute and function since they are not used in new WebBluetooth API.

::: dom/webidl/BluetoothAdapter2.webidl
@@ +54,5 @@
>  interface BluetoothAdapter : EventTarget {
> +  readonly attribute BluetoothAdapterState  state;
> +  readonly attribute DOMString              address;
> +  readonly attribute unsigned long          class;
> +  readonly attribute boolean                discovering;

nit: Put |discovering| after |discoverable| as on wiki: https://wiki.mozilla.org/B2G/Bluetooth/WebBluetooth-v2/BluetoothAdapter#BluetoothAdapter

@@ +57,5 @@
> +  readonly attribute unsigned long          class;
> +  readonly attribute boolean                discovering;
> +  readonly attribute DOMString              name;
> +  readonly attribute boolean                discoverable;
> +  readonly attribute unsigned long          discoverableTimeout; // in seconds

Remove |discoverableTimeout| and |setDiscoverableTimeout| if no gaia application require them.

@@ +92,5 @@
>    DOMRequest setName(DOMString name);
>    [NewObject, Throws]
>    DOMRequest setDiscoverable(boolean discoverable);
>    [NewObject, Throws]
>    DOMRequest setDiscoverableTimeout(unsigned long timeout);

Ditto.

Comment 6

[Jocelyn Liu [:jocelyn] [:joliu]]

Attachment: Patch1(v2): Revise BluetoothAdapter2.webidl for new WebBluetooth API

Hi Ben,

Thanks for the feedback, I have updated the patch according to the wiki page.
Please see the updated patch for webidl.
They will also be removed in implementation level later by Bug 1006308.

Thanks,
Jocelyn

Comment 7

[Ben Tian (inactive)]

Comment on attachment 8428560 [details] [diff] [review]
Patch1(v2): Revise BluetoothAdapter2.webidl for new WebBluetooth API

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

f=me. Please request :bz review IDL change.

Comment 8

[Ben Tian (inactive)]

Comment on attachment 8428486 [details] [diff] [review]
Patch2: Modify bluetooth2/BluetoothAdapter to support WebAPI change

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

r=me with comment addressed. Thanks.

::: dom/bluetooth2/BluetoothAdapter.cpp
@@ +670,5 @@
>  }
>  
> +/*
> +  TODO: Implement Enable/Disable functions
> +*/

nit: format comment as following.

/**
 * TODO: Implement Enable/Disable functions
 */

::: dom/bluetooth2/BluetoothAdapter.h
@@ +187,5 @@
>      PairUnpair(bool aPair, const nsAString& aDeviceAddress, ErrorResult& aRv);
>  
>    JS::Heap<JSObject*> mJsUuids;
>    JS::Heap<JSObject*> mJsDeviceAddresses;
> +  BluetoothAdapterState mState;

Assign initial value of mState in constructor.

Comment 9

[Jocelyn Liu [:jocelyn] [:joliu]]

Comment on attachment 8428560 [details] [diff] [review]
Patch1(v2): Revise BluetoothAdapter2.webidl for new WebBluetooth API

Hi Boris,

  I had revised BluetoothAdapter2.webidl for our new WebBluetooth API.
  (spec: https://wiki.mozilla.org/B2G/Bluetooth/WebBluetooth-v2/BluetoothAdapter#BluetoothAdapter)
  Would you mind to review this webidl change?

Thanks,
Jocelyn

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8428560 [details] [diff] [review]
Patch1(v2): Revise BluetoothAdapter2.webidl for new WebBluetooth API

r=me

Comment 11

[Jocelyn Liu [:jocelyn] [:joliu]]

Attachment: [Final] Patch1: Revise BluetoothAdapter2.webidl for new WebBluetooth API, f=btian, r=bz

Replace [Func="Navigator::HasBluetoothSupport"] with [CheckPermissions="bluetooth"], see bug 952486.

Comment 12

[Jocelyn Liu [:jocelyn] [:joliu]]

Attachment: [Final] Patch2: Modify bluetooth2/BluetoothAdapter to support WebAPI change, r=btian

NIT addressed.

Comment 13

[Ben Tian (inactive)]

https://hg.mozilla.org/integration/b2g-inbound/rev/4e836dee7dc6
https://hg.mozilla.org/integration/b2g-inbound/rev/411f2bef5560

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4e836dee7dc6
https://hg.mozilla.org/mozilla-central/rev/411f2bef5560