Browser test for the push messaging feature

chrome/browser/sync/profile_sync_service.cc

 // Copyright (c) 2012 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/sync/profile_sync_service.h"
 
 #include <cstddef>
 #include <map>
 #include <set>
 #include <utility>
@@ skipping 61 matching lines @@
 using browser_sync::DataTypeController;
 using browser_sync::DataTypeManager;
 using browser_sync::SyncBackendHost;
 using syncer::ModelType;
 using syncer::ModelTypeSet;
 using syncer::JsBackend;
 using syncer::JsController;
 using syncer::JsEventDetails;
 using syncer::JsEventHandler;
 using syncer::ModelSafeRoutingInfo;
+using syncer::ObjectIdSet;

[dcheng, 2012/09/13 21:00:48]

If you want to add using lines for this, you should remove redundant syncer::
prefixes from the other places in this file that refer to these types.
Otherwise, I would just remove these using lines and use syncer:: for
consistency.

[Pete Williamson, 2012/09/14 19:14:48]

Done.

+using syncer::ObjectIdStateMap;
 using syncer::SyncCredentials;
 using syncer::SyncProtocolError;
 using syncer::WeakHandle;
 
 typedef GoogleServiceAuthError AuthError;
 
 const char* ProfileSyncService::kSyncServerUrl =
     "https://clients4.google.com/chrome-sync";
 
 const char* ProfileSyncService::kDevServerUrl =
@@ skipping 373 matching lines @@
 
 void ProfileSyncService::UnregisterInvalidationHandler(
     syncer::InvalidationHandler* handler) {
   invalidator_registrar_.UnregisterHandler(handler);
 }
 
 syncer::InvalidatorState ProfileSyncService::GetInvalidatorState() const {
   return invalidator_registrar_.GetInvalidatorState();
 }
 
+// test method for system test to allow a test to send in an invalidation
+void ProfileSyncService::SendNotificationForTest(
+    const std::string& id_name,

[dcheng, 2012/09/13 21:00:48]

Why not have this take an ObjectId or an ObjectIdSet as an argument? Then we can
just pass it directly to OnIncomingInvalidation() after we call
ObjectIdSetToStateMap(ids, payload)

[Pete Williamson, 2012/09/14 19:14:48]

Since we have to look up the name in the invalidator registrar, it seems to me
that we have better encapsulation if we leave the logic to do the name lookup
here, instead of trying to do this from the test code.  Also, as best I can
tell, invalidator_registrar_ is not exposed to clients of the ProfileSyncService
class, so the test code would have no way to walk the list of ObjectIds looking
for the right one.

[dcheng, 2012/09/17 18:39:15]

I don't quite understand what you mean. You don't need to use
InvalidationRegistrar outside do you? You just pass in a constructed object ID
with the correct source ID and name. OnIncomingInvalidation has to do the lookup
either way, so this just results in a double lookup, essentially.

+    const std::string& payload) {
+  // the list of ids to notify
+  ObjectIdSet notify_ids;
+
+  // find a registered id (if any) with the same name
+  ObjectIdSet registered_ids = invalidator_registrar_.GetAllRegisteredIds();
+  ObjectIdSet::const_iterator it = registered_ids.begin();
+  for (; it != registered_ids.end(); ++it) {
+    if (0 == id_name.compare(it->name())) {
+      // if we find a registered id with the same name, insert a copy of it
+      // into the list of ids we will notify
+      notify_ids.insert(invalidation::ObjectId(it->source(), it->name()));
+    }
+  }
+
+  // build an object state map, and use it to forward the call to
+  // registered clients such as PushMessagingHandler::OnIncomingNotification
+  // as if this came from the server
+  const ObjectIdStateMap& id_state_map =
+      ObjectIdSetToStateMap(notify_ids, payload);
+  OnIncomingInvalidation(id_state_map, syncer::REMOTE_INVALIDATION);
+}
+
 void ProfileSyncService::Shutdown() {
   ShutdownImpl(false);
 }
 
 void ProfileSyncService::ShutdownImpl(bool sync_disabled) {
   // First, we spin down the backend and wait for it to stop syncing completely
   // before we Stop the data type manager.  This is to avoid a late sync cycle
   // applying changes to the sync db that wouldn't get applied via
   // ChangeProcessors, leading to back-from-the-dead bugs.
   base::Time shutdown_start_time = base::Time::Now();
@@ skipping 1329 matching lines @@
   // See http://stackoverflow.com/questions/6224121/is-new-this-myclass-undefined-behaviour-after-directly-calling-the-destru.
   ProfileSyncService* old_this = this;
   this->~ProfileSyncService();
   new(old_this) ProfileSyncService(
       new ProfileSyncComponentsFactoryImpl(profile,
                                            CommandLine::ForCurrentProcess()),
       profile,
       signin,
       behavior);
 }