shared/js/media/remote_controls.js also use bluetooth functionality in music app. we could use bluetooth_helper to keep bluetooth api call isolated from app.

Comment on attachment 8597305 [details] [gaia] gasolin:issue-1158192-2 > mozilla-b2g:master Since remote control is used by music app, we can use bluetoothAdapter to abstract the bluetooth call between API difference. I test the patch with BT v1/v2 devices and all work fine. Please take a look and raise your consideration if this is a proper approach.

Comment on attachment 8597305 [details] [gaia] gasolin:issue-1158192-2 > mozilla-b2g:master Feedback+ with me. Since we have to support two API versions, it's better to move Bluetooth relative work out off app's controller. These version detection and interface(adapter) wrapper is done in bluetooth_helper.js already.

Comment on attachment 8597305 [details] [gaia] gasolin:issue-1158192-2 > mozilla-b2g:master Test added for both remoteControl & BluetoothHelper. test fine on devices.

This should only goes to master, correct me if I am wrong, thanks.

yes, it should only goes to master

Comment on attachment 8597305 [details] [gaia] gasolin:issue-1158192-2 > mozilla-b2g:master Besides of some nits addressed on GitHub, the patch is good for me. After patch updated, I'm happy to review it again. Thanks:)

Fred, thanks for working on this, before I actually review the patch, I saw you included one js file in index.html. To make sure we didn't slow down the startup times, would you please use Raptor[1] to test your patch then paste the before/after results here? (see the setup we used in Bug 1153985 comment 3) We do this for reason because recently we found performance regression from 2.1 to 2.2(Bug 1153985), so I hope for the new patches we don't break it again, or it's really hard to find the root causes after we landed many new patches. [1] https://developer.mozilla.org/en-US/Firefox_OS/Automated_testing/Raptor

thanks for the instruction, master 1bf37ff2a500f91249b51cb986aace60a397ccb9 Metric Mean Median Min Max StdDev p95 -------------------------------- -------- -------- -------- -------- ------ --- coldlaunch.navigationLoaded 2482.000 2482.000 2482.000 2482.000 0.000 n/a coldlaunch.navigationInteractive 2637.000 2637.000 2637.000 2637.000 0.000 n/a coldlaunch.visuallyLoaded 2767.000 2767.000 2767.000 2767.000 0.000 n/a coldlaunch.contentInteractive 2768.000 2768.000 2768.000 2768.000 0.000 n/a coldlaunch.fullyLoaded 3651.000 3651.000 3651.000 3651.000 0.000 n/a coldlaunch.pss 23.200 23.200 23.200 23.200 0.000 n/a coldlaunch.uss 17.800 17.800 17.800 17.800 0.000 n/a coldlaunch.rss 38.200 38.200 38.200 38.200 0.000 n/a master with this patch Metric Mean Median Min Max StdDev p95 -------------------------------- -------- -------- -------- -------- ------ --- coldlaunch.navigationLoaded 2360.000 2360.000 2360.000 2360.000 0.000 n/a coldlaunch.navigationInteractive 2503.000 2503.000 2503.000 2503.000 0.000 n/a coldlaunch.visuallyLoaded 2618.000 2618.000 2618.000 2618.000 0.000 n/a coldlaunch.contentInteractive 2618.000 2618.000 2618.000 2618.000 0.000 n/a coldlaunch.fullyLoaded 3525.000 3525.000 3525.000 3525.000 0.000 n/a coldlaunch.rss 38.200 38.200 38.200 38.200 0.000 n/a coldlaunch.uss 17.800 17.800 17.800 17.800 0.000 n/a coldlaunch.pss 23.200 23.200 23.200 23.200 0.000 n/a

FYR result on master

If I intercept the result correctly, the patch improve about 100ms for performance.

Possible reason for performance gain: BluetoothHelper instance try to retrieve adapter when any of its method is first called, so the initial procedure in remote control doesn't do any real operation until any related method is called. https://github.com/gasolin/gaia/blob/master/shared/js/bluetooth_helper.js

Re-run with same base commit, result: master MacBook-Air:gaia:% APP=music node tests/raptor/launch_test RUNS=30 <master ✗> [Raptor] Preparing to start testing... [Cold Launch: Music] Priming application [Cold Launch: Music] Starting run 1 [Cold Launch: Music] Run 1 complete Metric Mean Median Min Max StdDev p95 -------------------------------- -------- -------- -------- -------- ------ --- coldlaunch.navigationLoaded 2523.000 2523.000 2523.000 2523.000 0.000 n/a coldlaunch.navigationInteractive 2656.000 2656.000 2656.000 2656.000 0.000 n/a coldlaunch.visuallyLoaded 2813.000 2813.000 2813.000 2813.000 0.000 n/a coldlaunch.contentInteractive 2814.000 2814.000 2814.000 2814.000 0.000 n/a coldlaunch.fullyLoaded 3719.000 3719.000 3719.000 3719.000 0.000 n/a coldlaunch.pss 23.300 23.300 23.300 23.300 0.000 n/a coldlaunch.uss 17.800 17.800 17.800 17.800 0.000 n/a coldlaunch.rss 38.400 38.400 38.400 38.400 0.000 n/a [Cold Launch: Music] Testing complete with patch MacBook-Air:gaia:% APP=music node tests/raptor/launch_test RUNS=30 [Raptor] Preparing to start testing... [Cold Launch: Music] Priming application [Cold Launch: Music] Starting run 1 [Cold Launch: Music] Run 1 complete Metric Mean Median Min Max StdDev p95 -------------------------------- -------- -------- -------- -------- ------ --- coldlaunch.navigationLoaded 2516.000 2516.000 2516.000 2516.000 0.000 n/a coldlaunch.navigationInteractive 2655.000 2655.000 2655.000 2655.000 0.000 n/a coldlaunch.visuallyLoaded 2793.000 2793.000 2793.000 2793.000 0.000 n/a coldlaunch.contentInteractive 2793.000 2793.000 2793.000 2793.000 0.000 n/a coldlaunch.fullyLoaded 3658.000 3658.000 3658.000 3658.000 0.000 n/a coldlaunch.rss 38.600 38.600 38.600 38.600 0.000 n/a coldlaunch.pss 23.500 23.500 23.500 23.500 0.000 n/a coldlaunch.uss 17.900 17.900 17.900 17.900 0.000 n/a [Cold Launch: Music] Testing complete

Thanks Fred, the results looks good and it seems not impacting the startup times, it should be expected because the bluetooth helper does similar works that we did for bluetooth in the remote controls. However, music app is planning to lazy load the remote controls module, it means we should do it for the bluetooth helper as well, so after this landed, we need a followup to lazy load the bluetooth helper.

Comment on attachment 8597305 [details] [gaia] gasolin:issue-1158192-2 > mozilla-b2g:master Fred, thanks for working on this, overall the patch looks good to me. For the remote controls part, I have several comments but mostly they are about to follow the original coding style, please address them before landing it. For the bluetooth helper part, Ian should be more familiar with it so I will leave it to him.

Comment on attachment 8597305 [details] [gaia] gasolin:issue-1158192-2 > mozilla-b2g:master Thanks for review. I add ondisable state handler in bluetooth helper and the test shows no difference between has/without that handler. Please kindly review it again.

Comment on attachment 8597305 [details] [gaia] gasolin:issue-1158192-2 > mozilla-b2g:master The patch is working fine on callscreen, music apps while they are also using Bluetooth headset. And the pairing process is normal as before. r+ with me. Thanks.

merged https://github.com/mozilla-b2g/gaia/commit/02d667c78e9a6a627fd891614053938cf91ed3c4 thanks!