[Sync] Add support for sync Persistence Errors

[Sync] Add support for sync Persistence Errors

Persistence errors are those detected by the native model's transaction
version being newer than sync's. They currently have no effect beyond
a normal association error, but will eventually result in delayed
association after a sync update.

BUG=239828
TBR=atwilson@chromium.org, sky@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=209810

[Nicolas Zea, 2013-06-07 21:07:50]

PTAL

[haitaol1, 2013-06-10 22:47:56]

https://codereview.chromium.org/15701022/diff/11001/sync/api/sync_error.h
File sync/api/sync_error.h (right):

https://codereview.chromium.org/15701022/diff/11001/sync/api/sync_error.h#new...
sync/api/sync_error.h:51: // Create a new Sync error of type |error_type|
triggered by |model_type|
Why don't combine these two constructors?

[Nicolas Zea, 2013-06-20 18:31:33]

https://codereview.chromium.org/15701022/diff/11001/sync/api/sync_error.h
File sync/api/sync_error.h (right):

https://codereview.chromium.org/15701022/diff/11001/sync/api/sync_error.h#new...
sync/api/sync_error.h:51: // Create a new Sync error of type |error_type|
triggered by |model_type|
On 2013/06/10 22:47:56, haitaol1 wrote:
> Why don't combine these two constructors?

This constructor autogenerates the message, to I think it's cleaner to not
provide the message paramater. Otherwise it's ambiguous when the message should
or not should be set.

[haitaol1, 2013-06-20 20:35:51]

https://codereview.chromium.org/15701022/diff/11001/sync/api/sync_error.h
File sync/api/sync_error.h (right):

https://codereview.chromium.org/15701022/diff/11001/sync/api/sync_error.h#new...
sync/api/sync_error.h:51: // Create a new Sync error of type |error_type|
triggered by |model_type|
I'm not sure how useful the auto-generated message is. Outputting error type has
same effect. And this constructor also accepts datatype error. Seems confusing
to me that there are two ways to create datatype error. Can you just add message
parameter to this constructor? In the future, maybe other types of error can
have customized message, too.

On 2013/06/20 18:31:33, Nicolas Zea wrote:
> On 2013/06/10 22:47:56, haitaol1 wrote:
> > Why don't combine these two constructors?
> 
> This constructor autogenerates the message, to I think it's cleaner to not
> provide the message paramater. Otherwise it's ambiguous when the message
should
> or not should be set.

[Nicolas Zea, 2013-07-01 21:46:20]

PTAL. I had  to go ahead and update the callsites for all SyncErrors to specify
the datatype error, as needed.

https://codereview.chromium.org/15701022/diff/11001/sync/api/sync_error.h
File sync/api/sync_error.h (right):

https://codereview.chromium.org/15701022/diff/11001/sync/api/sync_error.h#new...
sync/api/sync_error.h:51: // Create a new Sync error of type |error_type|
triggered by |model_type|
On 2013/06/20 20:35:51, haitaol1 wrote:
> I'm not sure how useful the auto-generated message is. Outputting error type
has
> same effect. And this constructor also accepts datatype error. Seems confusing
> to me that there are two ways to create datatype error. Can you just add
message
> parameter to this constructor? In the future, maybe other types of error can
> have customized message, too.
> 
> On 2013/06/20 18:31:33, Nicolas Zea wrote:
> > On 2013/06/10 22:47:56, haitaol1 wrote:
> > > Why don't combine these two constructors?
> > 
> > This constructor autogenerates the message, to I think it's cleaner to not
> > provide the message paramater. Otherwise it's ambiguous when the message
> should
> > or not should be set.
> 

Done.

[haitaol1, 2013-07-01 21:53:05]

lgtm

[Nicolas Zea, 2013-07-01 23:24:02]

+TBR suggested owners (updated the SyncError callsites). Committing via bot.

[commit-bot: I haz the power, 2013-07-01 23:25:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zea@chromium.org/15701022/26001

[commit-bot: I haz the power, 2013-07-01 23:41:19]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[commit-bot: I haz the power, 2013-07-01 23:51:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zea@chromium.org/15701022/44003

[commit-bot: I haz the power, 2013-07-02 01:42:38]

Retried try job too often on linux_rel for step(s) sync_integration_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-07-02 18:34:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zea@chromium.org/15701022/68001

[commit-bot: I haz the power, 2013-07-02 19:43:52]

Retried try job too often on linux_aura for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-07-02 20:15:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zea@chromium.org/15701022/83002

[commit-bot: I haz the power, 2013-07-02 21:45:41]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-07-02 21:58:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zea@chromium.org/15701022/83002

[commit-bot: I haz the power, 2013-07-02 23:24:17]

Change committed as 209810