Review comments. draft
authorNick Alexander <nalexander@mozilla.com>
Wed, 02 Mar 2016 16:04:02 -0800
changeset 336297 5e075b996af6f37cc96059352e4162539d8b00a3
parent 336296 0ca118abbe831b931531706c468c84b0d3d5ad5d
child 336298 4d7dd8db20c99f5716ce9f170a00e1fca331920b
child 336360 54825ffa0bb88199e1a878df3f31764a5cfd6fec
push id12029
push usernalexander@mozilla.com
push dateThu, 03 Mar 2016 00:10:36 +0000
milestone47.0a1
Review comments. MozReview-Commit-ID: GId7eLebFic
mobile/android/base/java/org/mozilla/gecko/push/PushClient.java
mobile/android/base/java/org/mozilla/gecko/push/PushManager.java
mobile/android/base/java/org/mozilla/gecko/util/GeckoBackgroundThread.java
mobile/android/tests/background/junit4/src/org/mozilla/gecko/push/TestPushManager.java
--- a/mobile/android/base/java/org/mozilla/gecko/push/PushClient.java
+++ b/mobile/android/base/java/org/mozilla/gecko/push/PushClient.java
@@ -32,43 +32,53 @@ public class PushClient {
     }
 
     private final AutopushClient autopushClient;
 
     public PushClient(String serverURI) {
         this.autopushClient = new AutopushClient(serverURI, Utils.newSynchronousExecutor());
     }
 
+    /**
+     * Each instance is <b>single-use</b>!  Exactly one delegate method should be invoked once,
+     * but we take care to handle multiple invocations (favoring the earliest), just to be safe.
+     */
     protected static class Delegate<T> implements AutopushClient.RequestDelegate<T> {
         Object result; // Oh, for an algebraic data type when you need one!
 
         @SuppressWarnings("unchecked")
         public T responseOrThrow() throws LocalException, AutopushClientException {
             if (result instanceof LocalException) {
                 throw (LocalException) result;
             }
             if (result instanceof AutopushClientException) {
                 throw (AutopushClientException) result;
             }
             return (T) result;
         }
 
         @Override
         public void handleError(Exception e) {
-            result = new LocalException(e);
+            if (result == null) {
+                result = new LocalException(e);
+            }
         }
 
         @Override
         public void handleFailure(AutopushClientException e) {
-            result = e;
+            if (result == null) {
+                result = e;
+            }
         }
 
         @Override
         public void handleSuccess(T response) {
-            result = response;
+            if (result == null) {
+                result = response;
+            }
         }
     }
 
     public RegisterUserAgentResponse registerUserAgent(@NonNull String token) throws LocalException, AutopushClientException {
         final Delegate<RegisterUserAgentResponse> delegate = new Delegate<>();
         autopushClient.registerUserAgent(token, delegate);
         return delegate.responseOrThrow();
     }
--- a/mobile/android/base/java/org/mozilla/gecko/push/PushManager.java
+++ b/mobile/android/base/java/org/mozilla/gecko/push/PushManager.java
@@ -4,28 +4,34 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.push;
 
 import android.support.annotation.NonNull;
 import android.util.Log;
 
 import org.json.JSONObject;
+import org.mozilla.gecko.AppConstants;
 import org.mozilla.gecko.gcm.GcmTokenClient;
 import org.mozilla.gecko.push.autopush.AutopushClientException;
 import org.mozilla.gecko.util.ThreadUtils;
 
 import java.io.IOException;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
 
 /**
- * XXX fill comment.
+ * The push manager advances push registrations, ensuring that the upstream autopush endpoint has
+ * a fresh GCM token.  It brokers channel subscription requests to the upstream and maintains
+ * local state.
+ * <p/>
+ * This class is not thread safe.  An individual instance should be accessed on a single
+ * (background) thread.
  */
 public class PushManager {
     public static final long TIME_BETWEEN_AUTOPUSH_UAID_REGISTRATION_IN_MILLIS = 7 * 24 * 60 * 60 * 1000L; // One week.
 
     public static class ProfileNeedsConfigurationException extends Exception {
         private static final long serialVersionUID = 3326738888L;
 
         public ProfileNeedsConfigurationException() {
@@ -60,17 +66,17 @@ public class PushManager {
             }
         }
         return null;
     }
 
     public Map<String, PushSubscription> allSubscriptionsForProfile(String profileName) {
         final PushRegistration registration = state.getRegistration(profileName);
         if (registration == null) {
-            return new HashMap<>();
+            return Collections.EMPTY_MAP;
         }
         return Collections.unmodifiableMap(registration.subscriptions);
     }
 
     public PushRegistration registerUserAgent(final @NonNull String profileName, final long now) throws ProfileNeedsConfigurationException, AutopushClientException, PushClient.LocalException, GcmTokenClient.NeedsGooglePlayServicesException, IOException {
         Log.i(LOG_TAG, "Registering user agent for profile named: " + profileName);
         return advanceRegistration(profileName, now);
     }
@@ -109,17 +115,17 @@ public class PushManager {
             throw new IllegalStateException("Cannot subscribeChannel with null uaid or secret!");
         }
 
         // Verify endpoint is not null?
         final PushClient pushClient = pushClientFactory.getPushClient(registration.autopushEndpoint, registration.debug);
 
         final SubscribeChannelResponse result = pushClient.subscribeChannel(uaid, secret);
         if (registration.debug) {
-            // Logger.pii(LOG_TAG, "Got chid: " + result.channelID + " and endpoint: " + result.endpoint);
+            Log.i(LOG_TAG, "Got chid: " + result.channelID + " and endpoint: " + result.endpoint);
         } else {
             Log.i(LOG_TAG, "Got chid and endpoint.");
         }
 
         final PushSubscription subscription = new PushSubscription(result.channelID, profileName, result.endpoint, service, serviceData);
         registration.putSubscription(result.channelID, subscription);
         state.checkpoint();
 
@@ -130,29 +136,28 @@ public class PushManager {
         Log.i(LOG_TAG, "Unsubscribing from channel with chid: " + chid);
 
         final PushRegistration registration = registrationForSubscription(chid);
         if (registration == null) {
             Log.w(LOG_TAG, "Cannot find registration corresponding to subscription; not unregistering remote subscription: " + chid);
             return null;
         }
 
+        // We remove the local subscription before the remote subscription:  without the local
+        // subscription we'll ignoring incoming messages, and after some amount of time the
+        // server will expire the channel due to non-activity.  This is also Desktop's approach.
         final PushSubscription subscription = registration.removeSubscription(chid);
         state.checkpoint();
 
         if (subscription == null) {
             // This should never happen.
-            registration.removeSubscription(chid);
-            state.checkpoint();
-
-            Log.e(LOG_TAG, "Subscription does not exist: " + chid);
+            Log.e(LOG_TAG, "Subscription did not exist: " + chid);
             return null;
         }
 
-
         final String uaid = registration.uaid.value;
         final String secret = registration.secret;
         if (uaid == null || secret == null) {
             Log.e(LOG_TAG, "Cannot unsubscribeChannel with null registration uaid or secret!");
             return null;
         }
 
         final PushClient pushClient = pushClientFactory.getPushClient(registration.autopushEndpoint, registration.debug);
@@ -174,19 +179,19 @@ public class PushManager {
 
     public PushRegistration configure(final @NonNull String profileName, final @NonNull String endpoint, final boolean debug, final long now) {
         Log.i(LOG_TAG, "Updating configuration.");
         final PushRegistration registration = state.getRegistration(profileName);
         final PushRegistration newRegistration;
         if (registration != null) {
             if (!endpoint.equals(registration.autopushEndpoint)) {
                 if (debug) {
-                    Log.i(LOG_TAG, "Push configuration autopushEndpoint changed!");
+                    Log.i(LOG_TAG, "Push configuration autopushEndpoint changed! Was: " + registration.autopushEndpoint + "; now: " + endpoint);
                 } else {
-                    Log.i(LOG_TAG, "Push configuration autopushEndpoint changed! Was: " + registration.autopushEndpoint + "; now: " + endpoint);
+                    Log.i(LOG_TAG, "Push configuration autopushEndpoint changed!");
                 }
 
                 newRegistration = new PushRegistration(endpoint, debug, Fetched.now(null), null);
 
                 if (registration.uaid.value != null) {
                     // New endpoint!  All registrations and subscriptions have been dropped, and
                     // should be removed remotely.
                     unregisterUserAgentOnBackgroundThread(registration);
@@ -194,19 +199,19 @@ public class PushManager {
             } else if (debug != registration.debug) {
                 Log.i(LOG_TAG, "Push configuration debug changed: " + debug);
                 newRegistration = registration.withDebug(debug);
             } else {
                 newRegistration = registration;
             }
         } else {
             if (debug) {
-                Log.i(LOG_TAG, "Push configuration set!");
+                Log.i(LOG_TAG, "Push configuration set: " + endpoint + "; debug: " + debug);
             } else {
-                Log.i(LOG_TAG, "Push configuration set: " + endpoint + "; debug: " + debug);
+                Log.i(LOG_TAG, "Push configuration set!");
             }
             newRegistration = new PushRegistration(endpoint, debug, new Fetched(null, now), null);
         }
 
         if (newRegistration != registration) {
             state.putRegistration(profileName, newRegistration);
             state.checkpoint();
         }
@@ -223,81 +228,79 @@ public class PushManager {
                     Log.i(LOG_TAG, "Unregistered user agent with uaid: " + registration.uaid.value);
                 } catch (PushClient.LocalException | AutopushClientException e) {
                     Log.w(LOG_TAG, "Failed to unregister user agent with uaid; ignoring: " + registration.uaid.value, e);
                 }
             }
         });
     }
 
-    public static final String MOZ_ANDROID_GCM_SENDER_ID = "829133274407";
-
     protected @NonNull PushRegistration advanceRegistration(final @NonNull String profileName, final long now) throws ProfileNeedsConfigurationException, AutopushClientException, PushClient.LocalException, GcmTokenClient.NeedsGooglePlayServicesException, IOException {
         final PushRegistration registration = state.getRegistration(profileName);
         if (registration == null || registration.autopushEndpoint == null) {
             Log.i(LOG_TAG, "Cannot advance to registered: registration needs configuration.");
             throw new ProfileNeedsConfigurationException();
         }
         return advanceRegistration(registration, profileName, now);
     }
 
     protected @NonNull PushRegistration advanceRegistration(final PushRegistration registration, final @NonNull String profileName, final long now) throws AutopushClientException, PushClient.LocalException, GcmTokenClient.NeedsGooglePlayServicesException, IOException {
-        final Fetched gcmToken = gcmClient.getToken(MOZ_ANDROID_GCM_SENDER_ID);
-        Log.w(LOG_TAG, "Using GCM token: " + gcmToken.value); // XXX PII
+        final Fetched gcmToken = gcmClient.getToken(AppConstants.MOZ_ANDROID_GCM_SENDERID, registration.debug);
 
         final PushClient pushClient = pushClientFactory.getPushClient(registration.autopushEndpoint, registration.debug);
 
         if (registration.uaid.value == null) {
             if (registration.debug) {
-                // Logger.pii(LOG_TAG, "No uaid; requesting from autopush endpoint: " + registration.autopushEndpoint);
+                Log.i(LOG_TAG, "No uaid; requesting from autopush endpoint: " + registration.autopushEndpoint);
             } else {
                 Log.i(LOG_TAG, "No uaid: requesting from autopush endpoint.");
             }
             final RegisterUserAgentResponse result = pushClient.registerUserAgent(gcmToken.value);
             if (registration.debug) {
-                // Logger.pii(LOG_TAG, "Got uaid: " + result.uaid + " and secret: " + result.secret);
+                Log.i(LOG_TAG, "Got uaid: " + result.uaid + " and secret: " + result.secret);
             } else {
                 Log.i(LOG_TAG, "Got uaid and secret.");
             }
             final long nextNow = System.currentTimeMillis();
             final PushRegistration nextRegistration = registration.withUserAgentID(result.uaid, result.secret, nextNow);
             state.putRegistration(profileName, nextRegistration);
             state.checkpoint();
             return advanceRegistration(nextRegistration, profileName, nextNow);
-        } else if (registration.uaid.timestamp + TIME_BETWEEN_AUTOPUSH_UAID_REGISTRATION_IN_MILLIS < now
+        }
+
+        if (registration.uaid.timestamp + TIME_BETWEEN_AUTOPUSH_UAID_REGISTRATION_IN_MILLIS < now
                 || registration.uaid.timestamp < gcmToken.timestamp) {
             if (registration.debug) {
-                // Logger.pii(LOG_TAG, "Stale uaid; re-registering with autopush endpoint: " + registration.autopushEndpoint);
+                Log.i(LOG_TAG, "Stale uaid; re-registering with autopush endpoint: " + registration.autopushEndpoint);
             } else {
                 Log.i(LOG_TAG, "Stale uaid: re-registering with autopush endpoint.");
             }
 
             pushClient.reregisterUserAgent(registration.uaid.value, registration.secret, gcmToken.value);
 
             Log.i(LOG_TAG, "Re-registered uaid and secret.");
             final long nextNow = System.currentTimeMillis();
             final PushRegistration nextRegistration = registration.withUserAgentID(registration.uaid.value, registration.secret, nextNow);
             state.putRegistration(profileName, nextRegistration);
             state.checkpoint();
             return advanceRegistration(nextRegistration, profileName, nextNow);
-        } else {
-            Log.d(LOG_TAG, "Existing uaid is fresh; no need to request from autopush endpoint.");
         }
 
+        Log.d(LOG_TAG, "Existing uaid is fresh; no need to request from autopush endpoint.");
         return registration;
     }
 
     public void invalidateGcmToken() {
         gcmClient.invalidateToken();
     }
 
     public void startup(long now) {
         try {
             Log.i(LOG_TAG, "Startup: requesting GCM token.");
-            gcmClient.getToken(MOZ_ANDROID_GCM_SENDER_ID); // For side-effects.
+            gcmClient.getToken(AppConstants.MOZ_ANDROID_GCM_SENDERID, false); // For side-effects.
         } catch (GcmTokenClient.NeedsGooglePlayServicesException e) {
             // Requires user intervention.  At App startup, we don't want to address this.  In
             // response to user activity, we do want to try to have the user address this.
             Log.w(LOG_TAG, "Startup: needs Google Play Services.  Ignoring until GCM is requested in response to user activity.");
             return;
         } catch (IOException e) {
             // We're temporarily unable to get a GCM token.  There's nothing to be done; we'll
             // try to advance the App's state in response to user activity or at next startup.
--- a/mobile/android/base/java/org/mozilla/gecko/util/GeckoBackgroundThread.java
+++ b/mobile/android/base/java/org/mozilla/gecko/util/GeckoBackgroundThread.java
@@ -4,17 +4,17 @@
 
 package org.mozilla.gecko.util;
 
 import android.os.Handler;
 import android.os.Looper;
 
 import java.util.concurrent.SynchronousQueue;
 
-public final class GeckoBackgroundThread extends Thread {
+final class GeckoBackgroundThread extends Thread {
     private static final String LOOPER_NAME = "GeckoBackgroundThread";
 
     // Guarded by 'GeckoBackgroundThread.class'.
     private static Handler handler;
     private static Thread thread;
 
     // The initial Runnable to run on the new thread. Its purpose
     // is to avoid us having to wait for the new thread to start.
@@ -47,18 +47,17 @@ public final class GeckoBackgroundThread
         thread = new GeckoBackgroundThread(initialRunnable);
         ThreadUtils.setBackgroundThread(thread);
 
         thread.setDaemon(true);
         thread.start();
     }
 
     // Get a Handler for a looper thread, or create one if it doesn't yet exist.
-    // Public for testing only.
-    public static synchronized Handler getHandler() {
+    /*package*/ static synchronized Handler getHandler() {
         if (thread == null) {
             startThread(null);
         }
 
         while (handler == null) {
             try {
                 GeckoBackgroundThread.class.wait();
             } catch (final InterruptedException e) {
--- a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/push/TestPushManager.java
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/push/TestPushManager.java
@@ -3,49 +3,45 @@
 
 package org.mozilla.gecko.push;
 
 import org.json.JSONObject;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mozilla.gecko.AppConstants;
 import org.mozilla.gecko.background.testhelpers.TestRunner;
 import org.mozilla.gecko.gcm.GcmTokenClient;
-import org.mozilla.gecko.util.GeckoBackgroundThread;
 import org.robolectric.RuntimeEnvironment;
-import org.robolectric.internal.ShadowExtractor;
-import org.robolectric.shadows.ShadowLooper;
 
 import java.util.UUID;
 
 import static org.mockito.Matchers.anyBoolean;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 
 
-// TODO: XXX: notes about timestamps and clocks.
-// TODO: XXX: notes about threading and concurrency.
 @RunWith(TestRunner.class)
 public class TestPushManager {
     private PushState state;
     private GcmTokenClient gcmTokenClient;
     private PushClient pushClient;
     private PushManager manager;
 
     @Before
     public void setUp() throws Exception {
         state = new PushState(RuntimeEnvironment.application, "test.json");
         gcmTokenClient = mock(GcmTokenClient.class);
-        doReturn("opaque-gcm-token").when(gcmTokenClient).getToken(anyString());
+        doReturn(new Fetched("opaque-gcm-token", System.currentTimeMillis())).when(gcmTokenClient).getToken(anyString(), anyBoolean());
 
         // Configure a mock PushClient.
         pushClient = mock(PushClient.class);
         doReturn(new RegisterUserAgentResponse("opaque-uaid", "opaque-secret"))
                 .when(pushClient)
                 .registerUserAgent(anyString());
 
         doReturn(new SubscribeChannelResponse("opaque-chid", "https://localhost:8085/opaque-push-endpoint"))
@@ -106,43 +102,37 @@ public class TestPushManager {
 
         // Changing the debug flag should update but not try to unregister the User Agent.
         registration = manager.configure("default", "http://localhost:8082", true, System.currentTimeMillis());
         assertRegistered(registration, "http://localhost:8082", true);
 
         // Changing the configuration endpoint should update and try to unregister the User Agent.
         registration = manager.configure("default", "http://localhost:8083", true, System.currentTimeMillis());
         assertOnlyConfigured(registration, "http://localhost:8083", true);
-
-//        // But the configuration endpoint should unregister the correct User Agent.
-//        registration = manager.configure("default", "http://localhost:8083", false, System.currentTimeMillis());
-//        assertOnlyConfigured(registration, "http://localhost:8083", false);
-//        // We shouldn't touch the other profile's registration.
-//        assertRegistered(registration1, "http://localhost:8081", false);
     }
 
     @Test
     public void testRegisterMultipleProfiles() throws Exception {
         PushRegistration registration1 = manager.configure("default1", "http://localhost:8081", true, System.currentTimeMillis());
         PushRegistration registration2 = manager.configure("default2", "http://localhost:8082", true, System.currentTimeMillis());
         assertOnlyConfigured(registration1, "http://localhost:8081", true);
         assertOnlyConfigured(registration2, "http://localhost:8082", true);
-        verify(gcmTokenClient, times(0)).getToken(anyString());
+        verify(gcmTokenClient, times(0)).getToken(anyString(), anyBoolean());
 
         registration1 = manager.registerUserAgent("default1", System.currentTimeMillis());
         assertRegistered(registration1, "http://localhost:8081", true);
 
         registration2 = manager.registerUserAgent("default2", System.currentTimeMillis());
         assertRegistered(registration2, "http://localhost:8082", true);
 
         // Just the debug flag should not unregister the User Agent.
         registration1 = manager.configure("default1", "http://localhost:8081", false, System.currentTimeMillis());
         assertRegistered(registration1, "http://localhost:8081", false);
- 
-         // But the configuration endpoint should unregister the correct User Agent.
+
+        // But the configuration endpoint should unregister the correct User Agent.
         registration2 = manager.configure("default2", "http://localhost:8083", false, System.currentTimeMillis());
     }
 
     @Test
     public void testSubscribeChannel() throws Exception {
         manager.configure("default", "http://localhost:8080", false, System.currentTimeMillis());
         PushRegistration registration = manager.registerUserAgent("default", System.currentTimeMillis());
         assertRegistered(registration, "http://localhost:8080", false);
@@ -175,22 +165,16 @@ public class TestPushManager {
         // We should be able to register with non-null serviceData.
         final JSONObject webpushData = new JSONObject();
         webpushData.put("version", 5);
         PushSubscription subscription = manager.subscribeChannel("default", "webpush", webpushData, System.currentTimeMillis());
         assertSubscribed(subscription);
 
         // No exception is success.
         manager.unsubscribeChannel(subscription.chid);
-
-        // The actual unsubscribe request is made on the Gecko background thread; wait for it to
-        // clear before verifying the push client was invoked.
-        // Cribbed from http://stackoverflow.com/a/33928853.
-        ((ShadowLooper) ShadowExtractor.extract(GeckoBackgroundThread.getHandler().getLooper())).idle();
-        verify(pushClient).unsubscribeChannel(registration.uaid.value, registration.secret, subscription.chid);
     }
 
     public void testUnsubscribeUnknownChannel() throws Exception {
         manager.configure("default", "http://localhost:8080", false, System.currentTimeMillis());
         PushRegistration registration = manager.registerUserAgent("default", System.currentTimeMillis());
         assertRegistered(registration, "http://localhost:8080", false);
 
         doThrow(new RuntimeException())
@@ -198,52 +182,43 @@ public class TestPushManager {
                 .unsubscribeChannel(anyString(), anyString(), anyString());
 
         // Un-subscribing from an unknown channel succeeds: we just ignore the request.
         manager.unsubscribeChannel(UUID.randomUUID().toString());
     }
 
     @Test
     public void testStartupBeforeConfiguration() throws Exception {
-        verify(gcmTokenClient, never()).getToken(anyString());
+        verify(gcmTokenClient, never()).getToken(anyString(), anyBoolean());
         manager.startup(System.currentTimeMillis());
-        verify(gcmTokenClient, times(1)).getToken(PushManager.MOZ_ANDROID_GCM_SENDER_ID);
+        verify(gcmTokenClient, times(1)).getToken(AppConstants.MOZ_ANDROID_GCM_SENDERID, false);
     }
 
     @Test
     public void testStartupBeforeRegistration() throws Exception {
         PushRegistration registration = manager.configure("default", "http://localhost:8080", true, System.currentTimeMillis());
         assertOnlyConfigured(registration, "http://localhost:8080", true);
 
         manager.startup(System.currentTimeMillis());
-        verify(gcmTokenClient, times(1)).getToken(anyString());
+        verify(gcmTokenClient, times(1)).getToken(anyString(), anyBoolean());
     }
 
     @Test
     public void testStartupAfterRegistration() throws Exception {
         PushRegistration registration = manager.configure("default", "http://localhost:8080", true, System.currentTimeMillis());
         assertOnlyConfigured(registration, "http://localhost:8080", true);
 
         registration = manager.registerUserAgent("default", System.currentTimeMillis());
         assertRegistered(registration, "http://localhost:8080", true);
 
         manager.startup(System.currentTimeMillis());
 
         // Rather tautological.
         PushRegistration updatedRegistration = manager.state.getRegistration("default");
         Assert.assertEquals(registration.uaid, updatedRegistration.uaid);
-
-        // Now, try well into the future.  Our registration has expired, and we
-        // have no subscriptions, so it should be dropped.
-        long future = System.currentTimeMillis() + 2 * PushManager.TIME_BETWEEN_AUTOPUSH_UAID_REGISTRATION_IN_MILLIS;
-        manager.startup(future);
-
-        ((ShadowLooper) ShadowExtractor.extract(GeckoBackgroundThread.getHandler().getLooper())).idle();
-        verify(pushClient).unregisterUserAgent(anyString(), anyString());
-        Assert.assertTrue(state.registrations.isEmpty());
     }
 
     @Test
     public void testStartupAfterSubscription() throws Exception {
         PushRegistration registration = manager.configure("default", "http://localhost:8080", true, System.currentTimeMillis());
         assertOnlyConfigured(registration, "http://localhost:8080", true);
 
         registration = manager.registerUserAgent("default", System.currentTimeMillis());
@@ -253,19 +228,10 @@ public class TestPushManager {
         assertSubscribed(subscription);
 
         manager.startup(System.currentTimeMillis());
 
         // Rather tautological.
         registration = manager.registrationForSubscription(subscription.chid);
         PushSubscription updatedSubscription = registration.getSubscription(subscription.chid);
         Assert.assertEquals(subscription.chid, updatedSubscription.chid);
-
-        // Now, try well into the future.  Our registration has expired, but we
-        // have subscriptions, so it shouldn't be dropped.
-        long future = System.currentTimeMillis() + 2 * PushManager.TIME_BETWEEN_AUTOPUSH_UAID_REGISTRATION_IN_MILLIS;
-        manager.startup(future);
-
-        ((ShadowLooper) ShadowExtractor.extract(GeckoBackgroundThread.getHandler().getLooper())).idle();
-        verify(pushClient, never()).unregisterUserAgent(anyString(), anyString());
-        Assert.assertNotNull(manager.registrationForSubscription(subscription.chid));
     }
 }