From a09e1e7662e223b9ec46dbaae637bf73894b5061 Mon Sep 17 00:00:00 2001 From: dosiecki Date: Mon, 6 Aug 2018 03:05:15 -0700 Subject: [PATCH] Rework sync service as JobService and bump target API to 26. --- clients/android/NewsBlur/AndroidManifest.xml | 12 +- .../src/com/newsblur/activity/NbActivity.java | 4 +- .../fragment/LoginAsDialogFragment.java | 1 - .../src/com/newsblur/fragment/NbFragment.java | 5 +- .../com/newsblur/service/BootReceiver.java | 25 +- .../com/newsblur/service/CleanupService.java | 33 +-- .../service/ImagePrefetchService.java | 147 ++++++------ .../com/newsblur/service/NBSyncService.java | 214 ++++++++++-------- .../newsblur/service/OriginalTextService.java | 35 +-- .../service/ServiceScheduleReceiver.java | 17 -- .../src/com/newsblur/service/SubService.java | 71 +++--- .../com/newsblur/service/UnreadsService.java | 40 ++-- .../src/com/newsblur/util/FeedUtils.java | 10 +- .../src/com/newsblur/util/PrefsUtils.java | 3 - 14 files changed, 284 insertions(+), 333 deletions(-) delete mode 100644 clients/android/NewsBlur/src/com/newsblur/service/ServiceScheduleReceiver.java diff --git a/clients/android/NewsBlur/AndroidManifest.xml b/clients/android/NewsBlur/AndroidManifest.xml index 03705924b..976ddcfac 100644 --- a/clients/android/NewsBlur/AndroidManifest.xml +++ b/clients/android/NewsBlur/AndroidManifest.xml @@ -1,12 +1,12 @@ + android:versionCode="159" + android:versionName="8.0.0" > + android:targetSdkVersion="26" /> @@ -137,7 +137,9 @@ - + @@ -145,8 +147,6 @@ - - diff --git a/clients/android/NewsBlur/src/com/newsblur/activity/NbActivity.java b/clients/android/NewsBlur/src/com/newsblur/activity/NbActivity.java index 54c2eb469..767396cbf 100644 --- a/clients/android/NewsBlur/src/com/newsblur/activity/NbActivity.java +++ b/clients/android/NewsBlur/src/com/newsblur/activity/NbActivity.java @@ -5,7 +5,6 @@ import android.os.Bundle; import android.support.v4.app.FragmentActivity; import android.widget.Toast; -import com.newsblur.service.NBSyncService; import com.newsblur.util.FeedUtils; import com.newsblur.util.PrefsUtils; import com.newsblur.util.PrefConstants.ThemeValue; @@ -118,8 +117,7 @@ public class NbActivity extends FragmentActivity { * Pokes the sync service to perform any pending sync actions. */ protected void triggerSync() { - Intent i = new Intent(this, NBSyncService.class); - startService(i); + FeedUtils.triggerSync(this); } /** diff --git a/clients/android/NewsBlur/src/com/newsblur/fragment/LoginAsDialogFragment.java b/clients/android/NewsBlur/src/com/newsblur/fragment/LoginAsDialogFragment.java index 8a4ba94d0..ad3047685 100644 --- a/clients/android/NewsBlur/src/com/newsblur/fragment/LoginAsDialogFragment.java +++ b/clients/android/NewsBlur/src/com/newsblur/fragment/LoginAsDialogFragment.java @@ -80,7 +80,6 @@ public class LoginAsDialogFragment extends DialogFragment { @Override protected void onPostExecute(Boolean result) { if (result) { - NBSyncService.resumeFromInterrupt(); Intent startMain = new Intent(activity, Main.class); activity.startActivity(startMain); } else { diff --git a/clients/android/NewsBlur/src/com/newsblur/fragment/NbFragment.java b/clients/android/NewsBlur/src/com/newsblur/fragment/NbFragment.java index 994a04a24..81f42c621 100644 --- a/clients/android/NewsBlur/src/com/newsblur/fragment/NbFragment.java +++ b/clients/android/NewsBlur/src/com/newsblur/fragment/NbFragment.java @@ -4,7 +4,7 @@ import android.app.Activity; import android.content.Intent; import android.support.v4.app.Fragment; -import com.newsblur.service.NBSyncService; +import com.newsblur.util.FeedUtils; public class NbFragment extends Fragment { @@ -14,8 +14,7 @@ public class NbFragment extends Fragment { protected void triggerSync() { Activity a = getActivity(); if (a != null) { - Intent i = new Intent(a, NBSyncService.class); - a.startService(i); + FeedUtils.triggerSync(a); } } diff --git a/clients/android/NewsBlur/src/com/newsblur/service/BootReceiver.java b/clients/android/NewsBlur/src/com/newsblur/service/BootReceiver.java index 09942f01d..6269e7572 100644 --- a/clients/android/NewsBlur/src/com/newsblur/service/BootReceiver.java +++ b/clients/android/NewsBlur/src/com/newsblur/service/BootReceiver.java @@ -1,13 +1,14 @@ package com.newsblur.service; -import android.app.AlarmManager; -import android.app.PendingIntent; +import android.app.job.JobInfo; +import android.app.job.JobScheduler; import android.content.BroadcastReceiver; +import android.content.ComponentName; import android.content.Context; import android.content.Intent; -import android.util.Log; import com.newsblur.util.AppConstants; +import com.newsblur.service.NBSyncService; /** * First receiver in the chain that starts with the device. Simply schedules another broadcast @@ -17,20 +18,18 @@ public class BootReceiver extends BroadcastReceiver { @Override public void onReceive(Context context, Intent intent) { - Log.d(this.getClass().getName(), "triggering sync service from device boot"); + com.newsblur.util.Log.d(this, "triggering sync service from device boot"); scheduleSyncService(context); } public static void scheduleSyncService(Context context) { - Log.d(BootReceiver.class.getName(), "scheduling sync service"); - - // wake up to check if a sync is needed less often than necessary to compensate for execution time - long interval = AppConstants.BG_SERVICE_CYCLE_MILLIS; - - AlarmManager alarmManager = (AlarmManager) context.getSystemService(Context.ALARM_SERVICE); - Intent i = new Intent(context, ServiceScheduleReceiver.class); - PendingIntent pi = PendingIntent.getBroadcast(context, 0, i, PendingIntent.FLAG_CANCEL_CURRENT); - alarmManager.setInexactRepeating(AlarmManager.ELAPSED_REALTIME_WAKEUP, interval, interval, pi); + com.newsblur.util.Log.d(BootReceiver.class.getName(), "scheduling sync service"); + JobInfo.Builder builder = new JobInfo.Builder(1, new ComponentName(context, NBSyncService.class)); + builder.setPeriodic(AppConstants.BG_SERVICE_CYCLE_MILLIS); + builder.setRequiredNetworkType(JobInfo.NETWORK_TYPE_ANY); + builder.setPersisted(true); + JobScheduler sched = (JobScheduler) context.getSystemService(Context.JOB_SCHEDULER_SERVICE); + sched.schedule(builder.build()); } } diff --git a/clients/android/NewsBlur/src/com/newsblur/service/CleanupService.java b/clients/android/NewsBlur/src/com/newsblur/service/CleanupService.java index 7fe0351db..683783bd7 100644 --- a/clients/android/NewsBlur/src/com/newsblur/service/CleanupService.java +++ b/clients/android/NewsBlur/src/com/newsblur/service/CleanupService.java @@ -6,7 +6,7 @@ import com.newsblur.util.PrefsUtils; public class CleanupService extends SubService { - private static volatile boolean Running = false; + public static boolean activelyRunning = false; public CleanupService(NBSyncService parent) { super(parent); @@ -14,16 +14,17 @@ public class CleanupService extends SubService { @Override protected void exec() { - gotWork(); - if (PrefsUtils.isTimeToCleanup(parent)) { - com.newsblur.util.Log.d(this.getClass().getName(), "cleaning up old stories"); - parent.dbHelper.cleanupVeryOldStories(); - if (!PrefsUtils.isKeepOldStories(parent)) { - parent.dbHelper.cleanupReadStories(); - } - PrefsUtils.updateLastCleanupTime(parent); + if (!PrefsUtils.isTimeToCleanup(parent)) return; + + activelyRunning = true; + + com.newsblur.util.Log.d(this.getClass().getName(), "cleaning up old stories"); + parent.dbHelper.cleanupVeryOldStories(); + if (!PrefsUtils.isKeepOldStories(parent)) { + parent.dbHelper.cleanupReadStories(); } + PrefsUtils.updateLastCleanupTime(parent); com.newsblur.util.Log.d(this.getClass().getName(), "cleaning up old story texts"); parent.dbHelper.cleanupStoryText(); @@ -42,19 +43,9 @@ public class CleanupService extends SubService { com.newsblur.util.Log.d(this.getClass().getName(), "cleaning up thumbnail cache"); FileCache thumbCache = FileCache.asThumbnailCache(parent); thumbCache.cleanupUnusedAndOld(parent.dbHelper.getAllStoryThumbnails(), PrefsUtils.getMaxCachedAgeMillis(parent)); - } - public static boolean running() { - return Running; + activelyRunning = false; } - @Override - protected void setRunning(boolean running) { - Running = running; - } - @Override - protected boolean isRunning() { - return Running; - } - + } diff --git a/clients/android/NewsBlur/src/com/newsblur/service/ImagePrefetchService.java b/clients/android/NewsBlur/src/com/newsblur/service/ImagePrefetchService.java index d05a0e8b9..b15746f73 100644 --- a/clients/android/NewsBlur/src/com/newsblur/service/ImagePrefetchService.java +++ b/clients/android/NewsBlur/src/com/newsblur/service/ImagePrefetchService.java @@ -12,7 +12,7 @@ import java.util.Set; public class ImagePrefetchService extends SubService { - private static volatile boolean Running = false; + public static boolean activelyRunning = false; FileCache storyImageCache; FileCache thumbnailCache; @@ -33,77 +33,77 @@ public class ImagePrefetchService extends SubService { @Override protected void exec() { - if (!PrefsUtils.isImagePrefetchEnabled(parent)) return; - if (!PrefsUtils.isBackgroundNetworkAllowed(parent)) return; + activelyRunning = true; + try { + if (!PrefsUtils.isImagePrefetchEnabled(parent)) return; + if (!PrefsUtils.isBackgroundNetworkAllowed(parent)) return; - gotWork(); + while (StoryImageQueue.size() > 0) { + if (! PrefsUtils.isImagePrefetchEnabled(parent)) return; + if (! PrefsUtils.isBackgroundNetworkAllowed(parent)) return; - while (StoryImageQueue.size() > 0) { - if (! PrefsUtils.isImagePrefetchEnabled(parent)) return; - if (! PrefsUtils.isBackgroundNetworkAllowed(parent)) return; - - startExpensiveCycle(); - com.newsblur.util.Log.d(this, "story images to prefetch: " + StoryImageQueue.size()); - // on each batch, re-query the DB for images associated with yet-unread stories - // this is a bit expensive, but we are running totally async at a really low priority - Set unreadImages = parent.dbHelper.getAllStoryImages(); - Set fetchedImages = new HashSet(); - Set batch = new HashSet(AppConstants.IMAGE_PREFETCH_BATCH_SIZE); - batchloop: for (String url : StoryImageQueue) { - batch.add(url); - if (batch.size() >= AppConstants.IMAGE_PREFETCH_BATCH_SIZE) break batchloop; - } - try { - fetchloop: for (String url : batch) { - if (parent.stopSync()) break fetchloop; - // dont fetch the image if the associated story was marked read before we got to it - if (unreadImages.contains(url)) { - if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "prefetching image: " + url); - storyImageCache.cacheFile(url); - } - fetchedImages.add(url); + startExpensiveCycle(); + com.newsblur.util.Log.d(this, "story images to prefetch: " + StoryImageQueue.size()); + // on each batch, re-query the DB for images associated with yet-unread stories + // this is a bit expensive, but we are running totally async at a really low priority + Set unreadImages = parent.dbHelper.getAllStoryImages(); + Set fetchedImages = new HashSet(); + Set batch = new HashSet(AppConstants.IMAGE_PREFETCH_BATCH_SIZE); + batchloop: for (String url : StoryImageQueue) { + batch.add(url); + if (batch.size() >= AppConstants.IMAGE_PREFETCH_BATCH_SIZE) break batchloop; } - } finally { - StoryImageQueue.removeAll(fetchedImages); - com.newsblur.util.Log.d(this, "story images fetched: " + fetchedImages.size()); - gotWork(); - } - } - - if (parent.stopSync()) return; - - while (ThumbnailQueue.size() > 0) { - if (! PrefsUtils.isImagePrefetchEnabled(parent)) return; - if (! PrefsUtils.isBackgroundNetworkAllowed(parent)) return; - - startExpensiveCycle(); - com.newsblur.util.Log.d(this, "story thumbs to prefetch: " + StoryImageQueue.size()); - // on each batch, re-query the DB for images associated with yet-unread stories - // this is a bit expensive, but we are running totally async at a really low priority - Set unreadImages = parent.dbHelper.getAllStoryThumbnails(); - Set fetchedImages = new HashSet(); - Set batch = new HashSet(AppConstants.IMAGE_PREFETCH_BATCH_SIZE); - batchloop: for (String url : ThumbnailQueue) { - batch.add(url); - if (batch.size() >= AppConstants.IMAGE_PREFETCH_BATCH_SIZE) break batchloop; - } - try { - fetchloop: for (String url : batch) { - if (parent.stopSync()) break fetchloop; - // dont fetch the image if the associated story was marked read before we got to it - if (unreadImages.contains(url)) { - if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "prefetching thumbnail: " + url); - thumbnailCache.cacheFile(url); + try { + fetchloop: for (String url : batch) { + if (parent.stopSync()) break fetchloop; + // dont fetch the image if the associated story was marked read before we got to it + if (unreadImages.contains(url)) { + if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "prefetching image: " + url); + storyImageCache.cacheFile(url); + } + fetchedImages.add(url); } - fetchedImages.add(url); + } finally { + StoryImageQueue.removeAll(fetchedImages); + com.newsblur.util.Log.d(this, "story images fetched: " + fetchedImages.size()); } - } finally { - ThumbnailQueue.removeAll(fetchedImages); - com.newsblur.util.Log.d(this, "story thumbs fetched: " + fetchedImages.size()); - gotWork(); } + + if (parent.stopSync()) return; + + while (ThumbnailQueue.size() > 0) { + if (! PrefsUtils.isImagePrefetchEnabled(parent)) return; + if (! PrefsUtils.isBackgroundNetworkAllowed(parent)) return; + + startExpensiveCycle(); + com.newsblur.util.Log.d(this, "story thumbs to prefetch: " + StoryImageQueue.size()); + // on each batch, re-query the DB for images associated with yet-unread stories + // this is a bit expensive, but we are running totally async at a really low priority + Set unreadImages = parent.dbHelper.getAllStoryThumbnails(); + Set fetchedImages = new HashSet(); + Set batch = new HashSet(AppConstants.IMAGE_PREFETCH_BATCH_SIZE); + batchloop: for (String url : ThumbnailQueue) { + batch.add(url); + if (batch.size() >= AppConstants.IMAGE_PREFETCH_BATCH_SIZE) break batchloop; + } + try { + fetchloop: for (String url : batch) { + if (parent.stopSync()) break fetchloop; + // dont fetch the image if the associated story was marked read before we got to it + if (unreadImages.contains(url)) { + if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "prefetching thumbnail: " + url); + thumbnailCache.cacheFile(url); + } + fetchedImages.add(url); + } + } finally { + ThumbnailQueue.removeAll(fetchedImages); + com.newsblur.util.Log.d(this, "story thumbs fetched: " + fetchedImages.size()); + } + } + } finally { + activelyRunning = false; } - } public void addUrl(String url) { @@ -118,27 +118,10 @@ public class ImagePrefetchService extends SubService { return (StoryImageQueue.size() + ThumbnailQueue.size()); } - @Override - public boolean haveWork() { - return (getPendingCount() > 0); - } - public static void clear() { StoryImageQueue.clear(); ThumbnailQueue.clear(); } - public static boolean running() { - return Running; - } - @Override - protected void setRunning(boolean running) { - Running = running; - } - @Override - protected boolean isRunning() { - return Running; - } - } diff --git a/clients/android/NewsBlur/src/com/newsblur/service/NBSyncService.java b/clients/android/NewsBlur/src/com/newsblur/service/NBSyncService.java index 72195f33c..6046f0411 100644 --- a/clients/android/NewsBlur/src/com/newsblur/service/NBSyncService.java +++ b/clients/android/NewsBlur/src/com/newsblur/service/NBSyncService.java @@ -1,6 +1,8 @@ package com.newsblur.service; import android.app.Service; +import android.app.job.JobParameters; +import android.app.job.JobService; import android.content.ComponentCallbacks2; import android.content.ContentValues; import android.content.Context; @@ -9,7 +11,6 @@ import android.database.Cursor; import android.os.IBinder; import android.os.PowerManager; import android.os.Process; -import android.util.Log; import com.newsblur.R; import com.newsblur.activity.NbActivity; @@ -31,6 +32,7 @@ import com.newsblur.util.AppConstants; import com.newsblur.util.DefaultFeedView; import com.newsblur.util.FeedSet; import com.newsblur.util.FileCache; +import com.newsblur.util.Log; import com.newsblur.util.NetworkUtils; import com.newsblur.util.NotificationUtils; import com.newsblur.util.PrefsUtils; @@ -66,9 +68,9 @@ import java.util.concurrent.TimeUnit; * after sync operations are performed. Activities can then refresh views and * query this class to see if progress indicators should be active. */ -public class NBSyncService extends Service { +public class NBSyncService extends JobService { - private static final Object WAKELOCK_MUTEX = new Object(); + private static final Object COMPLETION_CALLBACKS_MUTEX = new Object(); private static final Object PENDING_FEED_MUTEX = new Object(); private volatile static boolean ActionsRunning = false; @@ -88,7 +90,6 @@ public class NBSyncService extends Service { public volatile static Boolean isPremium = null; public volatile static Boolean isStaff = null; - private volatile static boolean isMemoryLow = false; private static long lastFeedCount = 0L; private static long lastFFConnMillis = 0L; private static long lastFFReadMillis = 0L; @@ -130,16 +131,18 @@ public class NBSyncService extends Service { Set disabledFeedIds = new HashSet(); private ExecutorService primaryExecutor; + private List outstandingStartIds = new ArrayList(); + private List outstandingStartParams = new ArrayList(); + private boolean mainSyncRunning = false; CleanupService cleanupService; OriginalTextService originalTextService; UnreadsService unreadsService; ImagePrefetchService imagePrefetchService; + private boolean forceHalted = false; - PowerManager.WakeLock wl = null; APIManager apiManager; BlurDatabaseHelper dbHelper; FileCache iconCache; - private int lastStartIdCompleted = -1; /** The time of the last hard API failure we encountered. Used to implement back-off so that the sync service doesn't spin in the background chewing up battery when the API is unavailable. */ @@ -152,10 +155,6 @@ public class NBSyncService extends Service { super.onCreate(); com.newsblur.util.Log.d(this, "onCreate"); HaltNow = false; - PowerManager pm = (PowerManager) getApplicationContext().getSystemService(POWER_SERVICE); - wl = pm.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, this.getClass().getSimpleName()); - wl.setReferenceCounted(true); - primaryExecutor = Executors.newFixedThreadPool(1); } @@ -164,7 +163,7 @@ public class NBSyncService extends Service { * parts of construction in onCreate, but save them for when we are in our own thread. */ private void finishConstruction() { - if (apiManager == null) { + if ((apiManager == null) || (dbHelper == null)) { apiManager = new APIManager(this); dbHelper = new BlurDatabaseHelper(this); iconCache = FileCache.asIconCache(this); @@ -177,42 +176,89 @@ public class NBSyncService extends Service { } /** - * Called serially, once per "start" of the service. This serves as a wakeup call - * that the service should check for outstanding work. + * Kickoff hook for when we are started via Context.startService() */ @Override public int onStartCommand(Intent intent, int flags, final int startId) { + com.newsblur.util.Log.d(this, "onStartCommand"); // only perform a sync if the app is actually running or background syncs are enabled if ((NbActivity.getActiveActivityCount() > 0) || PrefsUtils.isBackgroundNeeded(this)) { + HaltNow = false; // Services actually get invoked on the main system thread, and are not // allowed to do tangible work. We spawn a thread to do so. Runnable r = new Runnable() { public void run() { - doSync(startId); + mainSyncRunning = true; + doSync(); + mainSyncRunning = false; + // record the startId so when the sync thread and all sub-service threads finish, + // we can report that this invocation completed. + synchronized (COMPLETION_CALLBACKS_MUTEX) {outstandingStartIds.add(startId);} + checkCompletion(); } }; primaryExecutor.execute(r); } else { - com.newsblur.util.Log.d(this, "Skipping sync: app not active and background sync not enabled."); - stopSelf(startId); + com.newsblur.util.Log.i(this, "Skipping sync: app not active and background sync not enabled."); + synchronized (COMPLETION_CALLBACKS_MUTEX) {outstandingStartIds.add(startId);} + checkCompletion(); } - // indicate to the system that the service should be alive when started, but // needn't necessarily persist under memory pressure return Service.START_NOT_STICKY; } + /** + * Kickoff hook for when we are started via a JobScheduler + */ + @Override + public boolean onStartJob(final JobParameters params) { + com.newsblur.util.Log.d(this, "onStartJob"); + // only perform a sync if the app is actually running or background syncs are enabled + if ((NbActivity.getActiveActivityCount() > 0) || PrefsUtils.isBackgroundNeeded(this)) { + HaltNow = false; + // Services actually get invoked on the main system thread, and are not + // allowed to do tangible work. We spawn a thread to do so. + Runnable r = new Runnable() { + public void run() { + mainSyncRunning = true; + doSync(); + mainSyncRunning = false; + // record the JobParams so when the sync thread and all sub-service threads finish, + // we can report that this invocation completed. + synchronized (COMPLETION_CALLBACKS_MUTEX) {outstandingStartParams.add(params);} + checkCompletion(); + } + }; + primaryExecutor.execute(r); + } else { + com.newsblur.util.Log.d(this, "Skipping sync: app not active and background sync not enabled."); + synchronized (COMPLETION_CALLBACKS_MUTEX) {outstandingStartParams.add(params);} + checkCompletion(); + } + return true; // indicate that we are async + } + + @Override + public boolean onStopJob(JobParameters params) { + com.newsblur.util.Log.d(this, "onStopJob"); + HaltNow = true; + // return false to indicate that we don't necessarily need re-invocation ahead of schedule. + // background syncs can pick up where the last one left off and forground syncs aren't + // run via cancellable JobScheduler invocations. + return false; + } + /** * Do the actual work of syncing. */ - private synchronized void doSync(final int startId) { + private synchronized void doSync() { try { if (HaltNow) return; - incrementRunningChild(); finishConstruction(); - if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "starting primary sync"); + Log.d(this, "starting primary sync"); if (NbActivity.getActiveActivityCount() < 1) { // if the UI isn't running, politely run at background priority @@ -250,16 +296,16 @@ public class NBSyncService extends Service { NbActivity.updateAllActivities(NbActivity.UPDATE_DB_READY); // async text requests might have been queued up and are being waiting on by the live UI. give them priority - originalTextService.start(startId); + originalTextService.start(); // first: catch up syncActions(); // if MD is stale, sync it first so unreads don't get backwards with story unread state - syncMetadata(startId); + syncMetadata(); // handle fetching of stories that are actively being requested by the live UI - syncPendingFeedStories(startId); + syncPendingFeedStories(); // re-apply the local state of any actions executed before local UI interaction finishActions(); @@ -268,20 +314,18 @@ public class NBSyncService extends Service { checkRecounts(); // async story and image prefetch are lower priority and don't affect active reading, do them last - unreadsService.start(startId); - imagePrefetchService.start(startId); + unreadsService.start(); + imagePrefetchService.start(); // almost all notifications will be pushed after the unreadsService gets new stories, but double-check // here in case some made it through the feed sync loop first pushNotifications(); - if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "finishing primary sync"); + Log.d(this, "finishing primary sync"); } catch (Exception e) { com.newsblur.util.Log.e(this.getClass().getName(), "Sync error.", e); - } finally { - decrementRunningChild(startId); - } + } } /** @@ -397,7 +441,7 @@ public class NBSyncService extends Service { if (HaltNow) return; if (FollowupActions.size() < 1) return; - if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "double-checking " + FollowupActions.size() + " actions"); + Log.d(this, "double-checking " + FollowupActions.size() + " actions"); int impactFlags = 0; for (ReadingAction ra : FollowupActions) { int impact = ra.doLocal(dbHelper, true); @@ -420,7 +464,7 @@ public class NBSyncService extends Service { * The very first step of a sync - get the feed/folder list, unread counts, and * unread hashes. Doing this resets pagination on the server! */ - private void syncMetadata(int startId) { + private void syncMetadata() { if (stopSync()) return; if (backoffBackgroundCalls()) return; int untriedActions = dbHelper.getUntriedActionCount(); @@ -559,8 +603,8 @@ public class NBSyncService extends Service { com.newsblur.util.Log.i(this.getClass().getName(), "got feed list: " + getSpeedInfo()); UnreadsService.doMetadata(); - unreadsService.start(startId); - cleanupService.start(startId); + unreadsService.start(); + cleanupService.start(); } finally { FFSyncRunning = false; @@ -654,7 +698,7 @@ public class NBSyncService extends Service { /** * Fetch stories needed because the user is actively viewing a feed or folder. */ - private void syncPendingFeedStories(int startId) { + private void syncPendingFeedStories() { // track whether we actually tried to handle the feedset and found we had nothing // more to do, in which case we will clear it boolean finished = false; @@ -679,6 +723,7 @@ public class NBSyncService extends Service { } if (fs == null) { + com.newsblur.util.Log.d(this.getClass().getName(), "No feed set to sync"); return; } @@ -731,7 +776,7 @@ public class NBSyncService extends Service { finishActions(); NbActivity.updateAllActivities(NbActivity.UPDATE_STORY | NbActivity.UPDATE_STATUS); - prefetchOriginalText(apiResponse, startId); + prefetchOriginalText(apiResponse); FeedPagesSeen.put(fs, pageNumber); totalStoriesSeen += apiResponse.stories.length; @@ -843,7 +888,7 @@ public class NBSyncService extends Service { dbHelper.insertStories(apiResponse, false); } - void prefetchOriginalText(StoriesResponse apiResponse, int startId) { + void prefetchOriginalText(StoriesResponse apiResponse) { storyloop: for (Story story : apiResponse.stories) { // only prefetch for unreads, so we don't grind to cache when the user scrolls // through old read stories @@ -856,10 +901,10 @@ public class NBSyncService extends Service { } } } - originalTextService.startConditional(startId); + originalTextService.start(); } - void prefetchImages(StoriesResponse apiResponse, int startId) { + void prefetchImages(StoriesResponse apiResponse) { storyloop: for (Story story : apiResponse.stories) { // only prefetch for unreads, so we don't grind to cache when the user scrolls // through old read stories @@ -874,7 +919,7 @@ public class NBSyncService extends Service { imagePrefetchService.addThumbnailUrl(story.thumbnailUrl); } } - imagePrefetchService.startConditional(startId); + imagePrefetchService.start(); } void pushNotifications() { @@ -892,27 +937,28 @@ public class NBSyncService extends Service { closeQuietly(cUnread); } - void incrementRunningChild() { - synchronized (WAKELOCK_MUTEX) { - wl.acquire(); - } - } - - void decrementRunningChild(int startId) { - synchronized (WAKELOCK_MUTEX) { - if (wl == null) return; - if (wl.isHeld()) { - wl.release(); + /** + * Check to see if all async sync tasks have completed, indicating that sync can me marked as + * complete. Call this any time any individual sync task finishes. + */ + void checkCompletion() { + //Log.d(this, "checking completion"); + if (mainSyncRunning) return; + if ((cleanupService != null) && cleanupService.isRunning()) return; + if ((originalTextService != null) && originalTextService.isRunning()) return; + if ((unreadsService != null) && unreadsService.isRunning()) return; + if ((imagePrefetchService != null) && imagePrefetchService.isRunning()) return; + Log.d(this, "confirmed completion"); + // iff all threads have finished, mark all received work as completed + synchronized (COMPLETION_CALLBACKS_MUTEX) { + for (JobParameters params : outstandingStartParams) { + jobFinished(params, forceHalted); } - // our wakelock reference counts. only stop the service if it is in the background and if - // we are the last thread to release the lock. - if (!wl.isHeld()) { - if (NbActivity.getActiveActivityCount() < 1) { - stopSelf(startId); - } - lastStartIdCompleted = startId; - if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "wakelock depleted"); + for (Integer startId : outstandingStartIds) { + stopSelf(startId); } + outstandingStartIds.clear(); + outstandingStartParams.clear(); } } @@ -945,18 +991,6 @@ public class NBSyncService extends Service { return true; } - public void onTrimMemory (int level) { - if (level > ComponentCallbacks2.TRIM_MEMORY_UI_HIDDEN) { - isMemoryLow = true; - } - - // this is also called when the UI is hidden, so double check if we need to - // stop - if ( (lastStartIdCompleted != -1) && (NbActivity.getActiveActivityCount() < 1)) { - stopSelf(lastStartIdCompleted); - } - } - /** * Is the main feed/folder list sync running and blocking? */ @@ -994,14 +1028,14 @@ public class NBSyncService extends Service { if (OfflineNow) return context.getResources().getString(R.string.sync_status_offline); if (HousekeepingRunning) return context.getResources().getString(R.string.sync_status_housekeeping); if (FFSyncRunning) return context.getResources().getString(R.string.sync_status_ffsync); - if (CleanupService.running()) return context.getResources().getString(R.string.sync_status_cleanup); + if (CleanupService.activelyRunning) return context.getResources().getString(R.string.sync_status_cleanup); if (brief && !AppConstants.VERBOSE_LOG) return null; if (ActionsRunning) return String.format(context.getResources().getString(R.string.sync_status_actions), lastActionCount); if (RecountsRunning) return context.getResources().getString(R.string.sync_status_recounts); if (StorySyncRunning) return context.getResources().getString(R.string.sync_status_stories); - if (UnreadsService.running()) return String.format(context.getResources().getString(R.string.sync_status_unreads), UnreadsService.getPendingCount()); - if (OriginalTextService.running()) return String.format(context.getResources().getString(R.string.sync_status_text), OriginalTextService.getPendingCount()); - if (ImagePrefetchService.running()) return String.format(context.getResources().getString(R.string.sync_status_images), ImagePrefetchService.getPendingCount()); + if (UnreadsService.activelyRunning) return String.format(context.getResources().getString(R.string.sync_status_unreads), UnreadsService.getPendingCount()); + if (OriginalTextService.activelyRunning) return String.format(context.getResources().getString(R.string.sync_status_text), OriginalTextService.getPendingCount()); + if (ImagePrefetchService.activelyRunning) return String.format(context.getResources().getString(R.string.sync_status_images), ImagePrefetchService.getPendingCount()); return null; } @@ -1047,7 +1081,7 @@ public class NBSyncService extends Service { PendingFeed = fs; PendingFeedTarget = desiredStoryCount; - //if (AppConstants.VERBOSE_LOG) Log.d(NBSyncService.class.getName(), "callerhas: " + callerSeen + " have:" + alreadySeen + " want:" + desiredStoryCount + " pending:" + alreadyPending); + //Log.d(NBSyncService.class.getName(), "callerhas: " + callerSeen + " have:" + alreadySeen + " want:" + desiredStoryCount + " pending:" + alreadyPending); if (!fs.equals(LastFeedSet)) { return true; @@ -1144,15 +1178,15 @@ public class NBSyncService extends Service { ImagePrefetchService.clear(); } - public static void resumeFromInterrupt() { - HaltNow = false; - } - @Override public void onDestroy() { try { - com.newsblur.util.Log.d(this, "onDestroy - stopping execution"); - HaltNow = true; + com.newsblur.util.Log.d(this, "onDestroy"); + synchronized (COMPLETION_CALLBACKS_MUTEX) { + if ((outstandingStartIds.size() > 0) || (outstandingStartParams.size() > 0)) { + com.newsblur.util.Log.w(this, "Service scheduler destroyed before all jobs marked done?"); + } + } if (cleanupService != null) cleanupService.shutdown(); if (unreadsService != null) unreadsService.shutdown(); if (originalTextService != null) originalTextService.shutdown(); @@ -1166,21 +1200,15 @@ public class NBSyncService extends Service { Thread.currentThread().interrupt(); } } - if (dbHelper != null) dbHelper.close(); - com.newsblur.util.Log.d(this, "onDestroy - execution halted"); - super.onDestroy(); + if (dbHelper != null) { + dbHelper.close(); + dbHelper = null; + } + com.newsblur.util.Log.d(this, "onDestroy done"); } catch (Exception ex) { com.newsblur.util.Log.e(this, "unclean shutdown", ex); } - } - - @Override - public IBinder onBind(Intent intent) { - return null; - } - - public static boolean isMemoryLow() { - return isMemoryLow; + super.onDestroy(); } public static String getSpeedInfo() { diff --git a/clients/android/NewsBlur/src/com/newsblur/service/OriginalTextService.java b/clients/android/NewsBlur/src/com/newsblur/service/OriginalTextService.java index a2edc1d55..f7213df40 100644 --- a/clients/android/NewsBlur/src/com/newsblur/service/OriginalTextService.java +++ b/clients/android/NewsBlur/src/com/newsblur/service/OriginalTextService.java @@ -13,13 +13,13 @@ import java.util.regex.Pattern; public class OriginalTextService extends SubService { + public static boolean activelyRunning = false; + // special value for when the API responds that it could fatally could not fetch text public static final String NULL_STORY_TEXT = "__NULL_STORY_TEXT__"; private static final Pattern imgSniff = Pattern.compile("]*src=(['\"])((?:(?!\\1).)*)\\1[^>]*>", Pattern.CASE_INSENSITIVE); - private static volatile boolean Running = false; - /** story hashes we need to fetch (from newly found stories) */ private static Set Hashes; static {Hashes = new HashSet();} @@ -33,11 +33,15 @@ public class OriginalTextService extends SubService { @Override protected void exec() { - while ((Hashes.size() > 0) || (PriorityHashes.size() > 0)) { - if (parent.stopSync()) return; - gotWork(); - fetchBatch(PriorityHashes); - fetchBatch(Hashes); + activelyRunning = true; + try { + while ((Hashes.size() > 0) || (PriorityHashes.size() > 0)) { + if (parent.stopSync()) return; + fetchBatch(PriorityHashes); + fetchBatch(Hashes); + } + } finally { + activelyRunning = false; } } @@ -97,27 +101,10 @@ public class OriginalTextService extends SubService { return (Hashes.size() + PriorityHashes.size()); } - @Override - public boolean haveWork() { - return (getPendingCount() > 0); - } - public static void clear() { Hashes.clear(); PriorityHashes.clear(); } - public static boolean running() { - return Running; - } - @Override - protected void setRunning(boolean running) { - Running = running; - } - @Override - public boolean isRunning() { - return Running; - } - } diff --git a/clients/android/NewsBlur/src/com/newsblur/service/ServiceScheduleReceiver.java b/clients/android/NewsBlur/src/com/newsblur/service/ServiceScheduleReceiver.java deleted file mode 100644 index 17a1d0b0a..000000000 --- a/clients/android/NewsBlur/src/com/newsblur/service/ServiceScheduleReceiver.java +++ /dev/null @@ -1,17 +0,0 @@ -package com.newsblur.service; - -import android.content.BroadcastReceiver; -import android.content.Context; -import android.content.Intent; -import android.util.Log; - -public class ServiceScheduleReceiver extends BroadcastReceiver { - - @Override - public void onReceive(Context context, Intent intent) { - Log.d(this.getClass().getName(), "starting sync service"); - Intent i = new Intent(context, NBSyncService.class); - context.startService(i); - } - -} diff --git a/clients/android/NewsBlur/src/com/newsblur/service/SubService.java b/clients/android/NewsBlur/src/com/newsblur/service/SubService.java index f13a8e0b2..7d4e49560 100644 --- a/clients/android/NewsBlur/src/com/newsblur/service/SubService.java +++ b/clients/android/NewsBlur/src/com/newsblur/service/SubService.java @@ -1,14 +1,16 @@ package com.newsblur.service; +import android.app.job.JobParameters; import android.os.Process; -import android.util.Log; import com.newsblur.activity.NbActivity; import com.newsblur.util.AppConstants; +import com.newsblur.util.Log; -import java.util.concurrent.Executors; -import java.util.concurrent.ExecutorService; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; +import java.util.concurrent.RejectedExecutionException; /** * A utility construct to make NbSyncService a bit more modular by encapsulating sync tasks @@ -19,8 +21,7 @@ import java.util.concurrent.TimeUnit; public abstract class SubService { protected NBSyncService parent; - private ExecutorService executor; - protected int startId; + private ThreadPoolExecutor executor; private long cycleStartTime = 0L; private SubService() { @@ -29,15 +30,13 @@ public abstract class SubService { SubService(NBSyncService parent) { this.parent = parent; - executor = Executors.newFixedThreadPool(1); + executor = new ThreadPoolExecutor(1, 1, 0L, TimeUnit.MILLISECONDS, new LinkedBlockingQueue()); } - public void start(final int startId) { - if (parent.stopSync()) return; - parent.incrementRunningChild(); - this.startId = startId; + public void start() { Runnable r = new Runnable() { public void run() { + if (parent.stopSync()) return; if (NbActivity.getActiveActivityCount() < 1) { Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND + Process.THREAD_PRIORITY_LESS_FAVORABLE ); } else { @@ -45,44 +44,37 @@ public abstract class SubService { } Thread.currentThread().setName(this.getClass().getName()); exec_(); - parent.decrementRunningChild(startId); } }; - executor.execute(r); - } - - public void startConditional(int startId) { - if (haveWork()) start(startId); - } - - /** - * Stub - children should implement a queue check or ready check so that startConditional() - * can more efficiently allocate threads. - */ - protected boolean haveWork() { - return true; + try { + executor.execute(r); + // enqueue a check task that will run strictly after the real one, so the callback + // can effectively check queue size to see if there are queued tasks + executor.execute(new Runnable() { + public void run() { + parent.checkCompletion(); + NbActivity.updateAllActivities(NbActivity.UPDATE_STATUS); + } + }); + } catch (RejectedExecutionException ree) { + // this is perfectly normal, as service soft-stop mechanics might have shut down our thread pool + // while peer subservices are still running + } } private synchronized void exec_() { try { - //if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "SubService started"); exec(); - //if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "SubService completed"); cycleStartTime = 0; } catch (Exception e) { com.newsblur.util.Log.e(this.getClass().getName(), "Sync error.", e); - } finally { - if (isRunning()) { - setRunning(false); - NbActivity.updateAllActivities(NbActivity.UPDATE_STATUS); - } - } + } } protected abstract void exec(); public void shutdown() { - if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "SubService stopping"); + Log.d(this, "SubService stopping"); executor.shutdown(); try { executor.awaitTermination(AppConstants.SHUTDOWN_SLACK_SECONDS, TimeUnit.SECONDS); @@ -90,21 +82,18 @@ public abstract class SubService { executor.shutdownNow(); Thread.currentThread().interrupt(); } finally { - if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "SubService stopped"); + Log.d(this, "SubService stopped"); } } - protected void gotWork() { - setRunning(true); - NbActivity.updateAllActivities(NbActivity.UPDATE_STATUS); - } - protected void gotData(int updateType) { NbActivity.updateAllActivities(updateType); } - protected abstract void setRunning(boolean running); - protected abstract boolean isRunning(); + public boolean isRunning() { + // don't advise completion until there are no tasks, or just one check task left + return (executor.getQueue().size() > 0); + } /** * If called at the beginning of an expensive loop in a SubService, enforces the maximum duty cycle diff --git a/clients/android/NewsBlur/src/com/newsblur/service/UnreadsService.java b/clients/android/NewsBlur/src/com/newsblur/service/UnreadsService.java index 861dcaeca..11a909ca6 100644 --- a/clients/android/NewsBlur/src/com/newsblur/service/UnreadsService.java +++ b/clients/android/NewsBlur/src/com/newsblur/service/UnreadsService.java @@ -16,7 +16,7 @@ import java.util.Set; public class UnreadsService extends SubService { - private static volatile boolean Running = false; + public static boolean activelyRunning = false; private static volatile boolean doMetadata = false; @@ -30,17 +30,20 @@ public class UnreadsService extends SubService { @Override protected void exec() { - if (doMetadata) { - gotWork(); - syncUnreadList(); - doMetadata = false; - } + activelyRunning = true; + try { + if (doMetadata) { + syncUnreadList(); + doMetadata = false; + } - if (StoryHashQueue.size() > 0) { - getNewUnreadStories(); - parent.pushNotifications(); + if (StoryHashQueue.size() > 0) { + getNewUnreadStories(); + parent.pushNotifications(); + } + } finally { + activelyRunning = false; } - } private void syncUnreadList() { @@ -133,7 +136,6 @@ public class UnreadsService extends SubService { boolean isEnableNotifications = PrefsUtils.isEnableNotifications(parent); if (! (isOfflineEnabled || isEnableNotifications)) return; - gotWork(); startExpensiveCycle(); List hashBatch = new ArrayList(AppConstants.UNREAD_FETCH_BATCH_SIZE); @@ -161,8 +163,8 @@ public class UnreadsService extends SubService { StoryHashQueue.remove(hash); } - parent.prefetchOriginalText(response, startId); - parent.prefetchImages(response, startId); + parent.prefetchOriginalText(response); + parent.prefetchImages(response); } } @@ -202,17 +204,5 @@ public class UnreadsService extends SubService { return doMetadata; } - public static boolean running() { - return Running; - } - @Override - protected void setRunning(boolean running) { - Running = running; - } - @Override - protected boolean isRunning() { - return Running; - } - } diff --git a/clients/android/NewsBlur/src/com/newsblur/util/FeedUtils.java b/clients/android/NewsBlur/src/com/newsblur/util/FeedUtils.java index d81d2befb..ad04d266d 100644 --- a/clients/android/NewsBlur/src/com/newsblur/util/FeedUtils.java +++ b/clients/android/NewsBlur/src/com/newsblur/util/FeedUtils.java @@ -4,7 +4,12 @@ import java.util.Collections; import java.util.HashSet; import java.util.Set; +import android.app.job.JobInfo; +import android.app.job.JobParameters; +import android.app.job.JobScheduler; +import android.app.job.JobWorkItem; import android.content.Context; +import android.content.ComponentName; import android.content.Intent; import android.os.AsyncTask; import android.text.TextUtils; @@ -49,7 +54,10 @@ public class FeedUtils { } } - private static void triggerSync(Context c) { + public static void triggerSync(Context c) { + // NB: when our minSDKversion hits 28, it could be possible to start the service via the JobScheduler + // with the setImportantWhileForeground() flag via an enqueue() and get rid of all legacy startService + // code paths Intent i = new Intent(c, NBSyncService.class); c.startService(i); } diff --git a/clients/android/NewsBlur/src/com/newsblur/util/PrefsUtils.java b/clients/android/NewsBlur/src/com/newsblur/util/PrefsUtils.java index 5fceb5544..27148a229 100644 --- a/clients/android/NewsBlur/src/com/newsblur/util/PrefsUtils.java +++ b/clients/android/NewsBlur/src/com/newsblur/util/PrefsUtils.java @@ -50,7 +50,6 @@ public class PrefsUtils { edit.putString(PrefConstants.PREF_COOKIE, cookie); edit.putString(PrefConstants.PREF_UNIQUE_LOGIN, userName + "_" + System.currentTimeMillis()); edit.commit(); - NBSyncService.resumeFromInterrupt(); } public static boolean checkForUpgrade(Context context) { @@ -126,8 +125,6 @@ public class PrefsUtils { s.append("\n"); s.append("server: ").append(APIConstants.isCustomServer() ? "default" : "custom"); s.append("\n"); - s.append("memory: ").append(NBSyncService.isMemoryLow() ? "low" : "normal"); - s.append("\n"); s.append("speed: ").append(NBSyncService.getSpeedInfo()); s.append("\n"); s.append("pending actions: ").append(NBSyncService.getPendingInfo());