Enable invalidations for arbitrary objects on Android.

chrome/browser/invalidation/invalidation_service_android.cc

 // 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.
 
 #include "chrome/browser/invalidation/invalidation_service_android.h"
 
+#include "base/android/jni_array.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "content/public/browser/notification_service.h"
+#include "google/cacheinvalidation/types.pb.h"
+#include "sync/jni/InvalidationController_jni.h"
 
 namespace invalidation {
 
 InvalidationServiceAndroid::InvalidationServiceAndroid(Profile* profile)
     : invalidator_state_(syncer::INVALIDATIONS_ENABLED) {
   DCHECK(CalledOnValidThread());
   registrar_.Add(this, chrome::NOTIFICATION_SYNC_REFRESH_REMOTE,
                  content::Source<Profile>(profile));
 }
 
 InvalidationServiceAndroid::~InvalidationServiceAndroid() { }
 
 void InvalidationServiceAndroid::RegisterInvalidationHandler(
     syncer::InvalidationHandler* handler) {
   DCHECK(CalledOnValidThread());
   invalidator_registrar_.RegisterHandler(handler);
 }
 
 void InvalidationServiceAndroid::UpdateRegisteredInvalidationIds(
     syncer::InvalidationHandler* handler,
     const syncer::ObjectIdSet& ids) {
   DCHECK(CalledOnValidThread());
+  // Registration with the Android invalidation controller is needed if any of
+  // the objects in the current or replacement object sets require it.
+  bool isRegistrationRequired = IsRegistrationRequired(ids) ||
+      IsRegistrationRequired(invalidator_registrar_.GetRegisteredIds(handler));
   invalidator_registrar_.UpdateRegisteredIds(handler, ids);
+  if (isRegistrationRequired)
+    UpdateRegistration();
 }
 
 void InvalidationServiceAndroid::UnregisterInvalidationHandler(
     syncer::InvalidationHandler* handler) {
   DCHECK(CalledOnValidThread());
   invalidator_registrar_.UnregisterHandler(handler);
 }
 
 void InvalidationServiceAndroid::AcknowledgeInvalidation(
     const invalidation::ObjectId& id,
@@ skipping 39 matching lines @@
       effective_invalidation_map);
 }
 
 void InvalidationServiceAndroid::TriggerStateChangeForTest(
     syncer::InvalidatorState state) {
   DCHECK(CalledOnValidThread());
   invalidator_state_ = state;
   invalidator_registrar_.UpdateInvalidatorState(invalidator_state_);
 }
 
+void InvalidationServiceAndroid::SetUpdateRegistrationCallbackForTest(
+    const UpdateRegistrationCallback& callback) {
+  update_registration_callback_ = callback;
+}
+
+void InvalidationServiceAndroid::UpdateRegistration() {
+  // Create a list of object sources and names which require registration.
+  std::vector<int> sources;
+  std::vector<std::string> names;
+  syncer::ObjectIdSet ids = invalidator_registrar_.GetAllRegisteredIds();
+  syncer::ObjectIdSet::const_iterator id;
+  for (id = ids.begin(); id != ids.end(); ++id) {
+    if (IsRegistrationRequired(*id)) {
+      sources.push_back(id->source());
+      names.push_back(id->name());
+    }
+  }
+
+  // Invoke callback for testing if provided.
+  if (!update_registration_callback_.is_null()) {

[rlarocque, 2013/09/09 18:32:23]

I'd prefer to keep test code out of this class, if possible.

Come to think of it, it would also be nice to keep the JNI code quarantined
somewhere.

Could this code be refactored so that we have some sort of
"AndroidInvalidationRegistrationTracker" interface that this class communicates
with?  (Feel free to pick a better name for it.)  You could use this both to
hide the JNI bits and to inject a mock instance for testing.

[Steve Condie, 2013/09/09 22:58:26]

Done. For lack of a better name, I called it InvalidationControllerAndroid, as
it is essentially just a wrapper for the Java-side InvalidationController class.
Now the unit test can subclass InvalidationControllerAndroid for a clean way to
inject a mock.

+    update_registration_callback_.Run(sources, names);
+    return;
+  }
+
+  // Get a reference to the invalidation controller.
+  JNIEnv* env = base::android::AttachCurrentThread();
+  DCHECK(env);
+  if (invalidation_controller_.is_null()) {
+    invalidation_controller_.Reset(Java_InvalidationController_get(
+        env,
+        base::android::GetApplicationContext()));
+  }
+
+  // Update the registration with the invalidation controller.
+  Java_InvalidationController_setRegisteredObjectIds(
+      env,
+      invalidation_controller_.obj(),
+      base::android::ToJavaIntArray(env, sources).obj(),
+      base::android::ToJavaArrayOfStrings(env, names).obj());
+}
+
+bool InvalidationServiceAndroid::IsRegistrationRequired(
+    const syncer::ObjectIdSet& ids) {
+  syncer::ObjectIdSet::const_iterator id;
+  for (id = ids.begin(); id != ids.end(); ++id)
+    if (IsRegistrationRequired(*id))
+      return true;
+  return false;
+}
+
+bool InvalidationServiceAndroid::IsRegistrationRequired(

[rlarocque, 2013/09/09 18:32:23]

Could we put this filtering logic on the Java side?

It's the Java classes that have the sync-specific registration logic.  It seems
logical that they should also manage the filtering that is required by this
sync-specific special case.

[Steve Condie, 2013/09/09 22:58:26]

I think that's a good idea; done. My only concern was the overhead with calling
into Java and writing to preferences (where registrations are persisted) extra
times, but I think the simplicity of the new approach is worth it, and there
should only be a small number of calls to update registrations anyway.

+    const invalidation::ObjectId& id) {
+  // Sync types are registered on the Java side, so only non-Sync objects must
+  // be registered with the Android invalidation controller by this class.
+  return id.source() != ipc::invalidation::ObjectSource::CHROME_SYNC;
+}
+
+bool RegisterInvalidationController(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}
+
 } //  namespace invalidation