Closed
Bug 1089534
Opened 10 years ago
Closed 10 years ago
[B2G][Telephony] should GetCallFromEverywhere in Telephony::NotifyError
Categories
(Firefox OS Graveyard :: RIL, defect)
Tracking
(blocking-b2g:2.0M+, b2g-v2.0 affected, b2g-v2.0M fixed, b2g-v2.1 affected, b2g-v2.2 fixed)
People
(Reporter: hsinyi, Assigned: hsinyi)
References
Details
Attachments
(2 files)
(deleted),
patch
|
aknow
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
aknow
:
review+
|
Details | Diff | Splinter Review |
Consider a scenario: a call in a conference is released with error. Then, [1] is reached and an exception is thrown because that call isn't in mCalls but in mGroup. We need to fix it by calling |nsRefPtr<TelephonyCall> callToNotify = GetCallFromEverywhere(aServiceId, aCallIndex);|
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → htsai
Assignee | ||
Comment 1•10 years ago
|
||
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #0)
> Consider a scenario: a call in a conference is released with error. Then,
> [1] is reached and an exception is thrown because that call isn't in mCalls
> but in mGroup. We need to fix it by calling |nsRefPtr<TelephonyCall>
> callToNotify = GetCallFromEverywhere(aServiceId, aCallIndex);|
[1] http://dxr.mozilla.org/mozilla-central/source/dom/telephony/Telephony.cpp?from=Telephony.cpp#637
Assignee | ||
Comment 2•10 years ago
|
||
Assignee | ||
Comment 3•10 years ago
|
||
Comment on attachment 8511863 [details] [diff] [review]
WIP
Review of attachment 8511863 [details] [diff] [review]:
-----------------------------------------------------------------
Hi Germán ,
Mind giving a try ?
Attachment #8511863 -
Flags: feedback?(gtorodelvalle)
Comment 4•10 years ago
|
||
I looove it!!! :) At least I am starting to see the end of the tunnel :p
OK, good news is that I am getting the same results as Johan did last Friday as mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=977056#c76 You can see the logs here: https://pastebin.mozilla.org/6943662 , since Johan's pastebin expired :(
As mentioned in that comment, we are getting an event notification which was not expected according to our specs: https://bug977056.bugzilla.mozilla.org/attachment.cgi?id=8510217 It is the last one, from line 47 on in https://pastebin.mozilla.org/6943662 Is it right? Just to add it to our specs or to file a bug in case it is not right :)
Comment 5•10 years ago
|
||
BTW, I didn't want to set the feedback to + or - until we clarify this last event notification issue :)
Flags: needinfo?(htsai)
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Germán Toro del Valle from comment #4)
> I looove it!!! :) At least I am starting to see the end of the tunnel :p
>
> OK, good news is that I am getting the same results as Johan did last Friday
> as mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=977056#c76 You
> can see the logs here: https://pastebin.mozilla.org/6943662 , since Johan's
> pastebin expired :(
>
> As mentioned in that comment, we are getting an event notification which was
> not expected according to our specs:
> https://bug977056.bugzilla.mozilla.org/attachment.cgi?id=8510217 It is the
> last one, from line 47 on in https://pastebin.mozilla.org/6943662 Is it
> right? Just to add it to our specs or to file a bug in case it is not right
> :)
The last additional event TelephonyCallGroup.onstatechange is expected because in this case, telephonyCallGroup state is changing from 'connected' to 'unknown' (no calls in the group). Everything looks perfect from the API point of view :)
Flags: needinfo?(htsai)
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8511863 [details] [diff] [review]
WIP
Review of attachment 8511863 [details] [diff] [review]:
-----------------------------------------------------------------
Hi Aknow,
Mind taking a look? Thank you!
Attachment #8511863 -
Flags: review?(szchen)
Comment 9•10 years ago
|
||
Comment on attachment 8511863 [details] [diff] [review]
WIP
Review of attachment 8511863 [details] [diff] [review]:
-----------------------------------------------------------------
LGTM. Thank you.
Attachment #8511863 -
Flags: review?(szchen) → review+
Assignee | ||
Comment 10•10 years ago
|
||
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=171722783fd1 looks good in addition to nice local marionette-webapi tests.
Going to land this soon
Assignee | ||
Comment 11•10 years ago
|
||
Comment on attachment 8511863 [details] [diff] [review]
WIP
Thanks for the comment!
Attachment #8511863 -
Flags: feedback?(gtorodelvalle)
Assignee | ||
Comment 12•10 years ago
|
||
Comment 13•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 14•10 years ago
|
||
Attachment #8540518 -
Flags: review+
Comment 15•10 years ago
|
||
I suggest to land this patch on 2.0m and the risk is pretty low.
From our analysis, we suspect that it's the root cause that lead to Bug 1108966. Moreover, we also observed the exactly same syndrome in the log and that really convinced me. So, please consider the uplifting.
blocking-b2g: --- → 2.0M?
Flags: needinfo?(jocheng)
Comment 16•10 years ago
|
||
Hi Aknow,
Thanks for the heads up.
Hi Kai-Zhen,
Can you help to land this to 2.0M? Thanks!
Blocks: Woodduck
blocking-b2g: 2.0M? → 2.0M+
status-b2g-v2.0:
--- → affected
status-b2g-v2.0M:
--- → affected
status-b2g-v2.1:
--- → affected
status-b2g-v2.2:
--- → fixed
Flags: needinfo?(jocheng) → needinfo?(kli)
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → ARM
Comment 18•10 years ago
|
||
Flags: needinfo?(kli)
Comment 19•10 years ago
|
||
Does this need to land on v2.1? If so, please request b2g34 approval on the patch.
Flags: needinfo?(htsai)
Assignee | ||
Comment 20•10 years ago
|
||
(In reply to [Away 24-Dec to 4-Jan] Ryan VanderMeulen [:RyanVM UTC-5] from comment #19)
> Does this need to land on v2.1? If so, please request b2g34 approval on the
> patch.
Thanks Ryan!
Hi Josh,
Do we need this for v2.1 as well?
Flags: needinfo?(htsai) → needinfo?(jocheng)
Target Milestone: --- → 2.0 S1 (9may)
Comment 21•10 years ago
|
||
Hi Bhavana,
Per my comment at https://bugzilla.mozilla.org/show_bug.cgi?id=1102671#c13
Please help to make the decision. Thanks!
Flags: needinfo?(jocheng) → needinfo?(bbajaj)
Comment 22•10 years ago
|
||
:hsinyi, if you think this is a critical path for partners on 2.1, its fine to uplift.
Flags: needinfo?(bbajaj)
You need to log in
before you can comment on or make changes to this bug.
Description
•