Skip to content
This repository has been archived by the owner on Oct 18, 2018. It is now read-only.

Bug 863103 - Add BackgroundService runIntentInService() and WakeLock #307

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Bug 863103 - Add BackgroundService runIntentInService() and WakeLock #307

wants to merge 1 commit into from

Conversation

cpeterso
Copy link
Contributor

@rnewman r?

After reviewing Google's Cloud Messaging (GCM) implementation, here are two BackgroundService additions that may be useful:

  1. Add runIntentInService() utility method to ensure the original Intent's extras are copied to rebroadcast Intent:

    https://code.google.com/p/gcm/source/browse/gcm-client/src/com/google/android/gcm/GCMBaseIntentService.java?r=3f8285f108caecf9ee040cdadda3a024b81f7e3e#268

  2. Add WakeLock to ensure BackgroundService receives the Intent after the BroadcastReceiver returns.

GCM releases its WakeLock at the end of onHandleIntent():

https://code.google.com/p/gcm/source/browse/gcm-client/src/com/google/android/gcm/GCMBaseIntentService.java?r=3f8285f108caecf9ee040cdadda3a024b81f7e3e#241

Can we safely assume that Android will always call our onDestroy() method?� If not, then we risk leaking a WakeLock.

This change is tricky because it relies on every subclass calling super.onHandleIntent() and super.onDestroy() (if the subclass has overridden those methods). Also, we might be able to remove the ugly EXTRA_RELEASE_WAKE_LOCK extra if we assume BackgroundServices are always launched from runIntentForService() and thus called acquireWakeLock().

@rnewman
Copy link
Contributor

rnewman commented Apr 19, 2013

onDestroy is documented as being called once a Service is no longer being consumed. So there should be no risk of leaking a WakeLock.

@rnewman
Copy link
Contributor

rnewman commented Apr 19, 2013

r+ on the runIntent stuff. f+ on the WakeLock changes, but I think there are some implementation improvements that can be made. Still, hurrah for rigor improvements…

@rnewman
Copy link
Contributor

rnewman commented Apr 30, 2013

Chris, want to spend two minutes rebasing this, even if you don't want to wrestle with the WakeLock stuff yet?

@cpeterso
Copy link
Contributor Author

Richard, here is a simpler proposal that uses (less efficient, but less error-prone) timed WakeLocks. Comments in the .java file explain more. Note that this code is not well-tested yet! <:)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants