[Dialer] Dialer may call CDMA-specific bluetooth functions on non-CDMA phone

RESOLVED FIXED in Firefox OS v1.3

Status

Firefox OS
Gaia::Dialer
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jaliu, Assigned: Mukesh kumar)

Tracking

unspecified
1.3 C3/1.4 S3(31jan)
All
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:1.3+, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

358 bytes, text/html
etienne
: review+
Details
(Reporter)

Description

4 years ago
BluetoothHfpManager::AnswerWaitingCall() is a CDMA-specific function.
holdAndAnswer() in calls_handler.js may call btHelper.answerWaitingCall(); in non-CDMA phone and trigger the assertion "MOZ_ASSERT(mPhoneType == PhoneType::CDMA)" in BluetoothHfpManager::AnswerWaitingCall().
The bug may crashes B2G when user hold a phone call on non-CDMA phone even the BT is unconnected to any devices.
(Reporter)

Comment 1

4 years ago
Correction:
The bug cause the assertion failed but it wouldn't crash B2G in release build.
Severity: major → normal
(Assignee)

Comment 2

4 years ago
Hi Etienne,

As per my understanding,it looks to be a small fix in Calls_handler.js
I can work on this issue.
Please assign to me in case no one is working.
Flags: needinfo?(etienne)

Comment 3

4 years ago
Mukesh,

Just click 'take' in 'Assign To:' field to take this bug, and ask for Etienne's review once you've done the fix. Thanks.
Flags: needinfo?(etienne)

Comment 4

4 years ago
assigning to Mukesh as per his request
Assignee: nobody → mukeshk1990
(In reply to Mukesh kumar from comment #2)
> Hi Etienne,
> 
> As per my understanding,it looks to be a small fix in Calls_handler.js
> I can work on this issue.
> Please assign to me in case no one is working.

Awesome :)
(Assignee)

Comment 6

4 years ago
Created attachment 8364865 [details]
Pointer to Pull Request.html

Hi,

Please check if this pull request can fix the issue.
https://github.com/mozilla-b2g/gaia/pull/15668

Thank you.
Attachment #8364865 - Flags: review?(etienne)
Comment on attachment 8364865 [details]
Pointer to Pull Request.html

The code change looks good, but we need to add a unit test.

You can find some inspiration on how to test the |holdAndAnswer()| function here [1], how to test the bluetooth interactions there [2], and for the CDMA case you can look here [3].

Cheers!

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/test/unit/calls_handler_test.js#L540-544
[2] https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/test/unit/calls_handler_test.js#L640-645
[3] https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/test/unit/calls_handler_test.js#L265-274
Attachment #8364865 - Flags: review?(etienne)
(Assignee)

Comment 8

4 years ago
Created attachment 8365972 [details]
Pointer to Pull Request.html

Hi Etienne,

Thanks for your help.
Updated pull request with test case added.

Thank you.
Attachment #8364865 - Attachment is obsolete: true
Attachment #8365972 - Flags: review?(etienne)
Comment on attachment 8365972 [details]
Pointer to Pull Request.html

All good, thank!
Attachment #8365972 - Flags: review?(etienne) → review+
(Assignee)

Updated

4 years ago
Whiteboard: 894457
(Assignee)

Updated

4 years ago
Whiteboard: 894457
(Assignee)

Comment 10

4 years ago
Hi,

 Please merge the patch to master.
 The patch is reviewed and good to merge.
Just restarted the failing travis job, will land as soon as it's green.
https://github.com/mozilla-b2g/gaia/commit/b58937b36da25e627d65a6aad38ecabaca6f3ffe
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED

Updated

4 years ago
Blocks: 979733

Comment 13

4 years ago
Request 1.3? per bug 979733 comment 5.

Updated

4 years ago
No longer blocks: 979733
Duplicate of this bug: 979733
Cert blocker. 1.3+.
blocking-b2g: --- → 1.3+
Please request approval-gaia-v1.3 on this patch when this is ready for uplift.

Comment 17

4 years ago
Mukesh/Etienne, can you help uplift the gaia fix to 1.3?

This bug becomes a 1.3 blocker since it fails BT certification test (bug 979733).
Flags: needinfo?(mukeshk1990)
Flags: needinfo?(etienne)
(Assignee)

Comment 18

4 years ago
 I dont have the permission to uplift this to v1.3 train.

 Etienne, Can you help uplifting this to 1.3?

Thanks!
Flags: needinfo?(mukeshk1990)
https://github.com/mozilla-b2g/gaia/commit/e1da401e7f86ed9b32b12ca8d3fd05f88341148d
status-b2g-v1.3: --- → fixed
Flags: needinfo?(etienne)
status-b2g-v1.3T: --- → fixed
status-b2g-v1.4: --- → fixed
Target Milestone: --- → 1.3 C3/1.4 S3(31jan)
You need to log in before you can comment on or make changes to this bug.