[Sync] Handle invalid specifics field numbers gracefully

[Sync] Handle invalid specifics field numbers gracefully

Change GetModelTypeFromSpecificsFieldNumber() to not NOTREACHED() on an
unknown field number.  Instead, have callers compare the return value
to UNSPECIFIED and handle that case.

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

[akalin, 2012-12-10 23:04:21]

+zea for review

[Nicolas Zea, 2012-12-10 23:13:31]

LGTM. Ideally I'd like not following the pattern to be a compile failure, but I
can't think of any good way to do that :-/

[akalin, 2012-12-10 23:47:41]

On 2012/12/10 23:13:31, Nicolas Zea wrote:
> LGTM. Ideally I'd like not following the pattern to be a compile failure, but
I
> can't think of any good way to do that :-/

Yeah, me neither. :/ But fortunately, there's only a small number of callers.

[commit-bot: I haz the power, 2012-12-10 23:49:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/akalin@chromium.org/11490018/1

[tim (not reviewing), 2012-12-10 23:49:37]

https://codereview.chromium.org/11490018/diff/1/sync/engine/syncer_proto_util.cc
File sync/engine/syncer_proto_util.cc (right):

https://codereview.chromium.org/11490018/diff/1/sync/engine/syncer_proto_util...
sync/engine/syncer_proto_util.cc:334: NOTREACHED() << "Unknown field number " <<
field_number;
Is there any way to add a test for this?

[akalin, 2012-12-11 00:15:50]

https://codereview.chromium.org/11490018/diff/1/sync/engine/syncer_proto_util.cc
File sync/engine/syncer_proto_util.cc (right):

https://codereview.chromium.org/11490018/diff/1/sync/engine/syncer_proto_util...
sync/engine/syncer_proto_util.cc:334: NOTREACHED() << "Unknown field number " <<
field_number;
On 2012/12/10 23:49:37, timsteele wrote:
> Is there any way to add a test for this?

Yeah.  Are you okay with doing it in another CL?  I'd like to keep this one
small in case we merge it.

[tim (not reviewing), 2012-12-11 01:19:46]

OK.

On Mon, Dec 10, 2012 at 4:15 PM, <akalin@chromium.org> wrote:

>
> https://codereview.chromium.**org/11490018/diff/1/sync/**
>
engine/syncer_proto_util.cc<https://codereview.chromium.org/11490018/diff/1/sync/engine/syncer_proto_util.cc>
> File sync/engine/syncer_proto_util.**cc (right):
>
> https://codereview.chromium.**org/11490018/diff/1/sync/**
>
engine/syncer_proto_util.cc#**newcode334<https://codereview.chromium.org/11490018/diff/1/sync/engine/syncer_proto_util.cc#newcode334>
> sync/engine/syncer_proto_util.**cc:334: NOTREACHED() << "Unknown field
> number " << field_number;
> On 2012/12/10 23:49:37, timsteele wrote:
>
>> Is there any way to add a test for this?
>>
>
> Yeah.  Are you okay with doing it in another CL?  I'd like to keep this
> one small in case we merge it.
>
>
https://codereview.chromium.**org/11490018/<https://codereview.chromium.org/1...
>

[commit-bot: I haz the power, 2012-12-11 01:55:02]

List of reviewers changed. tim@chromium.org did a drive-by without LGTM'ing!

[tim (not reviewing), 2012-12-11 02:02:40]

On 2012/12/11 01:55:02, I haz the power (commit-bot) wrote:
> List of reviewers changed. mailto:tim@chromium.org did a drive-by without
LGTM'ing!

LGTM

[commit-bot: I haz the power, 2012-12-11 02:03:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/akalin@chromium.org/11490018/1

[commit-bot: I haz the power, 2012-12-11 02:05:08]

Change committed as 172232