Ensure we never try to register for the object id "NULL"

sync/android/java/src/org/chromium/sync/notifier/InvalidationService.java

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.sync.notifier;
 
 import android.accounts.Account;
 import android.app.PendingIntent;
 import android.content.ContentResolver;
 import android.content.Intent;
@@ skipping 278 matching lines @@
      * Reads the saved sync types from storage (if any) and returns a set containing the
      * corresponding object ids.
      */
     @VisibleForTesting
     Set<ObjectId> readRegistrationsFromPrefs() {
         Set<String> savedTypes = new InvalidationPreferences(this).getSavedSyncedTypes();
         if (savedTypes == null) {
             return Collections.emptySet();
         } else {
             Set<ModelType> modelTypes = ModelType.syncTypesToModelTypes(savedTypes);
+            modelTypes = ModelType.stripNonInvalidationTypes(modelTypes);
             Set<ObjectId> objectIds = Sets.newHashSetWithExpectedSize(modelTypes.size());

[Yaron, 2013/08/08 01:52:16]

These 4 lines can be replaced with ModelType.modelTypesToObjectIds.
That said we're now iterating through all types 3 times, creating copies each
time (or maybe twice depending on other comment). Seems suboptimal.

Also, is it too weird to just do this in modelTypesToObjectIds? You'd never want
the null type there, so you could strip it at conversion time and leave sync
types alone.

[nyquist, 2013/08/08 17:59:28]

Simplified this, and moved logic to modelTypesToObjectIds.
Still iterates twice though, but code is still fairly easy to read.

The manual call to toObjectId seems to only be used in tests, so added an
explanatory comment that it should be used with caution.

             for (ModelType modelType : modelTypes) {
                 objectIds.add(modelType.toObjectId());
             }
             return objectIds;
         }
     }
 
     /**
      * Sets the types for which notifications are required to {@code syncTypes}. {@code syncTypes}
      * is either a list of specific types or the special wildcard type
@@ skipping 21 matching lines @@
         if (sClientId == null) {
             return;
         }
 
         // We do have a ready client. Unregister any existing registrations not present in the
         // new set and register any elements in the new set not already present. This call does
         // expansion of the ALL_TYPES_TYPE wildcard.
         // NOTE: syncTypes MUST NOT be used below this line, since it contains an unexpanded
         // wildcard.
         Set<ModelType> newRegisteredTypes = ModelType.syncTypesToModelTypes(syncTypes);
+        // Some types should never receive notifications.
+        newRegisteredTypes = ModelType.stripNonInvalidationTypes(newRegisteredTypes);
 
         List<ObjectId> unregistrations = Lists.newArrayList();
         List<ObjectId> registrations = Lists.newArrayList();
         computeRegistrationOps(existingRegistrations,
                 ModelType.modelTypesToObjectIds(newRegisteredTypes), registrations,
                 unregistrations);
         unregister(sClientId, unregistrations);
         register(sClientId, registrations);
     }
 
@@ skipping 104 matching lines @@
     }
 
     private static void setClientId(byte[] clientId) {
         sClientId = clientId;
     }
 
     private static void setIsClientStarted(boolean isStarted) {
         sIsClientStarted = isStarted;
     }
 }