Add return values to SyncerCommand

chrome/browser/sync/engine/syncer_error.h

+// Copyright (c) 2011 The Chromium Authors. All rights reserved.

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

2012

[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.
+
+#ifndef CHROME_BROWSER_SYNC_ENGINE_SYNCER_ERROR_H_
+#define CHROME_BROWSER_SYNC_ENGINE_SYNCER_ERROR_H_
+#pragma once
+
+namespace browser_sync {
+
+enum SyncerError {

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

Please add a comment above here describing what this enum is for and how it
differs from other, similarly named error enums. (Or make it a file comment, up
above the ifdefs. Up to you.)

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

Done.

+  UNINITIALIZED = 0,       // default value

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

style nit - Comments must begin with a capital letter and end with a period.

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

Maybe INVALID?  UNINITIALIZED seems ambiguous...

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

The way I see it, there is a difference.  INVALID means that the value doesn't
make sense.  UNINITIALIZED means that no one has bothered to change the value
from its default.  I would have made separate enum values for both of these,
except that we currently have no use for INVALID.  

Unfortunately, we're going to need check for UNINITIALIZED in a few cases.  The
current structure of the code is such that we will need to return a value for
"PostCommitMessageCommand", regardless of whether or not it is actually
executed.  We can check that return value against UNINITIALIZED to see whether
or not we attempted to post a commit during that sync cycle.  

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

Fair enough.  Well, maybe "UNSET" or a synonym then?  The point I was aiming at
was if we're looking at code / debugging (or looking at values in a debugger and
talking about them in person),

SyncerError e;

is uninitialized.  But

SyncerError e(UNINITIALIZED);

is too.

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

Good point.  I'll change it to UNSET.

+  DIRECTORY_LOOKUP_FAILED, // local directory lookup failure
+
+  NETWORK_CONNECTION_UNAVAILABLE,  // connectivity failure
+  NETWORK_IO_ERROR,                // response buffer read error
+  SYNC_SERVER_ERROR,               // non auth HTTP error
+  SYNC_AUTH_ERROR,                 // HTTP auth error
+
+  // Based on values returned by server.  Most are defined in sync.proto.
+  SERVER_RETURN_INVALID_CREDENTIAL,
+  SERVER_RETURN_UNKNOWN_ERROR,
+  SERVER_RETURN_THROTTLED,
+  SERVER_RETURN_TRANSIENT_ERROR,
+  SERVER_RETURN_MIGRATION_DONE,
+  SERVER_RETURN_CLEAR_PENDING,
+  SERVER_RETURN_NOT_MY_BIRTHDAY,
+  SERVER_RESPONSE_VALIDATION_FAILED,
+
+  NO_ERROR,
+};
+
+const char * GetSyncerErrorString(SyncerError);
+
+}  // namespace browser_sync
+
+#endif  // CHROME_BROWSER_SYNC_ENGINE_SYNCER_ERROR_H_