738530 - Move PowerManager from shell.js to gaia

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Kan-Ru Chen [:kanru] (UTC+9)]

This should be handled by gaia. We will need to know the idle time though.

Comment 1

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

Attachment: Patch v1

Let's speed up the progress of this bug so by the time bug 715041 is ready they can be checked in altogether.

Comment 2

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

Hum, looks like this code do more than listen to idle API.

@Kanru, does the ScreenManager in Gaia System need to handle wake lock state before deciding whether to turn off the screen? If so I will do that in https://github.com/mozilla-b2g/gaia/pull/1890

Comment 3

[Kan-Ru Chen [:kanru] (UTC+9)]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TW) from comment #2)
> Hum, looks like this code do more than listen to idle API.
> 
> @Kanru, does the ScreenManager in Gaia System need to handle wake lock state
> before deciding whether to turn off the screen? If so I will do that in
> https://github.com/mozilla-b2g/gaia/pull/1890

Yes, it's the core logic of the PowerManager ;-)

BTW, your patch has unrelated changes.

Comment 4

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

(In reply to Kan-Ru Chen [:kanru] from comment #3)
> (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TW) from comment #2)
> > Hum, looks like this code do more than listen to idle API.
> > 
> > @Kanru, does the ScreenManager in Gaia System need to handle wake lock state
> > before deciding whether to turn off the screen? If so I will do that in
> > https://github.com/mozilla-b2g/gaia/pull/1890
> 
> Yes, it's the core logic of the PowerManager ;-)

Thanks

> BTW, your patch has unrelated changes.

White space cleanup from the editor ... Will get rid of them before check-in (or it doesn't matter?)

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

I'm fine with this change but do you mind if I keep it unreviewed for now until bug 715041 is resolved? Otherwise I will forgot it!

Comment 6

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

(In reply to Vivien Nicolas (:vingtetun) from comment #5)
> I'm fine with this change but do you mind if I keep it unreviewed for now
> until bug 715041 is resolved? Otherwise I will forgot it!

I don't know how Bugzilla and checkin-needed works but I think the better approach would be have a "Patch to commit with bug 715041" ready and remind people on that bug to checked in the two together when it's finished. It is better or worse?

Comment 7

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

Attachment: Patch to commit with bug 715041

Comment 8

[Ryan VanderMeulen [:RyanVM]]

checkin-needed implies that you want the patch checked in *now*. If I'm reading this correctly, you don't want this landed until bug 715041 is also ready to land. Assuming that's the case, hold off on adding checkin-needed until that time.

Comment 9

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

(In reply to Ryan VanderMeulen from comment #8)
> checkin-needed implies that you want the patch checked in *now*. If I'm
> reading this correctly, you don't want this landed until bug 715041 is also
> ready to land. Assuming that's the case, hold off on adding checkin-needed
> until that time.

bug 715041 *is* landed. Thanks.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Silly timing on that one :-). The patch was bitrotted a bit, so take a look just to make sure I didn't mess anything up when fixing it.

https://hg.mozilla.org/integration/mozilla-inbound/rev/ef8319d2f498

Comment 11

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

(In reply to Ryan VanderMeulen from comment #10)
> Silly timing on that one :-). The patch was bitrotted a bit, so take a look
> just to make sure I didn't mess anything up when fixing it.
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/ef8319d2f498

Thank you very much :)

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ef8319d2f498