superreview requested: [Bug 830213] [Bluetooth][Certification]HFP PTS TC_AG_ATH_BV_04_I failed due to there is no API for disconnect SCO connection : [Attachment 747266] Patch 1(v1): Add ConnectSco, DisconnectSco, IsScoConnected in nsIDOMBluetoothAdapter

[bugzill...@mozilla.org]

Gina Yeh [:gyeh] [:ginayeh] <gy...@mozilla.com> has asked Blake Kaplan (:mrbkap)
<mrb...@gmail.com> for superreview:
Bug 830213: [Bluetooth][Certification]HFP PTS TC_AG_ATH_BV_04_I failed due to
there is no API for disconnect SCO connection
https://bugzilla.mozilla.org/show_bug.cgi?id=830213

Attachment 747266: Patch 1(v1): Add ConnectSco, DisconnectSco, IsScoConnected
in nsIDOMBluetoothAdapter
https://bugzilla.mozilla.org/attachment.cgi?id=747266&action=edit


------- Additional Comments from Gina Yeh [:gyeh] [:ginayeh] <gy...@mozilla.com>
Three interfaces are added in this patch.

[bugzill...@mozilla.org]

Gina Yeh [:gyeh] [:ginayeh] <gy...@mozilla.com> has asked Blake Kaplan (:mrbkap)
<mrb...@gmail.com> for superreview:
Bug 830213: [Bluetooth][Certification]HFP PTS TC_AG_ATH_BV_04_I failed due to
there is no API for disconnect SCO connection
https://bugzilla.mozilla.org/show_bug.cgi?id=830213

Attachment 747262: Patch 2(v1): Implementation in BluetoothHfpManager
https://bugzilla.mozilla.org/attachment.cgi?id=747262&action=edit

[bugzill...@mozilla.org]

Gina Yeh [:gyeh] [:ginayeh] <gy...@mozilla.com> has asked Blake Kaplan (:mrbkap)
<mrb...@gmail.com> for superreview:
Bug 830213: [Bluetooth][Certification]HFP PTS TC_AG_ATH_BV_04_I failed due to
there is no API for disconnect SCO connection
https://bugzilla.mozilla.org/show_bug.cgi?id=830213

Attachment 747377: Patch 2(v2): Implementation in BluetoothHfpManager
https://bugzilla.mozilla.org/attachment.cgi?id=747377&action=edit


------- Additional Comments from Gina Yeh [:gyeh] [:ginayeh] <gy...@mozilla.com>
Update patch as comments from echou.

[bugzill...@mozilla.org]

Gina Yeh [:gyeh] [:ginayeh] <gy...@mozilla.com> has canceled Gina Yeh [:gyeh]
[:ginayeh] <gy...@mozilla.com>'s request for superreview:

Bug 830213: [Bluetooth][Certification]HFP PTS TC_AG_ATH_BV_04_I failed due to
there is no API for disconnect SCO connection
https://bugzilla.mozilla.org/show_bug.cgi?id=830213

Attachment 747262: Patch 2(v1): Implementation in BluetoothHfpManager
https://bugzilla.mozilla.org/attachment.cgi?id=747262&action=edit

[bugzill...@mozilla.org]

Blake Kaplan (:mrbkap) <mrb...@gmail.com> has granted Gina Yeh [:gyeh]

[:ginayeh] <gy...@mozilla.com>'s request for superreview:
Bug 830213: [Bluetooth][Certification]HFP PTS TC_AG_ATH_BV_04_I failed due to
there is no API for disconnect SCO connection
https://bugzilla.mozilla.org/show_bug.cgi?id=830213

Attachment 747266: Patch 1(v1): Add ConnectSco, DisconnectSco, IsScoConnected
in nsIDOMBluetoothAdapter
https://bugzilla.mozilla.org/attachment.cgi?id=747266&action=edit

------- Additional Comments from Blake Kaplan (:mrbkap) <mrb...@gmail.com>
Review of attachment 747266:
-----------------------------------------------------------------

::: dom/bluetooth/linux/BluetoothDBusService.cpp
@@ +2867,5 @@
> + return;
> + }
> +
> + NS_NAMED_LITERAL_STRING(replyError,
> + "SCO socket doesn't exist or HFP is not connected");

Super-nit: there's an extra space between "not" and "connected".

@@ +2879,5 @@
> +
> + BluetoothHfpManager* hfp = BluetoothHfpManager::Get();
> + NS_ENSURE_TRUE_VOID(hfp);
> + DispatchBluetoothReply(aRunnable,
> + hfp->IsScoConnected(), NS_LITERAL_STRING(""));

Use EmptyString() instead of NS_LITERAL_STRING("") here.

[bugzill...@mozilla.org]

Blake Kaplan (:mrbkap) <mrb...@gmail.com> has granted Gina Yeh [:gyeh]
[:ginayeh] <gy...@mozilla.com>'s request for superreview:
Bug 830213: [Bluetooth][Certification]HFP PTS TC_AG_ATH_BV_04_I failed due to
there is no API for disconnect SCO connection
https://bugzilla.mozilla.org/show_bug.cgi?id=830213

Attachment 747377: Patch 2(v2): Implementation in BluetoothHfpManager
https://bugzilla.mozilla.org/attachment.cgi?id=747377&action=edit

------- Additional Comments from Blake Kaplan (:mrbkap) <mrb...@gmail.com>

Review of attachment 747377:
-----------------------------------------------------------------

::: dom/bluetooth/BluetoothHfpManager.cpp
@@ +1490,5 @@
> + // For active connection request, we need to reply the DOMRequest
> + if (mScoRunnable) {
> + DispatchBluetoothReply(mScoRunnable,
> + BluetoothValue(true), NS_LITERAL_STRING(""));
> + mScoRunnable.forget();

This should be: mScoRunnable = nullptr; otherwise it'll leak.

@@ +1505,5 @@
> + if (mScoRunnable) {
> + NS_NAMED_LITERAL_STRING(replyError, "Failed to create SCO socket!");
> + DispatchBluetoothReply(mScoRunnable, BluetoothValue(), replyError);
> +
> + mScoRunnable.forget();

This also leaks.

@@ +1516,5 @@
> +BluetoothHfpManager::OnScoDisconnect()
> +{
> + if (mScoSocketStatus == SocketConnectionStatus::SOCKET_CONNECTED) {
> + ListenSco();
> + NotifyAudioManager(NS_LITERAL_STRING(""));

All of the places where you use NS_LITERAL_STRING("") should be changed to use
EmptyString().

@@ +1568,5 @@
> + BluetoothService* bs = BluetoothService::Get();
> + NS_ENSURE_TRUE(bs, false);
> + nsresult rv = bs->GetScoSocket(mDeviceAddress, true, false, mScoSocket);
> +
> + return NS_FAILED(rv) ? false : true;

Please change this to: return NS_SUCCEEDED(rv).