Closed Bug 992948 Opened 6 years ago Closed 6 years ago

Use the telephony-call-ended system message to fill call log

Categories

(Firefox OS Graveyard :: Gaia::Dialer, defect)

x86
macOS
defect
Not set

Tracking

(blocking-b2g:1.3T+, firefox29 wontfix, firefox30 fixed, firefox31 fixed, b2g-v1.3 unaffected, b2g-v1.3T fixed, b2g-v1.4 fixed, b2g-v2.0 fixed)

RESOLVED FIXED
1.4 S6 (25apr)
blocking-b2g 1.3T+
Tracking Status
firefox29 --- wontfix
firefox30 --- fixed
firefox31 --- fixed
b2g-v1.3 --- unaffected
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: etienne, Assigned: etienne)

References

Details

(Whiteboard: [1.4-approval-needed])

Attachments

(3 files, 1 obsolete file)

In prep for bug 990003.
This will remove one callscreen-appwindow dependency.
1.3T? because it's blocking a 1.3t+
Assignee: nobody → etienne
Blocks: 990003
blocking-b2g: --- → 1.3T?
Attached file Gaia Part (PR)
Hey, sorry for the emergency review (blocking a big 1.3t), but we're gaining in coverage :)
Attachment #8402757 - Flags: review?(anthony)
Hey Hsin-Yi, are you available to review this small gecko ril patch?
(it's blocking a pretty big 1.3t :/)
Attachment #8402759 - Flags: review?(htsai)
Comment on attachment 8402759 [details] [diff] [review]
Gecko part (patch)

Review of attachment 8402759 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for this!!!
Attachment #8402759 - Flags: review?(htsai) → review+
The checking-needed is for the gecko part only. It can safely land before the gaia part.
Keywords: checkin-needed
triage: 1.3T+ for resolving dialer incoming call performance
blocking-b2g: 1.3T? → 1.3T+
Whiteboard: [tarako_only]
Re-opening for the gaia part, sorry I didn't put the correct leave-open keyword, will do next time :)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Not sure why the tarako_ony flag was set...
But this will land on master too.

Vivien can you help with making sure the gecko part is checked in where it needs? A bit lost here.
Flags: needinfo?(21)
Whiteboard: [tarako_only]
Ryan,
sorry for the confusion here. This patch is intended to help with the work beeing done in bug 990003 which tries to make the call screen appears instantly under heavy workload condition.

While that is something which is way more obvious on Tarako, this is something that is needed everywhere. Do you mind to uplift this patch to aurora and central as well ? Thanks.
Flags: needinfo?(ryanvm)
Keywords: leave-open
I can't help you with the Aurora uplift unless it gets approval (per the B2G Landing page, 1.3T+ doesn't grant approval for 1.4). Setting checkin-needed for trunk.
Comment on attachment 8402759 [details] [diff] [review]
Gecko part (patch)

Risk is very small since this patch only adds a new property to an existing message and does not change any logic on the Gecko side.
Attachment #8402759 - Flags: approval-mozilla-aurora?
Comment on attachment 8402759 [details] [diff] [review]
Gecko part (patch)

not affecting gecko is a good thing - uplift!
Attachment #8402759 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8402757 [details] [review]
Gaia Part (PR)

Yeah for less code and more tests!
Attachment #8402757 - Flags: review?(anthony) → review+
The gecko seems to be in; The gaia didn't patch cleanly.  Maybe there's something I did wrong?
Flags: needinfo?(etienne)
https://github.com/etiennesegonzac/gaia/commit/b3055042687fa2dd1a4f99eb4069662404c53e1a
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Flags: needinfo?(etienne)
Resolution: --- → FIXED
should this be uplift to v1.3t?
Would you please rebase your patch on v1.3t branch?
There're some conflicts when cherry-pick this into v1.3t branch.

#	双方修改:     apps/communications/dialer/js/calls_handler.js
#	双方修改:     apps/communications/dialer/js/dialer.js
#	双方修改:     apps/communications/dialer/js/handled_call.js
#	双方修改:     apps/communications/dialer/test/unit/calls_handler_test.js
#	双方修改:     apps/communications/dialer/test/unit/dialer_test.js
#	双方修改:     apps/communications/dialer/test/unit/handled_call_test.js
Flags: needinfo?(etienne)
Gaia part uplifted to 1.3t (Gecko part already landed there): https://github.com/mozilla-b2g/gaia/commit/23488b1a45221c17e6a32fdd4c9d0fdbdcf2d021
Flags: needinfo?(etienne)
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #22)
> https://hg.mozilla.org/releases/mozilla-aurora/rev/99430f6e5e24
> 
> v1.4:
> https://github.com/mozilla-b2g/gaia/commit/
> 28716364c7a9e43e6911c74e3f80ab37af0422b7

This seems to have broken gaia-unit tests on Aurora: https://tbpl.mozilla.org/php/getParsedLog.php?id=38438424&tree=Mozilla-Aurora
Comment on attachment 8402757 [details] [review]
Gaia Part (PR)

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 #):
[User impact] if declined:
[Testing completed]:
[Risk to taking this patch] (and alternatives if risky):
[String changes made]:
Attachment #8402757 - Flags: approval-gaia-v1.4?(praghunath)
Whiteboard: [approval-v1.4]
Whiteboard: [approval-v1.4] → [1.4-approval-needed]
Hi Etienne,

Mind if you can share if the broken issue on gaia-unit tests in v1.4 has been resolved. We need this patch in v1.4 since it is the blocker for partner's test.
Flags: needinfo?(etienne)
Ivan

Please fill in the responses into this form.


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 #):
[User impact] if declined:
[Testing completed]:
[Risk to taking this patch] (and alternatives if risky):
[String changes made]:
Flags: needinfo?(itsay)
Attached patch Patch for 1.4 with tests working (obsolete) — Splinter Review
Flags: needinfo?(etienne)
Comment on attachment 8402757 [details] [review]
Gaia Part (PR)

No response on impact so not taking in 1.4 yet. Clearing noms till response is received.
Attachment #8402757 - Flags: approval-gaia-v1.4?(praghunath)
Comment on attachment 8402757 [details] [review]
Gaia Part (PR)

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 #):
[User impact] if declined: Partner blocker on Dolphin
[Testing completed]: Yes
[Risk to taking this patch] (and alternatives if risky): Low
[String changes made]: N/A
Attachment #8402757 - Flags: approval-gaia-v1.4?(praghunath)
Flags: needinfo?(itsay)
(In reply to Preeti Raghunath(:Preeti) from comment #29)
> Comment on attachment 8402757 [details] [review]
> Gaia Part (PR)
> 
> No response on impact so not taking in 1.4 yet. Clearing noms till response
> is received.

Well it's a strong dependence of bug 990003 which is already 1.4+ [1] so I'm not sure why it has to get through approval again...

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=990003#c77
Attachment #8402757 - Flags: approval-gaia-v1.4?(praghunath) → approval-gaia-v1.4+
Comment on attachment 8402757 [details] [review]
Gaia Part (PR)

See https://bugzilla.mozilla.org/show_bug.cgi?id=990003#c85.
Attachment #8402757 - Flags: approval-gaia-v1.4+ → approval-gaia-v1.4-
Hi Gabriele,

Once the 1.4 compatible patch is ready, please help to submit "approval-gaia-v1.4=?" request for this bug to be uplifted to v1.4. Thanks you very much.
Flags: needinfo?(gsvelto)
Etienne had already provided a v1.4 patch for this in attachment 8432412 [details] [diff] [review] but for some reason it doesn't apply on my tree on top of bug 993018. I'm getting a clean uplift once I've applied the patch in bug 993018 with only a couple of unit-tests failing so I'll try to make that into a workable patch.
Flags: needinfo?(gsvelto)
This is a clean cherry-pick from master that applies on top of attachment 8446514 [details] [diff] [review], there were no conflicts while cherry-picking however two unit-tests were failing locally due to some information contained by a mock leaking from a test suite to another. I added a one-liner fix for the issue in this patch. This is a dependency of bug 990003 which should be uplifted to v1.4.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Decoupling needed for bug 990003
[User impact] if declined: Cannot uplift bug 990003 to v1.4
[Testing completed]: The patch required only a small change to a mock but no changes to the actual logic and has already been used in master for a while
[Risk to taking this patch] (and alternatives if risky): Uplifting the dependencies without this patch is unrealistic
[String changes made]: None

Quick question for Etienne, you've already made attachment 8432412 [details] [diff] [review] for this bug but it doesn't apply cleanly on top of the patch in bug 993018. Can you provide some information on that? Unless attachment 8432412 [details] [diff] [review] contained some additional important changes I'm not aware of I'd rather pick this one which is a straightforward cherry-pick from master.
Attachment #8446565 - Flags: approval-gaia-v1.4?(release-mgmt)
Flags: needinfo?(etienne)
(In reply to Gabriele Svelto [:gsvelto] from comment #35)
> Quick question for Etienne, you've already made attachment 8432412 [details] [diff] [review]
> [diff] [review] for this bug but it doesn't apply cleanly on top of the
> patch in bug 993018. Can you provide some information on that? Unless
> attachment 8432412 [details] [diff] [review] contained some additional
> important changes I'm not aware of I'd rather pick this one which is a
> straightforward cherry-pick from master.

No IIRC it was only test related changes (probably removing some too!), so please go ahead with the version that applies cleanly :)
Flags: needinfo?(etienne)
Comment on attachment 8432412 [details] [diff] [review]
Patch for 1.4 with tests working

Thanks for the info Etienne, I'm marking this patch as obsolete to avoid confusion.
Attachment #8432412 - Attachment is obsolete: true
Hi Lawrence,

Please help to check this bug for the 1.4 uplift approval. Thanks.
Flags: needinfo?(lmandel)
Comment on attachment 8446565 [details] [diff] [review]
Cherry-picked patch for v1.4

Approved for 1.4.
Attachment #8446565 - Flags: approval-gaia-v1.4?(release-mgmt) → approval-gaia-v1.4+
Flags: needinfo?(lmandel)
You need to log in before you can comment on or make changes to this bug.