1068104 - Wake up the screen when all calls are ended

Status: RESOLVED FIXED

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

Description

[Doug Sherk (:drs) (inactive)]

Currently, the screen is only woken up when a call comes. It appears (though this is liable to be incorrect) that the flow for this is:
get a call -> open attnwindow -> signal system attnwindow handler -> turn screen on -> callscreen app shown -> callscreen app acquires screen lock

The "turn screen on" step happens here:
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/screen_manager.js#L206

We should add hooks here for callschanged event to turn on the screen:
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/screen_manager.js#L268

Comment 1

[Doug Sherk (:drs) (inactive)]

Attachment: Wake up the screen when all calls are ended.

PR: https://github.com/mozilla-b2g/gaia/pull/24856

It turns out this was really simple.

There is one potential point of contention: I'm not sure if it's a valid assumption that `calls.length === 0` means that all calls have ended. Is it possible that we get a 'callschanged' while `calls.length === 0` both before and after the event? I might just be paranoid here.

Comment 2

[Doug Sherk (:drs) (inactive)]

Attachment: Get jslint passing for affected files.

Also on the PR as a separate commit.

Comment 3

[Etienne Segonzac (:etienne)]

Comment on attachment 8500728 [details] [diff] [review]
Wake up the screen when all calls are ended.

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

that was easy :)

Comment 4

[Etienne Segonzac (:etienne)]

Comment on attachment 8500729 [details] [diff] [review]
Get jslint passing for affected files.

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

thanks for splitting. made things much clearer.

Comment 5

[Etienne Segonzac (:etienne)]

(In reply to Doug Sherk (:drs) from comment #1)
> There is one potential point of contention: I'm not sure if it's a valid
> assumption that `calls.length === 0` means that all calls have ended. Is it
> possible that we get a 'callschanged' while `calls.length === 0` both before
> and after the event? I might just be paranoid here.

As long as we have the conferenceGroup check I'm pretty sure we're safe.

Comment 6

[Doug Sherk (:drs) (inactive)]

https://github.com/mozilla-b2g/gaia/commit/9d95fa88a643159eb12891f3a572bd170911da2a

Comment 7

[Doug Sherk (:drs) (inactive)]

Reverted in https://github.com/mozilla-b2g/gaia/commit/256f2c8c5dbfa5b410859a6fef4801c414a0ea34 for causing bug 1081068. We're going to re-land this once bug 1081811 lands, which should prevent bug 1081068 from happening again.

Comment 8

[Doug Sherk (:drs) (inactive)]

https://github.com/mozilla-b2g/gaia/commit/d893a9b971a0f3ee48e5a57dca516837d92cf52b

Re-landed now that bug 1081811 is fixed. I dogfooded this for a few hours and the screen didn't randomly turn on at all.

Johan, I'd like you to be aware of this. To save you from reading this thread: when this bug originally landed, the screen would turn on at fairly random times. Now that bug 1081811 is fixed, it shouldn't do that anymore. If anyone reports any bugs that sound similar to this, please let me know. Thanks!

Comment 9

[Johan Lorenzo [:jlorenzo]]

Acknowledged. I'll keep in mind that bug for the next weeks.