This bug is for formally switching to bluetooth APIv2. A follow-up bug will be fired later for removing bluetooth APIv1 after bluetooth APIv2 is stable.

Hi Shawn, We have went through PTS and smoke tests for bluetooth APIv2, all bugs discovered are now resolved. I would like to proceed to make APIv2 as the default one. APIv2 will first remain certified only and bluetooth1 API folder will remain in our code base for a while until APIv2 is stable. Could you provide your feedback on the switch, thanks.

Setting ni to inform other bluetooth members. Any feedback is welcome, thanks.

Comment on attachment 8614492 [details] [diff] [review] Bug 1167064: Switch to bluetooth APIv2. Review of attachment 8614492 [details] [diff] [review]: ----------------------------------------------------------------- Do we really need to add an extra flag in configure.in, since v2 is the default api? Or maybe you thought we will remove v1 api so it's not necessary to change the flag now?

Comment on attachment 8614492 [details] [diff] [review] Bug 1167064: Switch to bluetooth APIv2. Review of attachment 8614492 [details] [diff] [review]: ----------------------------------------------------------------- Just a question: do we want people to go back to v1 for testing purposes? In that case it might make sense to add a configure switch (i.e., AC_ENABLE). If we plan to remove v1 in the near term, the patch seems good to me.

Comment on attachment 8614492 [details] [diff] [review] Bug 1167064: Switch to bluetooth APIv2. Review of attachment 8614492 [details] [diff] [review]: ----------------------------------------------------------------- Sorry, I cleared the feedback flag.

Hi Eric, Bluetooth team have been working on WebBluetooth API refinement for a few months. The changes was tracked by Bug 1005848 which was marked as 'dev-doc-needed'. The refined Blutooth APIs (i.e. WebBluetooth v2 [1]) have been landed and tested for a while and we believe it's stable enough to be switched on by default. This bug provides a patch to switch Bluetooth API from v1 [2] to v2 [2]. And we plan to land this patch to m-c recently. [1] https://developer.mozilla.org/en-US/docs/Web/API/Web_Bluetooth_API [2] https://wiki.mozilla.org/B2G/Bluetooth/WebBluetooth-v2 Since BT API v2 are quite different to BT API v1, it would be helpful for developers to have updated documentation for Bluetooth v2 on MDN. How can we get help from MDN team for this new API set? Please guide us the proper way. Bluetooth team would be glad to provide any help. There is an additional question. Although we are going to switch to WebBluetooth v2 on m-c, we will still stick on WebBluetooth v1 on B2G v2.2. What would you suggest to handle the documentation of these two versions of API set. Thank you for your time. :) Regards, Jamin

Shawn, good point, I have revised the patch by substituting all v2 flag with v1 flag. Should be cleaner than my previous approach for our m-c now. Thomas, I think preserve v1 code base for a while (depends on how stable after we switch to v2) might be easier for testing purpose during this transition phase. Another reason for not removing it in this bug is I would like to take one step at a time, which might be safer. We don't have a clear timeline to remove v1 yet. But definitely ASAP as long as everything looks stable. Jamin, thanks a lot for pointing out MDN documentation! ;)

Comment on attachment 8615119 [details] [diff] [review] Bug 1167064 - Patch1: Switch to bluetooth APIv2. Review of attachment 8615119 [details] [diff] [review]: ----------------------------------------------------------------- Code makes sense and it looks good to me. Please try 'ics' version to make sure it won't break ICS (bluez).

Thanks for the review, Shawn.

try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c72ac038c34a

CLOBBER file was changed by other commits, need to rebase first. Clear the checkin needed flag for updating CLOBBER file.

* rebase

Backed out for various B2G mochitest failures. https://treeherder.mozilla.org/logviewer.html#?job_id=2113938&repo=b2g-inbound https://treeherder.mozilla.org/logviewer.html#?job_id=2113939&repo=b2g-inbound

Backout: https://hg.mozilla.org/integration/b2g-inbound/rev/a208fbe870ff

(In reply to Jocelyn Liu [:jocelyn] [:joliu] from comment #14) > try looks good: > https://treeherder.mozilla.org/#/jobs?repo=try&revision=c72ac038c34a Was there a reason no tests were run on this?

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #20) > (In reply to Jocelyn Liu [:jocelyn] [:joliu] from comment #14) > > try looks good: > > https://treeherder.mozilla.org/#/jobs?repo=try&revision=c72ac038c34a > > Was there a reason no tests were run on this? No, totally my bad, I was submitting try requests for multiple bugs and forgot to update the try syntax for this bug. I'll fix B2G mochitest failures before asking for relanding. Sorry for inconvenience, Jocelyn

Currently mochitest-3 will fail since we didn't provide 'required' members in the eventInit for BluetoothPairingEvent. [1] (There's not only BluetoothPairingEvent has this problem, we have other Bluetooth*Events also has this problem.) I try to assign values in test_all_synthetic_events.html, but it seems there are some members we cannot instantiate in the test. Like "required BluetoothPairingHandle handle"[1] in PairingEvent, we cannot instantiate a handle since we didn't define any constructors in BluetoothPairingHandle. From my understanding, possible ways to solve this problem is 1) Remove the entry in test_all_synthetic_events: Doesn't seem like an acceptable one, it seems it's mandatory to add all webidl events into this test. 2) Remove 'required' keywords and make those attributes nullable: A bit of breaking our original intention, but looks no harm. 3) Add constructors for those interfaces which are required members in our events Hi Boris, May I have your feedback on how to solve this problem? Are these three ways looks OK to you? What solution would you recommend here? Any other solutions you have in mind? Thanks, Jocelyn [1] https://dxr.mozilla.org/mozilla-central/source/dom/webidl/BluetoothPairingEvent.webidl#19

> May I have your feedback on how to solve this problem? How are actual consumers of your events expected to solve this problem? That is, how would a webpage normally go about creating a BluetoothPairingEvent?

(In reply to Not doing reviews right now from comment #23) > > May I have your feedback on how to solve this problem? > > How are actual consumers of your events expected to solve this problem? > That is, how would a webpage normally go about creating a > BluetoothPairingEvent? In our case, these events are create by gecko to report backend messages, a webpage shouldn't have needs to create these events.

Olli, what do you want done here? This is a slightly odd setup by modern event standards; generally people aim for events that pages can synthesize as needed....

Yeah, web pages in general should be able to create events. If there is some attribute which has such type that can't be constructed by JS, make it nullable, and then *EventInit shouldn't use required for the relevant property, but '?' and default value 'null'. That is what for example DragEvent does with .dataTransfer attribute.

Thanks for the input from both of you. Consider of web pages, I don't see a strong point of making those members required, especially we will need to make some members nullable if they can't be constructed by JS. I'll file another bug to remove those 'required' keyword. Thanks again, Jocelyn

Hi Blake, We're switching to bluetooth APIv2, interface tests need to be updated. Could you help to review my patch? Thanks, Jocelyn

- Combine some of lines if it won't exceed 80 chars.

Comment on attachment 8621934 [details] [diff] [review] Bug 1167064 - Patch3(v2): Update mochitests for switching to Bluetooth APIv2. Review of attachment 8621934 [details] [diff] [review]: ----------------------------------------------------------------- Hi Boris, Could you help to review my patch here? Please feel free to let me know if you're not available. Thanks, Jocelyn

Comment on attachment 8621934 [details] [diff] [review] Bug 1167064 - Patch3(v2): Update mochitests for switching to Bluetooth APIv2. r=me

Thanks a lot for your time, Boris!

I would like to ask for check-in again since two failed mochitests mentioned in Comment 18 has been fixed in Bug 1174071 and Patch3 in this bug. try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dd5ec7d51ff3 rc5 failed on Android 4.0 API11+ opt happens both before and after my try push, so I think it's not caused by my patch. Not sure why Gij 10 failed on B2G Desktop Linux x64 opt in my try push above, but after I pushed to try again without modifying my patch, the result is green, so I think my patch didn't break the test. my second try of Gij tests on B2G Desktop Linux x64 opt: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a58e67cbd8d6. This bug is depends on Bug 1174071, which is also checkin-needed, please help to check in this bug after Bug 1174071 to make sure b2g-inbound_ubuntu64_vm-b2g-emulator_test-mochitest-3 will not fail. Thanks, Jocelyn

https://hg.mozilla.org/integration/b2g-inbound/rev/0832ffdd8144 https://hg.mozilla.org/integration/b2g-inbound/rev/7e91b61455e9 https://hg.mozilla.org/integration/b2g-inbound/rev/43a68ed0bc6a

backout: https://hg.mozilla.org/integration/b2g-inbound/rev/60b45960828f backout: https://hg.mozilla.org/integration/b2g-inbound/rev/b99cb6564624 backout: https://hg.mozilla.org/integration/b2g-inbound/rev/0a22b1cf009e

../../dist/include/nsTArray.h:1512:14: error: 'nsTArray_Impl<E, Alloc>::elem_type* nsTArray_Impl<E, Alloc>::AppendElement(Item&&) [with Item = nsAutoString&; ActualAlloc = nsTArrayFallibleAllocator; E = nsString; Alloc = nsTArrayFallibleAllocator; nsTArray_Impl<E, Alloc>::elem_type = nsString]' is protected ../../../gecko/dom/bluetooth/bluetooth2/BluetoothAdapter.cpp:1022:241: error: within this context ../../dist/include/nsTArray.h:1512:14: error: 'nsTArray_Impl<E, Alloc>::elem_type* nsTArray_Impl<E, Alloc>::AppendElement(Item&&) [with Item = nsAutoString&; ActualAlloc = nsTArrayFallibleAllocator; E = nsString; Alloc = nsTArrayFallibleAllocator; nsTArray_Impl<E, Alloc>::elem_type = nsString]' is protected ../../../gecko/dom/bluetooth/bluetooth2/BluetoothAdapter.cpp:1050:217: error: within this context make[6]: *** [BluetoothAdapter.o] Error 1 make[5]: *** [dom/bluetooth/target] Error 2

I suppose this build break is caused by Bug 968520... I'll fix that and try again...

The build error is because Bug 968520 landed after I pushed to try server last time. I provide a fix in Bug 1174717, try again with the Bug 1174717 patch... https://treeherder.mozilla.org/#/jobs?repo=try&revision=dd9aedacba2e

try looks OK..https://treeherder.mozilla.org/#/jobs?repo=try&revision=dd9aedacba2e

https://hg.mozilla.org/integration/b2g-inbound/rev/a7c7b9345df7 https://hg.mozilla.org/integration/b2g-inbound/rev/41e5dc9380c0 https://hg.mozilla.org/integration/b2g-inbound/rev/a71239680fd8

https://hg.mozilla.org/mozilla-central/rev/a7c7b9345df7 https://hg.mozilla.org/mozilla-central/rev/41e5dc9380c0 https://hg.mozilla.org/mozilla-central/rev/a71239680fd8