Closed
Bug 846647
Opened 11 years ago
Closed 11 years ago
[b2g-bluetooth] Update call state before reseting call array
Categories
(Core :: DOM: Device Interfaces, defect)
Tracking
()
People
(Reporter: gyeh, Assigned: gyeh)
References
Details
Attachments
(3 files, 4 obsolete files)
2.38 KB,
patch
|
Details | Diff | Splinter Review | |
2.50 KB,
patch
|
Details | Diff | Splinter Review | |
1.20 KB,
patch
|
Details | Diff | Splinter Review |
STR: 1. Make a call 2. Hang up the call via bluetooth headset Program received signal SIGSEGV, Segmentation fault. 0x411c73b4 in nsTArray_Impl<mozilla::dom::bluetooth::Call, nsTArrayInfallibleAllocator>::ElementAt (this=<value optimized out>, i=<value optimized out>) at ../../dist/include/nsTArray.h:638 ---Type <return> to continue, or q <return> to quit--- 638 MOZ_ASSERT(i < Length(), "invalid array index"); When we hang up the phone call, the call array is reset/re-initialized, so we failed to update the state of call array.
Assignee | ||
Comment 1•11 years ago
|
||
Assignee: nobody → gyeh
Attachment #719838 -
Flags: review?(echou)
Assignee | ||
Comment 2•11 years ago
|
||
Since NS_ENSURE_FALSE_VOID will generate a warning message in debug build, I'd like to replace it with an if statement.
Assignee | ||
Comment 3•11 years ago
|
||
Let's update the patch! We should update call array after cacheing the previous call state.
Attachment #719838 -
Attachment is obsolete: true
Attachment #719838 -
Flags: review?(echou)
Attachment #719877 -
Flags: review?(echou)
Comment 4•11 years ago
|
||
Comment on attachment 719877 [details] [diff] [review] Patch 1(v2): Update call state before reseting call array Review of attachment 719877 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me.
Attachment #719877 -
Flags: review?(echou) → review+
Assignee | ||
Updated•11 years ago
|
Summary: [b2g-bluetooth] Assertion failed in call array length → [b2g-bluetooth] Update call state before reseting call array
Assignee | ||
Comment 5•11 years ago
|
||
Attachment #719877 -
Attachment is obsolete: true
Comment 6•11 years ago
|
||
Try run for 938dbd25ffe5 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=938dbd25ffe5 Results (out of 18 total builds): success: 17 warnings: 1 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/gyeh@mozilla.com-938dbd25ffe5
Comment 7•11 years ago
|
||
Try run for 12a7d1e71dbb is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=12a7d1e71dbb Results (out of 14 total builds): success: 14 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/gyeh@mozilla.com-12a7d1e71dbb
Comment 9•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e0bc07a40a67
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Updated•11 years ago
|
tracking-b2g18:
--- → ?
Comment 10•11 years ago
|
||
Looks like a good fix to get into v1.1, tracking.
status-b2g18:
--- → affected
Comment 11•11 years ago
|
||
This is going to need a branch-specific patch for b2g18.
Assignee | ||
Comment 12•11 years ago
|
||
This is blocked by Bug 827212, which has been marked as tracking-b2g? Can we land it after Bug 827212?
Comment 13•11 years ago
|
||
This still doesn't apply even with bug 827212 rebased onto b2g18. Looking at the m-c hg log for BluetoothHfpManager.cpp, it appears that a good number of other bugs landed in between these two. Either get approval for the other ones or post a branch-specific patch, please.
Assignee | ||
Comment 14•11 years ago
|
||
Sorry for the late reply. I've checked the implementation in b2g-18 and m-c and found that the architecture has changed a lot (as Ryan said, a good number of other bugs landed between these two). I think this issue should only be happen on m-c, so re-nominate to tracking-b2g18?
Comment 16•11 years ago
|
||
Due to bug 827212 has been landed to b2g18, I'd like to nominate this bug as tracking-b2g+ again, trying to make it checked into b2g18 branch. Otherwise it would hit an assertion if suer hanging up a phone call when a Bluetooth headset is connected.
Assignee | ||
Comment 17•11 years ago
|
||
Updated•11 years ago
|
Assignee | ||
Comment 18•11 years ago
|
||
Comment on attachment 731757 [details] [diff] [review] Final patch for b2g-18, r=echou NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): none User impact if declined: The call array is reset when user hang up all phone calls, and we still set value for invalid address of the call array. Testing completed: mozilla-central Risk to taking this patch (and alternatives if risky): Should be fairly low. The patch is to set the call array value before resetting it, and make sure we won't access invalid memory address. String or UUID changes made by this patch: none
Attachment #731757 -
Flags: approval-mozilla-b2g18?
Updated•11 years ago
|
Attachment #731757 -
Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
This patch seems still not merge into b2g18. This will block leo bluetooth certification.
blocking-b2g: --- → leo?
Sorry, after confirmation, it had been merged.
blocking-b2g: leo? → ---
blocking-b2g: --- → leo?
typo: Sorry, after confirmation, it had NOT been merged yet.
Comment 22•11 years ago
|
||
This doesn't apply to b2g18. Please post an updated patch.
Comment 23•11 years ago
|
||
(In reply to Shawn Huang from comment #19) > This patch seems still not merge into b2g18. This will block leo bluetooth > certification. This is approved for v1.1, but no justification was provided for why this would block v1.1 but not v1.0.1. Also, ni on gyeh for comment 22.
blocking-b2g: leo? → -
Flags: needinfo?(gyeh)
Assignee | ||
Comment 24•11 years ago
|
||
For v1.0.1, we just reset the call index (mCurrentCallIndex) to 0 after a phone call is disconnected without resetting the whole array (mCurrentCallStateArray). So we won't have this issue on v1.0.1.
Flags: needinfo?(gyeh)
Assignee | ||
Comment 25•11 years ago
|
||
I'll generate a new b28-specific patch later.
Assignee | ||
Comment 26•11 years ago
|
||
Updated•11 years ago
|
Attachment #731757 -
Attachment is obsolete: true
Comment 27•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g18/rev/d0c7d0c2d493
status-b2g18-v1.0.0:
--- → wontfix
status-b2g18-v1.0.1:
--- → wontfix
status-firefox21:
--- → wontfix
status-firefox22:
--- → fixed
Comment 28•11 years ago
|
||
This bug(and other 9 bugs) block Bluetooth certification. So, these bugs need to be marked as tef+ and landed to v1.0.1 in order to pass Bluetooth certification: bug 827255 bug 827212 bug 827266 bug 828175 bug 823346 bug 827230 bug 828798 bug 835740 bug 846647 bug 828160 So, mark these bugs as tef?. These fixes have some dependency and it would be better to have them landed in a specific order. Gina has a good view on this.
blocking-b2g: - → tef?
Assignee | ||
Comment 29•11 years ago
|
||
I recommend to land these patches in the following order: 01. bug827204 02. bug827255 03. bug823346 04. bug827230 05. bug828798 06. bug827212, patch 1 06. bug827212, patch 2 07. bug827266 08. bug828175 09. bug846647 10. bug825861 11. bug825851 12. bug835740 I'm going to attach patches for b2g18_v1_0_1 for each bug. Please land them in the above order. There should be no conflict and feel free to let me know if I can be any help.
Assignee | ||
Comment 30•11 years ago
|
||
Comment 31•11 years ago
|
||
tef- for now until partner confirms this is blocking BT cert.
blocking-b2g: tef? → -
Comment 32•11 years ago
|
||
Seems it is confirmed it blocks BT cer as per bug 868347
blocking-b2g: - → tef+
Comment 33•11 years ago
|
||
Gina, this doesn't apply to b2g18_v1_0_1 anymore. Can you please rebase?
Assignee | ||
Comment 34•11 years ago
|
||
v1.0.1 patch updated.
Attachment #738773 -
Attachment is obsolete: true
Assignee | ||
Comment 35•11 years ago
|
||
Ryan, I've updated all patches. Please land them in the following order, thanks. 01. bug827204 02. bug827255 (03. bug823346 has been landed on v1.0.1) 04. bug827230 05. bug828798 06. bug827212, patch 1 06. bug827212, patch 2 07. bug827266 08. bug828175 09. bug846647 10. bug825861 11. bug825851 12. bug835740
You need to log in
before you can comment on or make changes to this bug.
Description
•