Add return values to SyncerCommand

chrome/browser/sync/engine/syncer_command.h

Index: chrome/browser/sync/engine/syncer_command.h
diff --git a/chrome/browser/sync/engine/syncer_command.h b/chrome/browser/sync/engine/syncer_command.h
index 2aad6530b4c0decc709be09009cb7f95d7db407d..dce4a6bf158ba0cea7af26ebf5f552c3877caddb 100644
--- a/chrome/browser/sync/engine/syncer_command.h
+++ b/chrome/browser/sync/engine/syncer_command.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.

[tim (not reviewing), 2012/01/06 16:33:42]

FYI, you'll have to change these again :)

[rlarocque, 2012/01/06 19:44:21]

Done.

 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -8,6 +8,8 @@
 
 #include "base/basictypes.h"
 
+#include "chrome/browser/sync/engine/syncer_error.h"
+
 namespace browser_sync {
 
 namespace sessions {
@@ -32,10 +34,10 @@ class SyncerCommand {
   virtual ~SyncerCommand();
 
   // Execute dispatches to a derived class's ExecuteImpl.
-  void Execute(sessions::SyncSession* session);
+  SyncerError Execute(sessions::SyncSession* session);

[tim (not reviewing), 2012/01/06 16:33:42]

I think we should slap WARN_UNUSED_RESULT on this.  You'll have to edit
syncer.cc, but for now something like if (cmd.Execute() == UNINITIALIZED)
NOTREACHED() should be sufficient if there's nothing more intelligent to do.

[rlarocque, 2012/01/06 19:44:21]

I don't see a point in adding WARN_UNUSED_RESULT if we're going to hack around
it.  Maybe we should wait until the error detection and handling is more fully
baked?

[tim (not reviewing), 2012/01/06 20:07:13]

I admit, that's true.  But I do think long term it makes sense, unless things
change considerably (as you may be alluding to with the fully-baked comment). 
If you can't come up with something useful to do now with the result values, I
guess this is fine.  Consider a TODO.

[rlarocque, 2012/01/06 21:25:34]

I think the best way to track this is with crbug.com/109422.  It might be worth
an OKR, too.  

 
   // ExecuteImpl is where derived classes actually do work.
-  virtual void ExecuteImpl(sessions::SyncSession* session) = 0;
+  virtual SyncerError ExecuteImpl(sessions::SyncSession* session) = 0;
  private:
   void SendNotifications(sessions::SyncSession* session);
   DISALLOW_COPY_AND_ASSIGN(SyncerCommand);