Closed Bug 990003 Opened 6 years ago Closed 6 years ago

[Dolphin][Tarako][Perf][Dialer] It takes a long time for the call screen shows up

Categories

(Firefox OS Graveyard :: Performance, defect, P1, blocker)

ARM
Gonk (Firefox OS)
defect

Tracking

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

VERIFIED FIXED
1.4 S6 (25apr)
blocking-b2g 1.3T+
Tracking Status
b2g-v1.3T --- verified
b2g-v1.4 --- fixed

People

(Reporter: bli, Assigned: etienne)

References

Details

(Keywords: perf, Whiteboard: [c=progress p= s= u=tarako] [priority][sprd296141])

Attachments

(9 files, 4 obsolete files)

1.67 MB, video/webm
Details
407.50 KB, image/png
Details
46 bytes, text/x-github-pull-request
rik
: review+
alive
: review+
vingtetun
: review+
shawnjohnjr
: feedback+
AndreiH
: feedback+
alive
: feedback+
Details | Review
58.97 KB, image/png
Details
46 bytes, text/x-github-pull-request
Details | Review
64.96 KB, image/jpeg
Details
196.79 KB, image/tiff
Details
72.65 KB, image/tiff
Details
46 bytes, text/x-github-pull-request
Details | Review
Environment:
------------------------------------------------
Gaia 14ef4fcdf9199f04f7678755c917dc77f51e13ba
Gecko https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/b574a7967338
BuildID 20140329004002
Version 28.1
ro.build.version.incremental=70
ro.build.date=Fri Mar 28 06:17:40 CST 2014


Steps to reproduce:
----------------------------
1. Goto Dailer
2. Dail a number then tap on the green button on the bottom
3. Wait for the call screen showing up.


Actual result:
------------------------------
The average time for call screen showing up is 3.8s


Additional info:
-----------------------------
[android2.2.2] The average time is 2.2s
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 983476
oops, I misread.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
blocking-b2g: --- → 1.3?
Keywords: perf
blocking-b2g: 1.3? → 1.3T?
Etienne, do you mind looking at this? thanks
Flags: needinfo?(etienne)
Our best bet here would be the wip mentioned here: https://bugzilla.mozilla.org/show_bug.cgi?id=987022#c26

But it's a big/risky patch.
Flags: needinfo?(etienne)
we seem to be mixing things up in bug 987022
Etienne, do you think it makes sense to provide your patch https://bugzilla.mozilla.org/show_bug.cgi?id=987022#c26 in this bug instead and we track your patch using this bug?
Thanks
blocking-b2g: 1.3T? → 1.3T+
Flags: needinfo?(etienne)
(In reply to Joe Cheng [:jcheng] from comment #5)
> we seem to be mixing things up in bug 987022
> Etienne, do you think it makes sense to provide your patch
> https://bugzilla.mozilla.org/show_bug.cgi?id=987022#c26 in this bug instead
> and we track your patch using this bug?
> Thanks

Yes.
But what's the deadline? And can we do a specific QA round for this patch if we move forward (all call screen related scenarios, including bluetooth and headset support)?

Right now it's hard to say if this approach is realistic at all for 1.3t...
Flags: needinfo?(etienne)
we have until end of April to stabilize the tarako build but we don't want to wait until the end for major changes. what do you think the level of effort is on this bug? approximately how many working days would be needed? thanks
Flags: needinfo?(etienne)
Etienne, this bug seem to be a dup of Bug 983476 
do you have a preference on which bug you want to work on and dup one of it to the other?
Do you mind taking the bug? thanks
Whiteboard: [MP_Blocker]
Duplicate of this bug: 983476
Duplicate of this bug: 977480
Attached patch 1.3T POC (obsolete) — Splinter Review
Assignee: nobody → etienne
Status: REOPENED → NEW
Flags: needinfo?(etienne)
Whiteboard: [MP_Blocker] → [priority]
Depends on: 992948
Depends on: 993018
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [priority] → [c=progress p= s= u=tarako] [priority]
Severity: normal → blocker
Hi again Fabrice, I think we need some arbitrage re: bug 987022 comment 40. Getting mixed messages :)

Can you confirm we won't take this risky patch for 1.3t?
Do we want this patch ready "in case of emergency"?
Do we need it rock solid by the end of the week?

FWIW I'm more than happy to just continue this work as an investigation for future versions :)
Flags: needinfo?(fabrice)
Duplicate of this bug: 963963
Code for the demo

https://github.com/etiennesegonzac/gaia/tree/instacallscreen
https://github.com/etiennesegonzac/gaia/tree/instacallscreen-1.3t

TODO:
* cleanup the style/ (js already done)
* cleanup the system app changes
* add an IAC for the bluetooth/headset commands
* rebase on top of bug 992948 and bug 993018
(In reply to Etienne Segonzac (:etienne) from comment #12)
> Hi again Fabrice, I think we need some arbitrage re: bug 987022 comment 40.
> Getting mixed messages :)
> 
> Can you confirm we won't take this risky patch for 1.3t?

I confirm it's risky, but I want it.

> Do we want this patch ready "in case of emergency"?

That's already an emergency.

> Do we need it rock solid by the end of the week?

As solid as possible, yes!
Blocks: 989590
Flags: needinfo?(fabrice)
> TODO:
> * add an IAC for the bluetooth/headset commands
> * cleanup the system app changes

DONE:
> * cleanup the style/ (js already done)
> * rebase on top of bug 992948 and bug 993018
Please test outbound as well as inbound calls for the screen to come up.  If inbound calls shows a delay, please reopen bug 983476
Duplicate of this bug: 994266
(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #18)
> Please test outbound as well as inbound calls for the screen to come up.  If
> inbound calls shows a delay, please reopen bug 983476

It's all smooth.
We even tried inbound call while the phone is playing a video, all good :)
> DONE:
> > * cleanup the style/ (js already done)
> > * rebase on top of bug 992948 and bug 993018
> > * add an IAC for the bluetooth/headset commands (just listening to the system messages)
> > * cleanup the system app changes
> > * add loads of tests

Ok, I'm just waiting for bug 992948 to land (travis...) before I rebase and send the pull request.
Probably tomorrow morning Paris time.

In the meantime, the patch is now complete and ready to enjoy :)
https://github.com/etiennesegonzac/gaia/compare/instacallscreen

Naoki, can we do a QA spin on this branch? (it's marster + bug 992948 + the patch for this bug)
Flags: needinfo?(nhirata.bugzilla)
I couldn't cleanly apply bug 992948; having said that I built my own gecko 1.3T from that bug and then took Etienne's gaia branch.

The speed of the response of the ui for incoming and outgoing phone calls are tremendously increased.
Outgoing calls seem to have "Withheld number" for some reason though?  I think I've also noticed a few other oddities.

Gaia      6b7abd089aa3ac7e6e98db9cdba1751e43bd3017
Gecko     2474baf7cdb31b1986e59c5d1f3fbd8fb6288fc3
BuildID   20140410135103
Version   28.1
ro.build.version.incremental=eng.cltbld.20140410.040335
ro.build.date=Thu Apr 10 04:03:42 EDT 2014
Flags: needinfo?(nhirata.bugzilla)
Blocks: 994585
Attached image Documentation
Just adding quick documentation diagram to make reviews easier.
Attached file Gaia PR
Here's the pull request against master.
r? Rik for the dialer/callscreen parts
r? Alive for the system app part
r? Vivien for the small build system add
Attachment #8405198 - Flags: review?(anthony)
Attachment #8405198 - Flags: review?(alive)
Attachment #8405198 - Flags: review?(21)
(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #22)
> I couldn't cleanly apply bug 992948; having said that I built my own gecko
> 1.3T from that bug and then took Etienne's gaia branch.

Thanks for taking a look so quickly!
I'll make a proper 1.3t rebase later today, so we can test/discuss from the exact same version.
Hey Shawn, I think you showed me a Bluetooth certification test suite during a work week.
Can we run it against this PR to make sure we don't regress on any test?
(Already tested the classic Bluetooth use cases manually, but I think we cached a few race conditions with this suite :))
Flags: needinfo?(shuang)
Zac, I updated the call_screen region for the on-device python test, is there anything else missing?
Flags: needinfo?(zcampbell)
(In reply to Etienne Segonzac (:etienne) from comment #26)
> Hey Shawn, I think you showed me a Bluetooth certification test suite during
> a work week.
> Can we run it against this PR to make sure we don't regress on any test?
> (Already tested the classic Bluetooth use cases manually, but I think we
> cached a few race conditions with this suite :))

Hey Etienne,
Thanks for raising up the topic. Shall I wait for r+ first and perform the test? Or you wish to test before r+?
(In reply to Shawn Huang [:shuang] [:shawnjohnjr] from comment #28)
> Hey Etienne,
> Thanks for raising up the topic. Shall I wait for r+ first and perform the
> test? Or you wish to test before r+?

There shouldn't be too much code churn on this part, if it works for you I think Monday afternoon Taipei time is a good timing, will be online if there's any issue.
Comment on attachment 8405198 [details] [review]
Gaia PR

Bebe or AndreiH, can you reset-gaia this locally onto your phone and run all of the call tests? Cheers.
Attachment #8405198 - Flags: feedback?(florin.strugariu)
Attachment #8405198 - Flags: feedback?(andrei.hutusoru)
Flags: needinfo?(zcampbell)
Comment on attachment 8405198 [details] [review]
Gaia PR

The build part seems good to me.
Attachment #8405198 - Flags: review?(21) → review+
Comment on attachment 8405198 [details] [review]
Gaia PR

I tested this without --restart command, tests fail at launch because Bug 990580.
5/6 tests passed: https://pastebin.mozilla.org/4784066

But that can be easily fixed in our tests, changing the self.wait_for_element_not_present(*self._call_screen_locator) to displayed as it should be: https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/gaiatest/apps/phone/regions/call_screen.py#L48
Attachment #8405198 - Flags: feedback?(andrei.hutusoru) → feedback+
Ettienne, comment #32 is saying that the call screen (HTML) is not being removed from the DOM whereas previously it was.
Is this intentional? If so, just change line #48 there to `wait_for_element_not_displayed` as AndreiH suggested.
Flags: needinfo?(etienne)
(In reply to Zac C (:zac) from comment #33)
> Ettienne, comment #32 is saying that the call screen (HTML) is not being
> removed from the DOM whereas previously it was.
> Is this intentional? 

Yes we need to keep it in the DOM.

> If so, just change line #48 there to
> `wait_for_element_not_displayed` as AndreiH suggested.

Done!

Thanks!
Flags: needinfo?(etienne)
Comment on attachment 8405198 [details] [review]
Gaia PR

Just rebased against the new build system changes.
Yuren I think you should have a look.

We're not super proud of the shared/style/dialer folder, but it's the best compromise we found.
Attachment #8405198 - Flags: review?(yurenju.mozilla)
Looks like it failed on travis ( https://travis-ci.org/mozilla-b2g/gaia/builds/22770418 ) for marionette:
2 failing
1) improve b2g "before each" hook:
Error: timeout of 60000ms exceeded
at null.<anonymous> (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:165:14)
at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)

2) improve b2g "after each" hook:
Error: Not connected. To write data you must call connect first.
at TcpSync.send (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/drivers/tcp-sync.js:99:15)
at Object.send (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:458:36)
at Object.Client._sendCommand (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:504:19)
at Object.closeDriver (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:752:14)
at Object.executeHook (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:360:20)
at Object.Client.runHook (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:378:7)
at Object.destroySession [as deleteSession] (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:762:12)
at Context.<anonymous> (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-js-runner/lib/runtime/hostmanager.js:102:14)
at Hook.Runnable.run (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:194:15)
at next (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:257:10)
at Object._onImmediate (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:274:5)
at processImmediate [as _immediateCallback] (timers.js:330:15)
Attached file 1.3t version for QA (obsolete) —
Here is a branch with this patch and its 2 dependencies rebased on 1.3t.
Attachment #8402550 - Attachment is obsolete: true
Attachment #8405453 - Flags: feedback?(nhirata.bugzilla)
Comment on attachment 8405198 [details] [review]
Gaia PR

Don't want to be picky on attention screen change and I cannot figure out any drawback if we make callscreen in process. (Is the callscreen in process?)
BTW how do communicate with dialer since you cannot postMessage to window.parent?
Also there's a mozbrowserresize event to let callscreen to show its caller if my memory serves me right.
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/attention_screen.js#L67
Another question: how do you recover callscreen when you launch dialer or we are going to discard this feature?


And you owe me AttentionWindow review and help to move the transition. :)
Attachment #8405198 - Flags: review?(yurenju.mozilla)
Attachment #8405198 - Flags: review?(alive)
Attachment #8405198 - Flags: review+
Attachment #8405198 - Flags: feedback+
Attached image 2014-04-11-09-29-16.png
I just took the branch again and put it on the gecko.

The dialer UI comes up pretty consistently as almost as soon as the ring starts happening which is awesome.

There's one oddity that I noticed and I'm not sure if it's me or not.  When dialing out, the number (using a contact name or just calling using a number) will show as withheld.  Incoming calls seem fine.
Comment on attachment 8405453 [details]
1.3t version for QA

See attachment in regards to "Withheld number" when dialing out.
Otherwise this looks good from an user perspective.
Attachment #8405453 - Flags: feedback?(nhirata.bugzilla) → feedback-
(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #39)
> There's one oddity that I noticed and I'm not sure if it's me or not.  When
> dialing out, the number (using a contact name or just calling using a
> number) will show as withheld.  Incoming calls seem fine.

Interesting.
I can't reproduce it though, which gecko are you using?
(In reply to Alive Kuo [:alive][NEEDINFO!][God bless Taiwan.] from comment #38)
> Comment on attachment 8405198 [details] [review]
> Gaia PR
> 
> Don't want to be picky on attention screen change and I cannot figure out
> any drawback if we make callscreen in process. (Is the callscreen in
> process?)
> BTW how do communicate with dialer since you cannot postMessage to
> window.parent?
> Also there's a mozbrowserresize event to let callscreen to show its caller
> if my memory serves me right.
> https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/
> attention_screen.js#L67

Just repeating the discussion on IRC, those were handled by the dependency bugs.

> Another question: how do you recover callscreen when you launch dialer or we
> are going to discard this feature?

We're indeed losing the fact that when a call is ongoing opening the dialer won't expand the call screen from the status bar (but placing a new call will).
(In reply to Etienne Segonzac (:etienne) from comment #41)
> (In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from
> comment #39)
> > There's one oddity that I noticed and I'm not sure if it's me or not.  When
> > dialing out, the number (using a contact name or just calling using a
> > number) will show as withheld.  Incoming calls seem fine.
> 
> Interesting.
> I can't reproduce it though, which gecko are you using?

1.3t Gecko
Version   28.1
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/257dd37da601
Comment on attachment 8405198 [details] [review]
Gaia PR

Flagging Shawn for feedback so we don't loose track of the pending bluetooth testing.
Attachment #8405198 - Flags: feedback?(florin.strugariu) → feedback?(shuang)
I was able to reproduce the bug from Comment 39 with the latest PVT build.

It does not reproduce with this patch and a gecko from last week.
It does not reproduce without this patch on the latest gecko build.

But, did some debugging and call.number is "" (empty string) for outgoing calls :/
Tried checking again when the call is connected, still "".
The state of the call is good though, and goes from "dialing" to "connected"

Pretty weird, and works perfectly for incoming calls, any idea Hsin-Yi?
Flags: needinfo?(htsai)
(In reply to Etienne Segonzac (:etienne) from comment #45)
> I was able to reproduce the bug from Comment 39 with the latest PVT build.
> 
> It does not reproduce with this patch and a gecko from last week.
> It does not reproduce without this patch on the latest gecko build.
> 
> But, did some debugging and call.number is "" (empty string) for outgoing
> calls :/
> Tried checking again when the call is connected, still "".
> The state of the call is good though, and goes from "dialing" to "connected"
> 
> Pretty weird, and works perfectly for incoming calls, any idea Hsin-Yi?

Do you have any logs, including RIL messages? 

I have a problem with flashing gaia :( Will check it tomorrow!
I'm having trouble to test bluetooth handsfree profile. Now, b2g process will be killed by low memory shrink after making outgoing call with bluetooth headset.
Flags: needinfo?(shuang)
(In reply to Shawn Huang [:shuang] [:shawnjohnjr] from comment #47)
> I'm having trouble to test bluetooth handsfree profile. Now, b2g process
> will be killed by low memory shrink after making outgoing call with
> bluetooth headset.
See bug 994411.
Keep in? flag.
Flags: needinfo?(shuang)
Can I test dialer patch on branch 1.3 using Buri? I cannot test on Tarako currently due to b2g process will be killed due to out-of-memory. Since we only need to check the bluetooth logic part.
Flags: needinfo?(etienne)
(In reply to Shawn Huang [:shuang] [:shawnjohnjr] from comment #50)
> Can I test dialer patch on branch 1.3 using Buri? I cannot test on Tarako
> currently due to b2g process will be killed due to out-of-memory. Since we
> only need to check the bluetooth logic part.

Yes I did my some bluetooth testing on a buri 1.3 + the gaia from the PR, it worked fine for telephony stuffs.
Flags: needinfo?(etienne)
Blocks: 995979
Blocks: 996002
(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #46)
> Do you have any logs, including RIL messages? 

Sadly no, I only flash my own gaia on tarakos.

> 
> I have a problem with flashing gaia :( Will check it tomorrow!

Thanks !
Attached file Gaia PR against 1.3t
Attachment #8405453 - Attachment is obsolete: true
Summary: [Tarako][Perf][Dailer] It takes a long time for the call screen shows up → [Tarako][Perf][Dialer] It takes a long time for the call screen shows up
We need land this patch today.
Flags: needinfo?(styang)
Flags: needinfo?(jcheng)
Flags: needinfo?(fabrice)
That will land once Rik has given r+
Flags: needinfo?(fabrice)
Comment on attachment 8405198 [details] [review]
Gaia PR

I have tested:
* Bluetooth handsfree 3-way calling
* Bluetooth memory dialing / last number dialing
Attachment #8405198 - Flags: feedback?(shuang) → feedback+
Flags: needinfo?(shuang)
Depends on: 996421
(In reply to Etienne Segonzac (:etienne) from comment #45)
> I was able to reproduce the bug from Comment 39 with the latest PVT build.
> 
> It does not reproduce with this patch and a gecko from last week.
> It does not reproduce without this patch on the latest gecko build.
> 
> But, did some debugging and call.number is "" (empty string) for outgoing
> calls :/
> Tried checking again when the call is connected, still "".
> The state of the call is good though, and goes from "dialing" to "connected"
> 
> Pretty weird, and works perfectly for incoming calls, any idea Hsin-Yi?

There's a miss when we provided a 1.3t patch for bug 990467. I filed a followup bug 996421 to address the issue.
Flags: needinfo?(htsai)
(In reply to Fabrice Desré [:fabrice] from comment #56)
> That will land once Rik has given r+

And be uplifted only once bug 996421 lands on 1.3t.
Comment on attachment 8405198 [details] [review]
Gaia PR

We did a pair review yesterday and Étienne addressed all my comments, so good to go.
One "regression" from this patch is that when the last call hangs up, during the sliding transition, we see the phone background instead of the app. But I'm fine with it and this will be fixed in a follow up (probably the AttentionWindow refactoring).
Attachment #8405198 - Flags: review?(anthony) → review+
Attached image With patch applied
This is a representation of our users with this patch applied.
\o/

Landed on master:
https://github.com/mozilla-b2g/gaia/commit/7afce8a6e965a45f38988227828ccdfb315394f3

The PR again 1.3t is up to date, but if we land it before bug 996421 we'll trigger the bug mentioned in Comment 39.

Thanks all, this was a fun one :)
Status: ASSIGNED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Flags: needinfo?(jcheng)
Please land it on v1.3t.
Flags: needinfo?(fabrice)
Flags: needinfo?(fabrice)
Finally, I can build a image with slog disabled. Without this or other patches to improve performance, the call screen can be displayed within 2 ringbacks when playing music. Do we need to re-consider if we want to move callscreen into b2g process?
Flags: needinfo?(tlee)
Flags: needinfo?(fabrice)
Duplicate of this bug: 989594
Duplicate of this bug: 973596
Duplicate of this bug: 994585
(In reply to Patrick Wang (Chih-Kai Wang) (:kk1fff) from comment #65)
> Finally, I can build a image with slog disabled. Without this or other
> patches to improve performance, the call screen can be displayed within 2
> ringbacks when playing music. Do we need to re-consider if we want to move
> callscreen into b2g process?

James, could you please explain why slog is needed even in user build? thanks!
Flags: needinfo?(styang) → needinfo?(james.zhang)
(In reply to Patrick Wang (Chih-Kai Wang) (:kk1fff) from comment #65)
> Finally, I can build a image with slog disabled. Without this or other
> patches to improve performance, the call screen can be displayed within 2
> ringbacks when playing music. Do we need to re-consider if we want to move
> callscreen into b2g process?

Let's not do further changes. We need to stabilize now.
Flags: needinfo?(fabrice)
(In reply to Steven Yang [:styang] from comment #69)
> (In reply to Patrick Wang (Chih-Kai Wang) (:kk1fff) from comment #65)
> > Finally, I can build a image with slog disabled. Without this or other
> > patches to improve performance, the call screen can be displayed within 2
> > ringbacks when playing music. Do we need to re-consider if we want to move
> > callscreen into b2g process?
> 
> James, could you please explain why slog is needed even in user build?
> thanks!

OEM will open slog by engineer mode.
Flags: needinfo?(james.zhang)
And we will disable slog in user build.
Flags: needinfo?(tlee)
Blocks: 812059
No longer blocks: 812059
See Also: → 812059
Depends on: 997924
verified and fixed with today's daily build
screen show up time is 2.15 seconds

Gaia      3f1d8332d127f1d6bc0fdbb08146ce1deb0efbc0
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/5da76152c2bd
BuildID   20140421164001
Version   28.1
Status: RESOLVED → VERIFIED
Blocks: 999478
Depends on: 999545
Duplicate of this bug: 812059
Preeti, do we want this fix for 1.4?
Flags: needinfo?(praghunath)
See Also: → 998241
(In reply to Mike Lee [:mlee] from comment #75)
> Preeti, do we want this fix for 1.4?

Not knowing the answer to this is already causing issues with uplifts...
We need to know ASAP.
Depends on: 1000047
(In reply to Mike Lee [:mlee] from comment #75)
> Preeti, do we want this fix for 1.4?

Lets take it in 1.4 as well. Sorry for the delay.
Flags: needinfo?(praghunath)
(In reply to Preeti Raghunath(:Preeti) from comment #77)
> (In reply to Mike Lee [:mlee] from comment #75)
> > Preeti, do we want this fix for 1.4?
> 
> Lets take it in 1.4 as well. Sorry for the delay.

Yes, we have kicked off v1.4 dolphin and shark project now.
Please cherry-pick to v1.4 branch.
Whiteboard: [c=progress p= s= u=tarako] [priority] → [c=progress p= s= u=tarako] [priority][sprd296141]
Flags: needinfo?(ttsai)
Flags: needinfo?(kli)
Flags: needinfo?(kkuo)
Flags: needinfo?(ehung)
Depends on: 1010418
No longer depends on: 1010418
Duplicate of this bug: 985862
Flags: needinfo?(ehung)
Flags: needinfo?(kli)
When can we land it on v1.4?
Flags: needinfo?(styang)
Flags: needinfo?(etienne)
Comment on attachment 8405198 [details] [review]
Gaia 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 #8405198 - Flags: approval-gaia-v1.4?(praghunath)
(In reply to James Zhang from comment #80)
> When can we land it on v1.4?

Yes just let me know if there's a conflict while uplifting
Flags: needinfo?(etienne)
Flags: needinfo?(ttsai)
Whiteboard: [c=progress p= s= u=tarako] [priority][sprd296141] → [c=progress p= s= u=tarako] [priority][sprd296141][1.4-approval-needed]
Flags: needinfo?(kkuo)
Ivan

Please have these responses filled in.

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)
Hi Preeti, 

Here is the update, please help to proceed approval process. 

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): N/A
[User impact] if declined: Partner blocker on Dolphin
[Testing completed]: Yes
[Risk to taking this patch] (and alternatives if risky): Mid
[String changes made]: N/A
Flags: needinfo?(itsay) → needinfo?(praghunath)
Attachment #8405198 - Flags: approval-gaia-v1.4?(praghunath) → approval-gaia-v1.4+
Flags: needinfo?(praghunath)
Comment on attachment 8405198 [details] [review]
Gaia PR

This is way too risky to take into 1.4 at this point. There's no strong compelling case to consider this for 1.4 anyways, as this was specifically needed for Tarako.
Attachment #8405198 - Flags: approval-gaia-v1.4+ → approval-gaia-v1.4-
Whiteboard: [c=progress p= s= u=tarako] [priority][sprd296141][1.4-approval-needed] → [c=progress p= s= u=tarako] [priority][sprd296141]
No longer blocks: 1009596
(In reply to Jason Smith [:jsmith] from comment #85)
> Comment on attachment 8405198 [details] [review]
> Gaia PR
> 
> This is way too risky to take into 1.4 at this point. There's no strong
> compelling case to consider this for 1.4 anyways, as this was specifically
> needed for Tarako.

Just FTR, not uplifting this also means that every dialer related uplift to 1.4 will probably conflict (since we now have 2 separated apps).
Which is fine, but we better not change our opinion once again after that since we're going to make this *very* hard to uplift.

I raised this issue at the end for April (Comment 76) so having relman and QA still arguing now is troublesome.
(In reply to Etienne Segonzac (:etienne) from comment #86)
> (In reply to Jason Smith [:jsmith] from comment #85)
> > Comment on attachment 8405198 [details] [review]
> > Gaia PR
> > 
> > This is way too risky to take into 1.4 at this point. There's no strong
> > compelling case to consider this for 1.4 anyways, as this was specifically
> > needed for Tarako.
> 
> Just FTR, not uplifting this also means that every dialer related uplift to
> 1.4 will probably conflict (since we now have 2 separated apps).
> Which is fine, but we better not change our opinion once again after that
> since we're going to make this *very* hard to uplift.
> 
> I raised this issue at the end for April (Comment 76) so having relman and
> QA still arguing now is troublesome.

Lawrence is going to discuss this with Preeti. From my perspective, I don't think we should take this. This had a lot of blocking regression fallouts, which is something we can't take into 1.4 at this point. This is also technically a feature request for 1.4.
Flags: needinfo?(styang)
Target Milestone: --- → 1.4 S6 (25apr)
Whiteboard: [c=progress p= s= u=tarako] [priority][sprd296141] → [c=progress p= s= u=tarako] [priority][sprd296141][1.4-approval-needed]
I said this once, I'll say again. Way too risky for 1.4, this is not being taken to 1.4 at this time. There's no compelling reason to do so.
Whiteboard: [c=progress p= s= u=tarako] [priority][sprd296141][1.4-approval-needed] → [c=progress p= s= u=tarako] [priority][sprd296141]
(In reply to Jason Smith [:jsmith] from comment #88)
> I said this once, I'll say again. Way too risky for 1.4, this is not being
> taken to 1.4 at this time. There's no compelling reason to do so.

Hi Jason,

If this bug is not uplifted to v1.4, the impact to partner is that:
1. incoming screen appears about 2s after ringtone starts when there is no background app. If there is music on background, the delay is about 3s.
2. calling screen appears about 2.4s after user tap on dial button.

So, What if I can find the QA to verify the patch with v1.4 on Flame locally and see if it will cause any smoke test regression before requesting the approval again? what do you think?
Flags: needinfo?(jsmith)
Summary: [Tarako][Perf][Dialer] It takes a long time for the call screen shows up → [Dolphin][Tarako][Perf][Dialer] It takes a long time for the call screen shows up
I don't think local verification is enough here - even with thorough testing, a patch like this can blocking fallouts in areas we don't expect to fail. We also aren't doing heavy testing on 1.4 anymore, so this we really can take patches at this point that are low risk.

What I'd like to know here is the following:

1. How does the performance impact of Dolphin compare to Buri & Flame on 1.4?
2. Are we missing performance targets for Dolphin right now for the call screen? If so, how far off are we from those targets?
Flags: needinfo?(jsmith)
I agree with Jason in terms of the risk of this rather large patch. Jason is speaking from experience in landing this on 1.3T. There really needs to be an extremely strong need to take a bug like this on 1.4 at this point and I have not yet seen it in the comments. (We need to clarify the scope of our commitment for Dolphin.) I also didn't see much in the way of a risk assessment or explanation of the testing that has been competed in the uplift request in comment 84. I think Etienne made a good point about code divergence in comment 86. We owe it to everyone to come to a decision on this bug ASAP.

1. I would like to see answers to the questions in comment 90. 
2. I would also like to better understand the user impact from the numbers in comment 89. Do these delays make the phone unusable? ie. Can the delay on the incoming call cause a user to miss an incoming call?
3. Do we have alternative options to improve the times in comment 90 that require smaller and less risky changesets?
Flags: needinfo?(itsay)
(In reply to Lawrence Mandel [:lmandel] from comment #91)
> 3. Do we have alternative options to improve the times in comment 90 that
> require smaller and less risky changesets?

On the gaia side not really, we pretty much tried everything else before coming to this solution.
Performance test for call screen delay on Dolphin and Flame
(In reply to Lawrence Mandel [:lmandel] from comment #91)
> I agree with Jason in terms of the risk of this rather large patch. Jason is
> speaking from experience in landing this on 1.3T. There really needs to be
> an extremely strong need to take a bug like this on 1.4 at this point and I
> have not yet seen it in the comments. (We need to clarify the scope of our
> commitment for Dolphin.) I also didn't see much in the way of a risk
> assessment or explanation of the testing that has been competed in the
> uplift request in comment 84. I think Etienne made a good point about code
> divergence in comment 86. We owe it to everyone to come to a decision on
> this bug ASAP.
> 
> 1. I would like to see answers to the questions in comment 90. 

Here the test result on Dolphin and Flame. We do see a significant delay on Dolphin if more apps are in the background. The more memory is consumed, the longer the delay. Flame has less delay because it has 1G memory. The issue does significant impact the phone that is going to be shipped form our partner.

Dolphin: Delay from 2 seconds (no background app) up to 7 seconds (with background app Camera+Gallery+Music)
Flame: Delay from 1 second (no background app) up to 3 seconds (with background app Camera+Gallery+Music)

I attach the test result in the attachment for your reference.


> 2. I would also like to better understand the user impact from the numbers
> in comment 89. Do these delays make the phone unusable? ie. Can the delay on
> the incoming call cause a user to miss an incoming call?

Since the call screen for the user to pick up the phone is delayed, it is possible the users will miss the call because there is no way for them to pick up the call during the delay. For the worse case, the delay for 7 seconds is way to long that the phone should has been ringing for a while and the caller may hand up the phone. 

> 3. Do we have alternative options to improve the times in comment 90 that
> require smaller and less risky changesets?

Already answered by Eitan in comment 92.

Since we already land this bug in 2.0 an 1.3T, I think the risk should be manageable by my suggestion in comment 89. I can find the QA to verify the patch with v1.4 on Flame locally and see if it will cause any smoke test regression before uplifting the patch.
Flags: needinfo?(itsay)
Limit Flame to memory in 256MB. The delay of the call screen is also significant longer if there are background apps.
add ni? for answer the questions in comment 91
Flags: needinfo?(lmandel)
(In reply to Etienne Segonzac (:etienne) from comment #92)
> (In reply to Lawrence Mandel [:lmandel] from comment #91)
> > 3. Do we have alternative options to improve the times in comment 90 that
> > require smaller and less risky changesets?
> 
> On the gaia side not really, we pretty much tried everything else before
> coming to this solution.

Seems that this is the only option we have if we want to fix the issue in 1.4 then. Thanks.

(In reply to Ivan Tsay (:ITsay) from comment #94)
> Here the test result on Dolphin and Flame. We do see a significant delay on
> Dolphin if more apps are in the background. The more memory is consumed, the
> longer the delay. Flame has less delay because it has 1G memory. The issue
> does significant impact the phone that is going to be shipped form our
> partner.
> 
> Dolphin: Delay from 2 seconds (no background app) up to 7 seconds (with
> background app Camera+Gallery+Music)
> Flame: Delay from 1 second (no background app) up to 3 seconds (with
> background app Camera+Gallery+Music)
> 
> I attach the test result in the attachment for your reference.

Thank you for posting the results. They are helpful for this discussion.
 
> > 2. I would also like to better understand the user impact from the numbers
> > in comment 89. Do these delays make the phone unusable? ie. Can the delay on
> > the incoming call cause a user to miss an incoming call?
> 
> Since the call screen for the user to pick up the phone is delayed, it is
> possible the users will miss the call because there is no way for them to
> pick up the call during the delay. For the worse case, the delay for 7
> seconds is way to long that the phone should has been ringing for a while
> and the caller may hand up the phone. 

Agreed. If the network has established a connection and has initiated 'ringing' for the caller, a 7 second delay may well be enough time for them to disconnect before the phone rings.

Jason - Can you comment on this user impact of this issue from a QA perspective? (Not whether or not we should take a fix just how this issue compares to others that QA has been willing to accept in prior releases.)
  
> Since we already land this bug in 2.0 an 1.3T, I think the risk should be
> manageable by my suggestion in comment 89. I can find the QA to verify the
> patch with v1.4 on Flame locally and see if it will cause any smoke test
> regression before uplifting the patch.

I agree that we should have some sense of what is likely to break/regress based on our prior experience. However, a smoke test is not a thorough test. We are still opening up the branch to regressions by taking a large patch.

Jason - Can you itemize the specific regressions introduced when this patch landed on 1.3T and 2.0 so that we can have a sense of the scope of the risk based on our prior landings? 

I think the bigger question is whether this bug is in scope for the work that we agreed to take on the 1.4 branch at this point in the schedule (I don't know that supporting a 256MB device was previously a requirement of 1.4) and, if our partner is going to take this patch on their branch if we don't take it on 1.4, do we want to have more visibility and control over the regressions or leave the resolution of these issues to our partner.
Flags: needinfo?(lmandel)
(In reply to Lawrence Mandel [:lmandel] from comment #97)
> > > 2. I would also like to better understand the user impact from the numbers
> > > in comment 89. Do these delays make the phone unusable? ie. Can the delay on
> > > the incoming call cause a user to miss an incoming call?
> > 
> > Since the call screen for the user to pick up the phone is delayed, it is
> > possible the users will miss the call because there is no way for them to
> > pick up the call during the delay. For the worse case, the delay for 7
> > seconds is way to long that the phone should has been ringing for a while
> > and the caller may hand up the phone. 
> 
> Agreed. If the network has established a connection and has initiated
> 'ringing' for the caller, a 7 second delay may well be enough time for them
> to disconnect before the phone rings.
> 
> Jason - Can you comment on this user impact of this issue from a QA
> perspective? (Not whether or not we should take a fix just how this issue
> compares to others that QA has been willing to accept in prior releases.)

7 seconds is pretty bad. Although the question I have is this:

What happens on 1.3 Buri right now?

If we're worse than Buri, then that means we're shipping with lower quality than past releases that have already shipped. If we're the same, then that means at least one partner has been okay with shipping with this problem.

Ivan - Can you get comparable data here on Buri 1.4?

>   
> > Since we already land this bug in 2.0 an 1.3T, I think the risk should be
> > manageable by my suggestion in comment 89. I can find the QA to verify the
> > patch with v1.4 on Flame locally and see if it will cause any smoke test
> > regression before uplifting the patch.
> 
> I agree that we should have some sense of what is likely to break/regress
> based on our prior experience. However, a smoke test is not a thorough test.
> We are still opening up the branch to regressions by taking a large patch.
> 
> Jason - Can you itemize the specific regressions introduced when this patch
> landed on 1.3T and 2.0 so that we can have a sense of the scope of the risk
> based on our prior landings? 

The two known regressions are:

* bug 997924
* bug 1000047
Flags: needinfo?(itsay)
(In reply to Jason Smith [:jsmith] from comment #98)
> (In reply to Lawrence Mandel [:lmandel] from comment #97)
> > > > 2. I would also like to better understand the user impact from the numbers
> > > > in comment 89. Do these delays make the phone unusable? ie. Can the delay on
> > > > the incoming call cause a user to miss an incoming call?
> > > 
> > > Since the call screen for the user to pick up the phone is delayed, it is
> > > possible the users will miss the call because there is no way for them to
> > > pick up the call during the delay. For the worse case, the delay for 7
> > > seconds is way to long that the phone should has been ringing for a while
> > > and the caller may hand up the phone. 
> > 
> > Agreed. If the network has established a connection and has initiated
> > 'ringing' for the caller, a 7 second delay may well be enough time for them
> > to disconnect before the phone rings.
> > 
> > Jason - Can you comment on this user impact of this issue from a QA
> > perspective? (Not whether or not we should take a fix just how this issue
> > compares to others that QA has been willing to accept in prior releases.)
> 
> 7 seconds is pretty bad. Although the question I have is this:
> 
> What happens on 1.3 Buri right now?

This should say "1.4 Buri"
Hi Pei-pei,

Please help to test/check the this issue on Buri v1.4? Thanks!
Flags: needinfo?(itsay) → needinfo?(pcheng)
Buri v1.4 has similar problem.

1. No Background App - incoming call delay 2s
2. Music background - incoming call delay 3s
3. Gallery + Music - incoming call delay 5s+, up to 10s sometimes, Music will get killed after call.
   Marketplace - incoming call delay 5s+, up to 10s sometimes.

In test 3, frontend apps and Homescreen will be killed first (due to low memory) and then call screen was launched. The delay can be up to 10s if there is not much memory left on device.
Flags: needinfo?(pcheng)
Flags: needinfo?(kevin)
Comment on attachment 8405198 [details] [review]
Gaia PR

We've been requested by Spreadtrum to have this uplifted for Dolphin 1.4 release. Does that change anything?
Attachment #8405198 - Flags: approval-gaia-v1.4- → approval-gaia-v1.4?(jsmith)
Plus a depending bug has 1.4 uplift approval but this patch doesn't have it... (bug 1002327)
Comment on attachment 8405198 [details] [review]
Gaia PR

Not really. From Mozilla's perspective, we will only consider performance improvements for 1.4 that demonstrate that Dolphin is worse than Buri for 1.4. The above testing shows that Buri & Dolphin are on par for performance, which is something we've shipped with historically. On that regard, we will not be taking this into 1.4.
Attachment #8405198 - Flags: approval-gaia-v1.4?(jsmith) → approval-gaia-v1.4-
Cross-comment from bug 1002327, Gabriele doesn't agree:

(In reply to Gabriele Svelto [:gsvelto] from comment #34)
> (In reply to Jan Jongboom [:janjongboom] (Telenor) from comment #33)
> > This bug has v1.4 uplift approval, but bug 990003 hasn't. We should uplift
> > that one first then right? We want this patch in 1.4 for Dolphin release.
> 
> Yes, bug 990003 has so many other benefits besides this dependency that I
> think it's a must have for 1.4.
Flags: needinfo?(jsmith)
Dolphin 1.4 has real production, I think dolphin 1.4 should be better than tarako v1.3t. Nobody can accept that FirefoxOS 1.4 with CotexA7@1.2G and 256MB RAM is worse than FirefoxOS 1.3 with CotexA5@1G and 128MB RAM.
I'm not interested in continuing this argument. We've already been through this discussion multiple times & are in jeopardy of damaging one of our partner relationships if we take this.
Flags: needinfo?(jsmith)
(In reply to Jason Smith [:jsmith] from comment #107)
> I'm not interested in continuing this argument. We've already been through
> this discussion multiple times & are in jeopardy of damaging one of our
> partner relationships if we take this.

I think the wording here might be coming across a bit too strong. Let me try reword this & clarify a bit more.

I think there's agreement that we want Dolphin to have solid performance for the call screen in the 1.4 time frame. However, we have to be conscious of the risk vs. reward in triage decisions based on the current time of the release to know if we can safely take a patch into the mainline release and maintain the stability guidelines that match the active timing of the release. At this point in the release, we're post 12 weeks in stability, which places a requirement for the release as a whole that we must keep the influx of instability low since we are at the point where Mozilla is trying to close out the current release. I understand that there's late breaking partner needs that might arrive for a release at this point though, so my suggestion is that SPRD pull the patch here into their own custom gaia branch that they can use for 1.4 development. I think this strategy will satisfy SPRD's needs (e.g. comment 106) and Mozilla's needs for shutting down a release in a post 12 weeks of stabilization.
Hi Etienne,

Can you let us know if there is a 1.4 compatible patch ready and if so, please set request for 1.4 approval?

This is right now blocking partner testing and we'd like to see this move forward.
Thanks.
Flags: needinfo?(etienne)
(In reply to Wayne Chang [:wchang] from comment #109)
> Hi Etienne,
> 
> Can you let us know if there is a 1.4 compatible patch ready and if so,
> please set request for 1.4 approval?
> 

Gabriele will make a 1.4 version today.
This would have been so much easier to uplift back in May...
Flags: needinfo?(etienne)
FYI I'm prepping a v1.4 version of both this bug and bug 992948 which is a dependency. I'm doing this as I need to uplift bug 1002327. That being said since it's still unclear to me if this is approved for v1.4 or not I won't land right away. Worst case if we cannot uplift the dependencies I'll make a v1.4-tailored version of the patch in bug 1002327 though I'm not very fond of the idea.
Please make sure that this patch only goes on 1.4 dolphin branch.
Flags: needinfo?(praghunath)
Anshul

This isn't going in 1.4
Flags: needinfo?(praghunath)
The 1.4 branch is the Dolphin branch now.
(In reply to Gabriele Svelto [:gsvelto] from comment #111)
> FYI I'm prepping a v1.4 version of both this bug and bug 992948 which is a
> dependency. I'm doing this as I need to uplift bug 1002327. That being said
> since it's still unclear to me if this is approved for v1.4 or not I won't
> land right away. Worst case if we cannot uplift the dependencies I'll make a
> v1.4-tailored version of the patch in bug 1002327 though I'm not very fond
> of the idea.

Hi  Gabriele,

Thank you for preparing the 1.4 compatible patch. Once the patch is ready, please set the flag "approval-gaia-v1.4?" for requesting the uplift.
Attached patch WIP cherry-picked patch for v1.4 (obsolete) — Splinter Review
This is the first iteration of the cherry picked patch for v1.4. I had to fix a few conflicts which were all related to bugs that did not affect the callscreen/dialer directly so instead of uplifting them too I fixed the conflicts manually. I'm trying to keep the uplifts to a minimum in order to reduce our testing footprint. BTW this hasn't been tested yet except for the unit-tests which were run locally and seem to pass.

You can find this patch and its dependencies on this branch which can be used for testing the entire uplift:

https://github.com/gabrielesvelto/gaia/tree/callscreen-uplift
HI William,

Mind if you can help to test patch provide by Gabriele in comment 116 with v1.4. Thanks!
Flags: needinfo?(whsu)
Sure!
But, I cannot provide you the result today because I still need to rebuild the Dolphin build.
The actions I will finish is as following.
 - Reproduce this bug on Dolphin or Buri
 - Test the WIP patch
The deadline is 6/30 (UTC+8).
Sorry for any inconvenience.
Thanks!


* Note: Set up Dolphin build need one day effort.
(...keep needinfo whsu.)
Attached patch Cherry-picked patch for v1.4 (obsolete) — Splinter Review
I've cleaned up the patch and updated the PR mentioned in comment 116. Now all linter tests pass and I've also fixed a unit-test related to the TonePlayer. I'm now proceeding with actual device tests on my Hamachi. I'll ask for approval as soon as I'm confident this works as expected.
Attachment #8446670 - Attachment is obsolete: true
Unfortunately the Try-Gaia runs are failing due to harness issues, I suspect this is because we're using the v1.4 branch and not master:

https://tbpl.mozilla.org/?tree=Gaia-Try&rev=25c37fa041ac0638184ca52c4e963d2cfd590ead

This is not very helpful unfortunately.
Tests on the device are not very promising at this stage. The callscreen opens up correctly both while placing and receiving calls and also works properly during the call however once the call ends the attention screen containing it doesn't seem to go away properly. Maybe I'm missing some system / window manager / attention screen patches, will investigate further.
There seemed to have something wrong on the patch.
Please needinfo "whsu@mozilla.com" while the patch is ready to test.
Thanks Gabriele!
Flags: needinfo?(whsu)
I've been a bit stuck today as I couldn't get a working v1.4 build on my hamachi so I'm asking Etienne to have a look; being the original author of the patch maybe he can get past the roadblock I found.

To sum it up, bug 992948 and bug 993018 have been uplifted already and with attachment 8447073 [details] [diff] [review] applied I can get the callscreen to show up correctly both when receiving calls and when making ones. I've tested also some additional functionality such as the keypad, DTMF tones and friends and everything seems to work fine. However once the callscreen gets closed it slides back correctly but then the homescreen remains hidden. One can see only the background. I've looked at the dependencies and I'm not sure if I've missed something or not. I was also wondering if we might have needed to uplift some of the follow-ups such as bug 999478 and bug 997924.
Flags: needinfo?(etienne)
No longer blocks: 1032502
Depends on: 1032502
I've had a quick conversation on IRC with Etienne about this and he suggested a fix for the issue I've been experiencing. There's more to be done here though:

- Etienne mentioned that all of the blocking bugs should be uplifted, AFAIK we haven't done all of them already so that will take time.

- I haven't had a working v1.4 build since yesterday morning which effectively bars me from making any progress here.

- It's still not clear to me if we also have to take some of the follow ups to this bug or not (such as bug 997924).

I'll report ASAP here but all in all I sincerely doubt I'll be able to wrap all this stuff up by today.
Flags: needinfo?(etienne)
Update:
I just opened a pull request [1] uplifting this patch and the needed dependencies all at once.
I tested it on a hamachi, worked well.

While the CI is running on the pull request I'll do some more manual testing on the dolphin.

[1] https://github.com/mozilla-b2g/gaia/pull/21282
Yep, this pull request is the one :)

Now it's a matter of getting a try/travis build to run on it.
I'm off for the day, feel free to merge if needed, try is still running but looks like the only issues are intermittents.

If somebody merges before I get to it tomorrow morning, please update the tracking flag on all the bugs in the pull request.
Attachment #8447073 - Attachment is obsolete: true
Hi William,

Please help to test 1.4 with the PR from comment 129. Thanks.
Flags: needinfo?(whsu)
(In reply to Ivan Tsay (:ITsay) from comment #130)
> Hi William,
> 
> Please help to test 1.4 with the PR from comment 129. Thanks.

Roger that! I will post the result later. :)

(Keep NI? "whsu")
Try is looking good (except the known unit test nightmare, but travis was good for those).

I'll wait for the QA round before merging, just tell me when to press the green button :)
Hi, everyone,

I have verified this patch on master(Flame) and v1.3t(Tarako).
Here is the symptom I saw.

* Result:
 - Flame(256MB version): Sometimes, lag happens on dialer while you are answering a phone call and there have others apps are running in background.
   I think this is because we are not yet merged OOM/LMK related patches on master branch.
 - Tarako: Cannot reproduce this bug.

* Build information:
 @ Flame:
   - Gaia      7a32f4ce7f922ed174946cfe322f3d6df40f18ea
   - Gecko     https://hg.mozilla.org/mozilla-central/rev/06e9a27a6fcc
   - BuildID   20140703160203
   - Version   33.0a1
 @ Tarako:
   - Gaia      64716f3f6dd79c40ae79ccaf96ebd51645694ce3
   - Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/0c6e9a70115e
   - BuildID   20140703164010
   - Version   28.1


* Conclusion: We can merge this patch on v1.4 then fine tune dialer because I think we still need some OOM/LMK related patches to optimize Dolphin device.
Flags: needinfo?(whsu)
(In reply to Etienne Segonzac (:etienne) from comment #132)
> Try is looking good (except the known unit test nightmare, but travis was
> good for those).
> 
> I'll wait for the QA round before merging, just tell me when to press the
> green button :)

Etienne, with Williams' comment can you please merge this today?
Thakns
Flags: needinfo?(etienne)
Comment on attachment 8449618 [details] [review]
PR with dependencies against 1.4

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 #): bug found from partner test
[User impact] if declined: User may not be able to pick up the call since the callscreen will be delayed.
[Testing completed]: Yes in 1.3T, 2.0 official test, and 1.4 local test by QA
[Risk to taking this patch] (and alternatives if risky): Low, since it is rebased and tested by QA
[String changes made]: N/A
Attachment #8449618 - Flags: approval-gaia-v1.4?(lmandel)
Thanks, Ivan and Wayne!
Here is the build information we tested today.
It has merged Etienne's patch. FYI.

* Build information:
 - Gaia      b92011e9e7259f41c67b97df43568cadffa65a2f
 - Gecko     https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/4c05c3fb440d
 - BuildID   20140703000230
 - Version   30.0
Comment on attachment 8449618 [details] [review]
PR with dependencies against 1.4

This patch is known to break l10 in the lockscreen app. (See bug 1032502.) The solution being proposed in bug 1032502 is a build time hack. We should resolve this issue before landing this patch and causing a regression on 1.4.
Attachment #8449618 - Flags: approval-gaia-v1.4?(lmandel) → approval-gaia-v1.4-
(In reply to Lawrence Mandel [:lmandel] from comment #138)
> Comment on attachment 8449618 [details] [review]
> PR with dependencies against 1.4
> 
> This patch is known to break l10 in the lockscreen app. (See bug 1032502.)
> The solution being proposed in bug 1032502 is a build time hack. We should
> resolve this issue before landing this patch and causing a regression on 1.4.

Should I back this out right away?
(it landed on 1.4 this morning with the right dependencies)

Who's the ultimate decision maker here?

I'm sorry about the mess, but look at the bug history (which does not include all the email back an forth) we're maybe 6 "out of process over-ruling of the previous decision" in already.
We've been flushing engineering resources down the toilet and everybody's patience is draining very quickly...
Flags: needinfo?(lmandel)
(In reply to Etienne Segonzac (:etienne) from comment #139)
> (In reply to Lawrence Mandel [:lmandel] from comment #138)
> Should I back this out right away?
> (it landed on 1.4 this morning with the right dependencies)
Given that this has already landed and is going to have to land on 1.4, let's pull :drs into this bug (he's been fixing things in bug 1032502) to see if he has a preference for how to proceed.

> Who's the ultimate decision maker here?
Kevin Hu is the release owner and ultimate decision maker for 1.4.

> I'm sorry about the mess, but look at the bug history (which does not
> include all the email back an forth) we're maybe 6 "out of process
> over-ruling of the previous decision" in already.
> We've been flushing engineering resources down the toilet and everybody's
> patience is draining very quickly...
Completely understandable. I want to put this one to bed as well.
Flags: needinfo?(lmandel)
Flags: needinfo?(khu)
Flags: needinfo?(drs+bugzilla)
Sorry Etienne, I should have mentioned this at our standup, but I thought I would be ready to land my fixes as soon as this landed on v1.4. I think it's best if we leave this as-is instead of backing it out and I'll get bug 1032502 and bug 1030546 landed as well ASAP.
Flags: needinfo?(drs+bugzilla)
Comment on attachment 8449618 [details] [review]
PR with dependencies against 1.4

Change to 1.4+ based on drs response in comment 141. To be clear though, bug 1032502 needs to land in order for this to stick on 1.4.
Attachment #8449618 - Flags: approval-gaia-v1.4- → approval-gaia-v1.4+
(In reply to Doug Sherk (:drs) from comment #141)
> I think it's
> best if we leave this as-is instead of backing it out 
+1 to :drs's comment 141 here.
Flags: needinfo?(khu)
Depends on: 1030912
Flags: needinfo?(kevin)
Depends on: 1056614
Depends on: 1007598
Duplicate of this bug: 1020263
See Also: → 1226195
You need to log in before you can comment on or make changes to this bug.