diff options
author | dsmyers@chromium.org <dsmyers@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-01-29 23:08:36 +0000 |
---|---|---|
committer | dsmyers@chromium.org <dsmyers@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-01-29 23:08:36 +0000 |
commit | 42b5f9fed5a71037985843d4b15dde5d2322c56c (patch) | |
tree | 14f385c2ebf91ea5dee5f2239f00707fa3537991 /sync | |
parent | e42e1f590dc6db52bd67704d339a2221da89b238 (diff) | |
download | chromium_src-42b5f9fed5a71037985843d4b15dde5d2322c56c.zip chromium_src-42b5f9fed5a71037985843d4b15dde5d2322c56c.tar.gz chromium_src-42b5f9fed5a71037985843d4b15dde5d2322c56c.tar.bz2 |
Record client id in ready(), not reissueRegs().
BUG=172390
Review URL: https://chromiumcodereview.appspot.com/12096011
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@179454 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sync')
-rw-r--r-- | sync/android/java/src/org/chromium/sync/notifier/InvalidationService.java | 18 | ||||
-rw-r--r-- | sync/android/javatests/src/org/chromium/sync/notifier/InvalidationServiceTest.java | 27 |
2 files changed, 39 insertions, 6 deletions
diff --git a/sync/android/java/src/org/chromium/sync/notifier/InvalidationService.java b/sync/android/java/src/org/chromium/sync/notifier/InvalidationService.java index 2ace93c..8adbd4b 100644 --- a/sync/android/java/src/org/chromium/sync/notifier/InvalidationService.java +++ b/sync/android/java/src/org/chromium/sync/notifier/InvalidationService.java @@ -170,6 +170,11 @@ public class InvalidationService extends AndroidListener { @Override public void ready(byte[] clientId) { + setClientId(clientId); + + // We might have accumulated some registrations to do while we were waiting for the client + // to become ready. + reissueRegistrations(clientId); } @Override @@ -178,10 +183,6 @@ public class InvalidationService extends AndroidListener { if (!desiredRegistrations.isEmpty()) { register(clientId, desiredRegistrations); } - // TODO(dsmyers): [misc] reissue registrations is not guaranteed to be called by spec, - // although it will happen in practice. This code should be changed not to rely on it. - // Bug: https://code.google.com/p/chromium/issues/detail?id=172390 - setClientId(clientId); } @Override @@ -316,8 +317,7 @@ public class InvalidationService extends AndroidListener { prefs.commit(editContext); // If we do not have a ready invalidation client, we cannot change its registrations, so - // return. Later, when the client is ready, we will get a reissueRegistrations upcall and - // will supply the new registrations then. + // return. Later, when the client is ready, we will supply the new registrations. if (sClientId == null) { return; } @@ -431,6 +431,12 @@ public class InvalidationService extends AndroidListener { return sIsClientStarted; } + /** Returns the notification client id, for tests. */ + @VisibleForTesting + @Nullable static byte[] getClientIdForTest() { + return sClientId; + } + /** Returns the client name used for the notification client. */ private static byte[] getClientName() { // TODO(dsmyers): we should use the same client name as the native sync code. diff --git a/sync/android/javatests/src/org/chromium/sync/notifier/InvalidationServiceTest.java b/sync/android/javatests/src/org/chromium/sync/notifier/InvalidationServiceTest.java index cfb8c0c..a66f238 100644 --- a/sync/android/javatests/src/org/chromium/sync/notifier/InvalidationServiceTest.java +++ b/sync/android/javatests/src/org/chromium/sync/notifier/InvalidationServiceTest.java @@ -127,6 +127,33 @@ public class InvalidationServiceTest extends ServiceTestCase<TestableInvalidatio @SmallTest @Feature({"Sync"}) + public void testReady() { + /** + * Test plan: call ready. Verify that the service sets the client id correctly and reissues + * pending registrations. + */ + + // Persist some registrations. + InvalidationPreferences invPrefs = new InvalidationPreferences(getContext()); + EditContext editContext = invPrefs.edit(); + invPrefs.setSyncTypes(editContext, Lists.newArrayList("BOOKMARK", "SESSION")); + assertTrue(invPrefs.commit(editContext)); + + // Issue ready. + getService().ready(CLIENT_ID); + assertTrue(Arrays.equals(CLIENT_ID, InvalidationService.getClientIdForTest())); + byte[] otherCid = "otherCid".getBytes(); + getService().ready(otherCid); + assertTrue(Arrays.equals(otherCid, InvalidationService.getClientIdForTest())); + + // Verify registrations issued. + assertEquals( + Sets.newHashSet(ModelType.BOOKMARK.toObjectId(), ModelType.SESSION.toObjectId()), + Sets.newHashSet(getService().mRegistrations.get(0))); + } + + @SmallTest + @Feature({"Sync"}) public void testReissueRegistrations() { /* * Test plan: call the reissueRegistrations method of the listener with both empty and |