1234731 - Update lockscreen clock when screen turns on in a more direct manner

Status: RESOLVED FIXED

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

Description

[Ting-Yu Chou [:ting] (away)]

When screen turns on, the lockscreen clock gets updated after several asynchronous cycles that transfer from LockScreenClockWidgetSuspend to LockScreenClockWidgetTick state.

We should be able to update it immediately. In profile [1], it takes >60ms from LockScreenClockWidgetSuspend receiveing screenchange to updateClock().

[1] http://people.mozilla.org/~bgirard/cleopatra/#report=f3643e6d68c6d50ca04e09a08260d5f8ac171998

Comment 1

[Ting-Yu Chou [:ting] (away)]

What if we call updateClock() before transfering to LockScreenClockWidgetTick in LockScreenClockWidgetSuspend.handleSourceEvent() for screenchange event?

Comment 2

[Greg Weng [:snowmantw][:gweng][:λ]]

Attachment: Patch

We should profile it before we go further.

Comment 3

[Ting-Yu Chou [:ting] (away)]

Somehow attachment 8701317 [details] [review] makes the clock won't get updated, updateClock() is not called when screen on.

Comment 4

[Ting-Yu Chou [:ting] (away)]

The profile [1] with attachment 8701317 [details] [review] shows from transferToTick() to 1st nsRefreshDriver::Tick() now takes ~40ms, and it was ~90ms.

[1] http://people.mozilla.org/~bgirard/cleopatra/#report=0fa802172065d6958dad8014fd332ead851d7777

Comment 5

[Greg Weng [:snowmantw][:gweng][:λ]]

Comment on attachment 8701317 [details] [review]
Patch

Test works well on TaskCluster:

https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=793e41fec92307d0fc2a6db2e88d87c822af6aa0

Fixes:

* Call `updateClock` before the Suspend state transfers to Tick state, so that the rendering task will be queued in asynchronous queue earlier
* Added one more entry of Tick state to avoid do initialization twice
* Regards the condition for the entry, added a `previousState` property in the base component
* Added comments to describe the context and patch

Comment 6

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8701317 [details] [review]
Patch

This is sad. Maybe we don't really need to make the Clock state machine async at all?

Comment 7

[Greg Weng [:snowmantw][:gweng][:λ]]

It requires to re-write a specialized version of state machine with all the features we want during our long discussion. I feel maybe it's do-able but not worth to do that in this patch, for the addressed issue.

Comment 8

[Greg Weng [:snowmantw][:gweng][:λ]]

Landed the patch first. If we need further actions we can fire other bugs for more discussions.

master: https://github.com/mozilla-b2g/gaia/commit/71c56fafb76a12745a1b2c3e65b4868b64a297ac