Track merged nudge sources

sync/sessions/sync_session.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 "sync/sessions/sync_session.h"
 
 #include <algorithm>
 #include <iterator>
 
 #include "base/logging.h"
@@ skipping 62 matching lines @@
     const std::vector<ModelSafeWorker*>& workers)
     : context_(context),
       source_(source),
       write_transaction_(NULL),
       delegate_(delegate),
       workers_(workers),
       routing_info_(routing_info),
       enabled_groups_(ComputeEnabledGroups(routing_info_, workers_)) {
   status_controller_.reset(new StatusController(routing_info_));
   std::sort(workers_.begin(), workers_.end());
+  merged_sources_log_.push_back(source_);
 }
 
 SyncSession::~SyncSession() {}
 
 void SyncSession::Coalesce(const SyncSession& session) {
   if (context_ != session.context() || delegate_ != session.delegate_) {
     NOTREACHED();
     return;
   }
 
   // When we coalesce sessions, the sync update source gets overwritten with the

[tim (not reviewing), 2012/11/27 23:48:47]

Update comment.   Also, you say "merged" but you're only appending to a vector,
right?  In some instances we'll actually wind up appending a source that is
known to be the same as the one that's already in merged_sources_log_.  Should
we be smart about that, even though I think the only two cases are line 271 and
1023 in sync_scheduler_impl.cc, both of which should be removed?

[rlarocque, 2012/11/28 00:33:58]

I think the comment should stay the same.  The 'merged_sources_log' is only used
for debugging.  The coalesce logic for the source and payloads we report to the
server remains the same.

When I wrote this, I was thinking of "merged_sources_log" as a log of the
sources that have been merged into the one we finally ended up using.  The log
itself is not merged.  I can see how the name can be misleading, though.  I
should use a different name for this.  How about debug_info_sources_list?

I can't think of a strong reason to be smart about merging common sources. 
AFAIK, the costs of storing and sending multiple copies is not that high.  It
could be useful to see the sequence of events that led to a sync cycle exactly
as the syncer saw them.

Also, I'm not sure I understand your comment about the two cases.  What about
lines 502 and 632?

   // most recent, while the type/state map gets merged.
+  merged_sources_log_.push_back(session.source_);
   CoalesceStates(&source_.types, session.source_.types);
   source_.updates_source = session.source_.updates_source;
 
   std::vector<ModelSafeWorker*> temp;
   std::set_union(workers_.begin(), workers_.end(),
                  session.workers_.begin(), session.workers_.end(),
                  std::back_inserter(temp));
   workers_.swap(temp);
 
   // We have to update the model safe routing info to the union. In case the
@@ skipping 65 matching lines @@
   SyncSessionSnapshot snapshot(
       status_controller_->model_neutral_state(),
       is_share_useable,
       initial_sync_ended,
       download_progress_markers,
       delegate_->IsSyncingCurrentlySilenced(),
       status_controller_->num_encryption_conflicts(),
       status_controller_->num_hierarchy_conflicts(),
       status_controller_->num_server_conflicts(),
       source_,
+      merged_sources_log_,
       context_->notifications_enabled(),
       dir->GetEntriesCount(),
       status_controller_->sync_start_time(),
       num_entries_by_type,
       num_to_delete_entries_by_type);
 
   return snapshot;
 }
 
 void SyncSession::SendEventNotification(SyncEngineEvent::EventCause cause) {
@@ skipping 10 matching lines @@
 
 bool SyncSession::DidReachServer() const {
   const ModelNeutralState& state = status_controller_->model_neutral_state();
   return state.last_get_key_result >= FIRST_SERVER_RETURN_VALUE ||
       state.last_download_updates_result >= FIRST_SERVER_RETURN_VALUE ||
       state.commit_result >= FIRST_SERVER_RETURN_VALUE;
 }
 
 }  // namespace sessions
 }  // namespace syncer