Bug 1241810 - Restructure FeedService to always complete wakeful intent. r=mcomella draft
authorSebastian Kaspari <s.kaspari@gmail.com>
Thu, 25 Feb 2016 14:59:32 -0800
changeset 343936 cb943b087c76eb9042d593209e66cea8507771bd
parent 343935 449943603adce3e6e9d240910f78236b03fe648d
child 343937 187ea0285c49fb5fe23a37711bb0a6e951621df8
push id13713
push users.kaspari@gmail.com
push dateWed, 23 Mar 2016 15:10:41 +0000
reviewersmcomella
bugs1241810
milestone48.0a1
Bug 1241810 - Restructure FeedService to always complete wakeful intent. r=mcomella MozReview-Commit-ID: FyczEKtsTgv
mobile/android/base/java/org/mozilla/gecko/feeds/FeedService.java
mobile/android/base/java/org/mozilla/gecko/feeds/action/BaseAction.java
mobile/android/base/java/org/mozilla/gecko/feeds/action/CheckAction.java
mobile/android/base/java/org/mozilla/gecko/feeds/action/EnrollAction.java
mobile/android/base/java/org/mozilla/gecko/feeds/action/SetupAction.java
mobile/android/base/java/org/mozilla/gecko/feeds/action/SubscribeAction.java
mobile/android/base/java/org/mozilla/gecko/feeds/action/WithdrawAction.java
mobile/android/base/moz.build
--- a/mobile/android/base/java/org/mozilla/gecko/feeds/FeedService.java
+++ b/mobile/android/base/java/org/mozilla/gecko/feeds/FeedService.java
@@ -5,22 +5,24 @@
 
 package org.mozilla.gecko.feeds;
 
 import android.app.IntentService;
 import android.content.Context;
 import android.content.Intent;
 import android.net.ConnectivityManager;
 import android.net.NetworkInfo;
+import android.support.annotation.Nullable;
 import android.support.v4.net.ConnectivityManagerCompat;
 import android.util.Log;
 
 import com.keepsafe.switchboard.SwitchBoard;
 
 import org.mozilla.gecko.AppConstants;
+import org.mozilla.gecko.feeds.action.BaseAction;
 import org.mozilla.gecko.feeds.action.CheckAction;
 import org.mozilla.gecko.feeds.action.EnrollAction;
 import org.mozilla.gecko.feeds.action.SetupAction;
 import org.mozilla.gecko.feeds.action.SubscribeAction;
 import org.mozilla.gecko.feeds.action.WithdrawAction;
 import org.mozilla.gecko.feeds.subscriptions.SubscriptionStorage;
 import org.mozilla.gecko.util.Experiments;
 
@@ -60,65 +62,73 @@ public class FeedService extends IntentS
     public void onCreate() {
         super.onCreate();
 
         storage = new SubscriptionStorage(getApplicationContext());
     }
 
     @Override
     protected void onHandleIntent(Intent intent) {
-        if (intent == null) {
-            return;
-        }
+        try {
+            if (!SwitchBoard.isInExperiment(this, Experiments.CONTENT_NOTIFICATIONS)) {
+                Log.d(LOGTAG, "Not in content notifications experiment. Skipping.");
+                return;
+            }
+
+            BaseAction action = createActionForIntent(intent);
+            if (action == null) {
+                Log.d(LOGTAG, "No action to process");
+                return;
+            }
 
-        if (!SwitchBoard.isInExperiment(this, Experiments.CONTENT_NOTIFICATIONS)) {
-            Log.d(LOGTAG, "Not in content notifications experiment. Skipping.");
-            return;
-        }
+            if (action.requiresNetwork() && !isConnectedToUnmeteredNetwork()) {
+                // For now just skip if we are not connected or the network is metered. We do not want
+                // to use precious mobile traffic.
+                Log.d(LOGTAG, "Not connected to a network or network is metered. Skipping.");
+                return;
+            }
+
+            action.perform(intent);
 
-        if (!isConnectedToNetwork() || isActiveNetworkMetered()) {
-            // For now just skip if we are not connected or the network is metered. We do not want
-            // to use precious mobile traffic.
-            Log.d(LOGTAG, "Not connected to a network or network is metered. Skipping.");
-            return;
+            storage.persistChanges();
+        } finally {
+            FeedAlarmReceiver.completeWakefulIntent(intent);
+        }
+    }
+
+    @Nullable
+    private BaseAction createActionForIntent(Intent intent) {
+        if (intent == null) {
+            return null;
         }
 
         switch (intent.getAction()) {
             case ACTION_SETUP:
-                new SetupAction(this).perform();
-                break;
+                return new SetupAction(this);
 
             case ACTION_SUBSCRIBE:
-                new SubscribeAction(storage).perform(intent);
-                break;
+                return new SubscribeAction(storage);
 
             case ACTION_CHECK:
-                new CheckAction(this, storage).perform();
-                break;
+                return new CheckAction(this, storage);
 
             case ACTION_ENROLL:
-                new EnrollAction(this).perform();
-                break;
+                return new EnrollAction(this);
 
             case ACTION_WITHDRAW:
-                new WithdrawAction(this, storage).perform();
-                break;
+                return new WithdrawAction(this, storage);
 
             default:
                 Log.e(LOGTAG, "Unknown action: " + intent.getAction());
+                return null;
         }
-
-        storage.persistChanges();
-
-        FeedAlarmReceiver.completeWakefulIntent(intent);
     }
 
-    private boolean isConnectedToNetwork() {
+    private boolean isConnectedToUnmeteredNetwork() {
         ConnectivityManager manager = (ConnectivityManager) getSystemService(Context.CONNECTIVITY_SERVICE);
         NetworkInfo networkInfo = manager.getActiveNetworkInfo();
-        return networkInfo != null && networkInfo.isConnected();
-    }
+        if (networkInfo == null || !networkInfo.isConnected()) {
+            return false;
+        }
 
-    private boolean isActiveNetworkMetered() {
-        return ConnectivityManagerCompat.isActiveNetworkMetered(
-                (ConnectivityManager) getSystemService(Context.CONNECTIVITY_SERVICE));
+        return !ConnectivityManagerCompat.isActiveNetworkMetered(manager);
     }
 }
new file mode 100644
--- /dev/null
+++ b/mobile/android/base/java/org/mozilla/gecko/feeds/action/BaseAction.java
@@ -0,0 +1,25 @@
+/* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.gecko.feeds.action;
+
+import android.content.Intent;
+
+/**
+ * Interface for actions run by FeedService.
+ */
+public interface BaseAction {
+    /**
+     * Perform this action.
+     *
+     * @param intent used to start the service.
+     */
+    void perform(Intent intent);
+
+    /**
+     * Does this action require an active network connection?
+     */
+    boolean requiresNetwork();
+}
--- a/mobile/android/base/java/org/mozilla/gecko/feeds/action/CheckAction.java
+++ b/mobile/android/base/java/org/mozilla/gecko/feeds/action/CheckAction.java
@@ -27,28 +27,29 @@ import org.mozilla.gecko.util.StringUtil
 
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.List;
 
 /**
  * CheckAction: Check if feeds we subscribed to have new content available.
  */
-public class CheckAction {
+public class CheckAction implements BaseAction {
     private static final String LOGTAG = "FeedCheckAction";
 
     private Context context;
     private SubscriptionStorage storage;
 
     public CheckAction(Context context, SubscriptionStorage storage) {
         this.context = context;
         this.storage = storage;
     }
 
-    public void perform() {
+    @Override
+    public void perform(Intent intent) {
         final List<FeedSubscription> subscriptions = storage.getSubscriptions();
 
         Log.d(LOGTAG, "Checking feeds for updates (" + subscriptions.size() + " feeds) ..");
 
         List<Feed> updatedFeeds = new ArrayList<>();
 
         for (FeedSubscription subscription : subscriptions) {
             Log.i(LOGTAG, "Checking feed: " + subscription.getFeedTitle());
@@ -140,9 +141,14 @@ public class CheckAction {
 
     private FeedFetcher.FeedResponse fetchFeed(FeedSubscription subscription) {
         return FeedFetcher.fetchAndParseFeedIfModified(
                 subscription.getFeedUrl(),
                 subscription.getETag(),
                 subscription.getLastModified()
         );
     }
+
+    @Override
+    public boolean requiresNetwork() {
+        return true;
+    }
 }
--- a/mobile/android/base/java/org/mozilla/gecko/feeds/action/EnrollAction.java
+++ b/mobile/android/base/java/org/mozilla/gecko/feeds/action/EnrollAction.java
@@ -1,55 +1,62 @@
 /* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.feeds.action;
 
 import android.content.Context;
+import android.content.Intent;
 import android.database.Cursor;
 import android.text.TextUtils;
 import android.util.Log;
 
 import org.mozilla.gecko.GeckoProfile;
 import org.mozilla.gecko.db.BrowserContract;
 import org.mozilla.gecko.db.BrowserDB;
 import org.mozilla.gecko.feeds.FeedService;
 import org.mozilla.gecko.feeds.knownsites.KnownSiteBlogger;
 import org.mozilla.gecko.feeds.knownsites.KnownSite;
 import org.mozilla.gecko.feeds.knownsites.KnownSiteMedium;
 
 /**
  * EnrollAction: Search for bookmarks of known sites we can subscribe to.
  */
-public class EnrollAction {
+public class EnrollAction implements BaseAction {
     private static final String LOGTAG = "FeedEnrollAction";
 
     private static final KnownSite[] knownSites = {
         new KnownSiteMedium(),
         new KnownSiteBlogger(),
     };
 
     private Context context;
 
     public EnrollAction(Context context) {
         this.context = context;
     }
 
-    public void perform() {
+    @Override
+    public void perform(Intent intent) {
         Log.i(LOGTAG, "Searching for bookmarks to enroll in updates");
 
         BrowserDB db = GeckoProfile.get(context).getDB();
 
         for (KnownSite knownSite : knownSites) {
             searchFor(db, knownSite);
         }
     }
 
+    @Override
+    public boolean requiresNetwork() {
+        return false;
+    }
+
     private void searchFor(BrowserDB db, KnownSite knownSite) {
         Cursor cursor = db.getBookmarksForPartialUrl(context.getContentResolver(), "://" + knownSite.getURLSearchString() + "/");
         if (cursor == null) {
             Log.d(LOGTAG, "Nothing found");
             return;
         }
 
         try {
--- a/mobile/android/base/java/org/mozilla/gecko/feeds/action/SetupAction.java
+++ b/mobile/android/base/java/org/mozilla/gecko/feeds/action/SetupAction.java
@@ -13,32 +13,38 @@ import android.os.SystemClock;
 import android.util.Log;
 
 import org.mozilla.gecko.feeds.FeedAlarmReceiver;
 import org.mozilla.gecko.feeds.FeedService;
 
 /**
  * SetupAction: Set up alarms to run various actions every now and then.
  */
-public class SetupAction {
+public class SetupAction implements BaseAction {
     private static final String LOGTAG = "FeedSetupAction";
 
     private Context context;
 
     public SetupAction(Context context) {
         this.context = context;
     }
 
-    public void perform() {
+    @Override
+    public void perform(Intent intent) {
         final AlarmManager alarmManager = (AlarmManager) context.getSystemService(Context.ALARM_SERVICE);
 
         cancelPreviousAlarms(alarmManager);
         scheduleAlarms(alarmManager);
     }
 
+    @Override
+    public boolean requiresNetwork() {
+        return false;
+    }
+
     private void cancelPreviousAlarms(AlarmManager alarmManager) {
         final PendingIntent withdrawIntent = getWithdrawPendingIntent();
         alarmManager.cancel(withdrawIntent);
 
         final PendingIntent enrollIntent = getEnrollPendingIntent();
         alarmManager.cancel(enrollIntent);
 
         final PendingIntent checkIntent = getCheckPendingIntent();
--- a/mobile/android/base/java/org/mozilla/gecko/feeds/action/SubscribeAction.java
+++ b/mobile/android/base/java/org/mozilla/gecko/feeds/action/SubscribeAction.java
@@ -13,28 +13,29 @@ import org.mozilla.gecko.feeds.FeedFetch
 import org.mozilla.gecko.feeds.subscriptions.FeedSubscription;
 import org.mozilla.gecko.feeds.subscriptions.SubscriptionStorage;
 
 import java.util.UUID;
 
 /**
  * SubscribeAction: Try to fetch a feed and create a subscription if successful.
  */
-public class SubscribeAction {
+public class SubscribeAction implements BaseAction {
     private static final String LOGTAG = "FeedSubscribeAction";
 
     public static final String EXTRA_GUID = "guid";
     public static final String EXTRA_FEED_URL = "feed_url";
 
     private SubscriptionStorage storage;
 
     public SubscribeAction(SubscriptionStorage storage) {
         this.storage = storage;
     }
 
+    @Override
     public void perform(Intent intent) {
         Log.d(LOGTAG, "Subscribing to feed..");
 
         final Bundle extras = intent.getExtras();
 
         // TODO: Using a random UUID as fallback just so that I can subscribe for things that are not bookmarks (testing)
         final String guid = extras.getString(EXTRA_GUID, UUID.randomUUID().toString());
         final String feedUrl = intent.getStringExtra(EXTRA_FEED_URL);
@@ -42,16 +43,21 @@ public class SubscribeAction {
         if (storage.hasSubscriptionForBookmark(guid)) {
             Log.d(LOGTAG, "Already subscribed to " + feedUrl + ". Skipping.");
             return;
         }
 
         subscribe(guid, feedUrl);
     }
 
+    @Override
+    public boolean requiresNetwork() {
+        return true;
+    }
+
     private void subscribe(String guid, String feedUrl) {
         FeedFetcher.FeedResponse response = FeedFetcher.fetchAndParseFeed(feedUrl);
         if (response == null) {
             Log.w(LOGTAG, String.format("Could not fetch feed (%s). Not subscribing for now.", feedUrl));
             return;
         }
 
         Log.d(LOGTAG, "Subscribing to feed: " + response.feed.getTitle());
--- a/mobile/android/base/java/org/mozilla/gecko/feeds/action/WithdrawAction.java
+++ b/mobile/android/base/java/org/mozilla/gecko/feeds/action/WithdrawAction.java
@@ -1,51 +1,58 @@
 /* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.feeds.action;
 
 import android.content.Context;
+import android.content.Intent;
 import android.util.Log;
 
 import org.mozilla.gecko.GeckoProfile;
 import org.mozilla.gecko.db.BrowserDB;
 import org.mozilla.gecko.feeds.subscriptions.FeedSubscription;
 import org.mozilla.gecko.feeds.subscriptions.SubscriptionStorage;
 
 import java.util.List;
 
 /**
  * WithdrawAction: Look for feeds to unsubscribe from.
  */
-public class WithdrawAction {
+public class WithdrawAction implements BaseAction {
     private static final String LOGTAG = "FeedWithdrawAction";
 
     private Context context;
     private SubscriptionStorage storage;
 
     public WithdrawAction(Context context, SubscriptionStorage storage) {
         this.context = context;
         this.storage = storage;
     }
 
-    public void perform() {
+    @Override
+    public void perform(Intent intent) {
         BrowserDB db = GeckoProfile.get(context).getDB();
 
         List<FeedSubscription> subscriptions = storage.getSubscriptions();
 
         Log.d(LOGTAG, "Checking " + subscriptions.size() + " subscriptions");
 
         for (FeedSubscription subscription : subscriptions) {
             if (!db.hasBookmarkWithGuid(context.getContentResolver(), subscription.getBookmarkGUID())) {
                 unsubscribe(subscription);
             }
         }
     }
 
+    @Override
+    public boolean requiresNetwork() {
+        return false;
+    }
+
     private void unsubscribe(FeedSubscription subscription) {
         Log.d(LOGTAG, "Unsubscribing from: (" + subscription.getBookmarkGUID() + ") " + subscription.getFeedUrl());
 
         storage.removeSubscription(subscription);
     }
 }
--- a/mobile/android/base/moz.build
+++ b/mobile/android/base/moz.build
@@ -269,16 +269,17 @@ gbjar.sources += ['java/org/mozilla/geck
     'favicons/decoders/FaviconDecoder.java',
     'favicons/decoders/ICODecoder.java',
     'favicons/decoders/IconDirectoryEntry.java',
     'favicons/decoders/LoadFaviconResult.java',
     'favicons/Favicons.java',
     'favicons/LoadFaviconTask.java',
     'favicons/OnFaviconLoadedListener.java',
     'favicons/RemoteFavicon.java',
+    'feeds/action/BaseAction.java',
     'feeds/action/CheckAction.java',
     'feeds/action/EnrollAction.java',
     'feeds/action/SetupAction.java',
     'feeds/action/SubscribeAction.java',
     'feeds/action/WithdrawAction.java',
     'feeds/FeedAlarmReceiver.java',
     'feeds/FeedFetcher.java',
     'feeds/FeedService.java',