[Sync] InvalidationService shouldn't CHECK when registering ObjectId for more than one handler

components/invalidation/invalidator_registrar.cc

 // Copyright 2014 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 "components/invalidation/invalidator_registrar.h"
 
 #include <cstddef>
 #include <iterator>
 #include <utility>
 
@@ skipping 10 matching lines @@
   CHECK(handler_to_ids_map_.empty());
 }
 
 void InvalidatorRegistrar::RegisterHandler(InvalidationHandler* handler) {
   DCHECK(thread_checker_.CalledOnValidThread());
   CHECK(handler);
   CHECK(!handlers_.HasObserver(handler));
   handlers_.AddObserver(handler);
 }
 
-void InvalidatorRegistrar::UpdateRegisteredIds(
-    InvalidationHandler* handler,
-    const ObjectIdSet& ids) {
+bool InvalidatorRegistrar::UpdateRegisteredIds(InvalidationHandler* handler,
+                                               const ObjectIdSet& ids) {
   DCHECK(thread_checker_.CalledOnValidThread());
   CHECK(handler);
   CHECK(handlers_.HasObserver(handler));
 
   for (HandlerIdsMap::const_iterator it = handler_to_ids_map_.begin();
        it != handler_to_ids_map_.end(); ++it) {
     if (it->first == handler) {
       continue;
     }
 
     std::vector<invalidation::ObjectId> intersection;
     std::set_intersection(
         it->second.begin(), it->second.end(),
         ids.begin(), ids.end(),
         std::inserter(intersection, intersection.end()),
         ObjectIdLessThan());
-    CHECK(intersection.empty())
-        << "Duplicate registration: trying to register "
-        << ObjectIdToString(*intersection.begin()) << " for "
-        << handler << " when it's already registered for "
-        << it->first;
+    if (!intersection.empty()) {
+      DVLOG(0) << "Duplicate registration: trying to register "

[maniscalco, 2015/05/18 15:42:28]

Can you walk me through the thought process on the log level / severity to use
here?  This case is definitely an error of some kind, right?  Would LOG(ERROR) 
more appropriate?

[pavely, 2015/05/18 18:38:04]

I think LOG(ERROR) and DVLOG(0) have the same effect, but LOG(ERROR) is more
widely used and more descriptive so LOG(ERROR) it is.

[maniscalco, 2015/05/18 18:56:05]

Cool.  As we discussed offline, the V in DVLOG stands for verbose so DVLOG(0) is
actually an "INFO".

+               << ObjectIdToString(*intersection.begin()) << " for " << handler
+               << " when it's already registered for " << it->first;
+      return false;
+    }
   }
 
   if (ids.empty()) {
     handler_to_ids_map_.erase(handler);
   } else {
     handler_to_ids_map_[handler] = ids;
   }
+  return true;
 }
 
 void InvalidatorRegistrar::UnregisterHandler(InvalidationHandler* handler) {
   DCHECK(thread_checker_.CalledOnValidThread());
   CHECK(handler);
   CHECK(handlers_.HasObserver(handler));
   handlers_.RemoveObserver(handler);
   handler_to_ids_map_.erase(handler);
 }
 
@@ skipping 67 matching lines @@
   DCHECK(thread_checker_.CalledOnValidThread());
   return handlers_.HasObserver(handler);
 }
 
 void InvalidatorRegistrar::DetachFromThreadForTest() {
   DCHECK(thread_checker_.CalledOnValidThread());
   thread_checker_.DetachFromThread();
 }
 
 }  // namespace syncer