[Sync] Make syncer commands avoid posting tasks on threads with no work to do

chrome/browser/sync/engine/process_updates_command.cc

 // Copyright (c) 2011 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/engine/process_updates_command.h"
 
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/location.h"
 #include "chrome/browser/sync/engine/syncer.h"
 #include "chrome/browser/sync/engine/syncer_proto_util.h"
 #include "chrome/browser/sync/engine/syncer_util.h"
 #include "chrome/browser/sync/engine/syncproto.h"
 #include "chrome/browser/sync/sessions/sync_session.h"
 #include "chrome/browser/sync/syncable/directory_manager.h"
 #include "chrome/browser/sync/syncable/syncable.h"
 
 using std::vector;
 
 namespace browser_sync {
 
 using sessions::SyncSession;
 using sessions::StatusController;
+using sessions::UpdateProgress;
 
 ProcessUpdatesCommand::ProcessUpdatesCommand() {}
 ProcessUpdatesCommand::~ProcessUpdatesCommand() {}
 
+std::set<ModelSafeGroup> ProcessUpdatesCommand::GetGroupsToChange(
+    const sessions::SyncSession& session) const {
+  std::set<ModelSafeGroup> groups_with_verified_updates;
+
+  const std::set<ModelSafeGroup>& enabled_groups = session.GetEnabledGroups();
+
+  for (std::set<ModelSafeGroup>::const_iterator it =
+           enabled_groups.begin(); it != enabled_groups.end(); ++it) {
+    const UpdateProgress* update_progress =
+        session.status_controller().GetUnrestrictedUpdateProgress(*it);
+    if (update_progress &&
+        (update_progress->VerifiedUpdatesBegin() !=
+         update_progress->VerifiedUpdatesEnd())) {
+      groups_with_verified_updates.insert(*it);
+    }
+  }
+
+  return groups_with_verified_updates;

[tim (not reviewing), 2011/11/29 00:01:59]

again, maybe just keep an index on this when we add to VerifiedUpdates?  Seems
like it would be less code, less work, and more isolated...

[akalin, 2011/11/30 01:33:03]

I don't know if I agree with this.  Unlike the unapplied updates case, this is
just looping over a small (max 6 elements) set, and then looking up and
comparing a bunch of pointers.  I don't think it's less code or less work to
keep an index.

As for more isolated, I could move this code into SyncSession if you want. 
Similar code is already in there, with GetEnabledGroupsWithConflicts().  I just
didn't move this one because it's only used once.

[tim (not reviewing), 2011/11/30 02:21:23]

OK, yeah lets do that.

+}
+
 bool ProcessUpdatesCommand::ModelNeutralExecuteImpl(SyncSession* session) {
   const GetUpdatesResponse& updates =
       session->status_controller().updates_response().get_updates();
   const int update_count = updates.entries_size();
 
   // Don't bother processing updates if there were none.
   return update_count != 0;
 }
 
 void ProcessUpdatesCommand::ModelChangingExecuteImpl(SyncSession* session) {
@@ skipping 107 matching lines @@
     // Force application of this update, no matter what.
     target_entry.Put(syncable::IS_UNAPPLIED_UPDATE, true);
   }
 
   SyncerUtil::UpdateServerFieldsFromUpdate(&target_entry, update, name);
 
   return SUCCESS_PROCESSED;
 }
 
 }  // namespace browser_sync