Should dispatch a bluetoothreply to reject promise for BluetoothAdapter::Enable() and BluetoothAdapter::Disable if StartStopGonkBluetooth failed.

* Fix error handling bug for bt on/off.

Comment on attachment 8447836 [details] [diff] [review] Bug 1031253 - Patch(v1): Reject promise if StartStopGonkBluetooth failed. Review of attachment 8447836 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/bluetooth2/BluetoothService.cpp @@ +380,5 @@ > * even if Bluetooth is already enabled. > * > * Please see bug 892392 for more information. > */ > if (aIsStartup || !sBluetoothService->IsEnabled()) { Remove redundant |sBluetoothService->|. @@ +382,5 @@ > * Please see bug 892392 for more information. > */ > if (aIsStartup || !sBluetoothService->IsEnabled()) { > // Switch Bluetooth on > if (NS_FAILED(sBluetoothService->StartInternal(aRunnable))) { Ditto. @@ +384,5 @@ > if (aIsStartup || !sBluetoothService->IsEnabled()) { > // Switch Bluetooth on > if (NS_FAILED(sBluetoothService->StartInternal(aRunnable))) { > BT_WARNING("Bluetooth service failed to start!"); > + return NS_ERROR_FAILURE; We should return the return value of |StartInternal(aRunnable))| instead of NS_ERROR_FAILURE. @@ +441,5 @@ > * even if Bluetooth is disabled. > * > * Please see bug 892392 for more information. > */ > if (aIsStartup || sBluetoothService->IsEnabled()) { Ditto. @@ +443,5 @@ > * Please see bug 892392 for more information. > */ > if (aIsStartup || sBluetoothService->IsEnabled()) { > // Switch Bluetooth off > if (NS_FAILED(sBluetoothService->StopInternal(aRunnable))) { Ditto. @@ +445,5 @@ > if (aIsStartup || sBluetoothService->IsEnabled()) { > // Switch Bluetooth off > if (NS_FAILED(sBluetoothService->StopInternal(aRunnable))) { > BT_WARNING("Bluetooth service failed to stop!"); > + return NS_ERROR_FAILURE; Ditto. ::: dom/bluetooth2/bluedroid/BluetoothServiceBluedroid.cpp @@ +933,5 @@ > } > + > + // Reject Promise > + if(aRunnable) { > + DispatchBluetoothReply(aRunnable, BluetoothValue(), Error strings suffice for BluetoothReplyRunnable w/ DOMRequest but w/ promise we need to pass corresponding nsresult error. Please file a followup bug to make BluetoothReplyRunnable reply with nsresult errors. @@ +935,5 @@ > + // Reject Promise > + if(aRunnable) { > + DispatchBluetoothReply(aRunnable, BluetoothValue(), > + NS_LITERAL_STRING(ERR_START_BLUETOOTH)); > + sChangeAdapterStateRunnableArray.RemoveElementAt(0); sChangeAdapterStateRunnableArray.RemoveElement(aRunnable); @@ +967,5 @@ > + // Reject Promise > + if(aRunnable) { > + DispatchBluetoothReply(aRunnable, BluetoothValue(), > + NS_LITERAL_STRING(ERR_STOP_BLUETOOTH)); > + sChangeAdapterStateRunnableArray.RemoveElementAt(0); Ditto.

* Address review comments * Follow up bug of nsresult error is Bug 1032755

Comment on attachment 8448630 [details] [diff] [review] Bug 1031253 - Patch(v2): Reject promise if StartStopGonkBluetooth failed. Review of attachment 8448630 [details] [diff] [review]: ----------------------------------------------------------------- r=me. Thanks!

Thanks for your time, Ben.