Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(580)

Issue 11490018: [Sync] Handle invalid specifics field numbers gracefully (Closed)

Created:
8 years ago by akalin
Modified:
8 years ago
CC:
chromium-reviews, Raghu Simha, haitaol1
Visibility:
Public.

Description

[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

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+55 lines, -14 lines) Patch
M sync/engine/store_timestamps_command.cc View 1 chunk +6 lines, -7 lines 0 comments Download
M sync/engine/syncer_proto_util.cc View 2 chunks +15 lines, -5 lines 2 comments Download
M sync/internal_api/public/base/model_type.h View 1 chunk +19 lines, -1 line 0 comments Download
M sync/syncable/model_type.cc View 1 chunk +0 lines, -1 line 0 comments Download
M sync/syncable/model_type_unittest.cc View 1 chunk +15 lines, -0 lines 0 comments Download

Messages

Total messages: 11 (0 generated)
akalin
+zea for review
8 years ago (2012-12-10 23:04:21 UTC) #1
Nicolas Zea
LGTM. Ideally I'd like not following the pattern to be a compile failure, but I ...
8 years ago (2012-12-10 23:13:31 UTC) #2
akalin
On 2012/12/10 23:13:31, Nicolas Zea wrote: > LGTM. Ideally I'd like not following the pattern ...
8 years ago (2012-12-10 23:47:41 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/akalin@chromium.org/11490018/1
8 years ago (2012-12-10 23:49:21 UTC) #4
tim (not reviewing)
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 sync/engine/syncer_proto_util.cc:334: NOTREACHED() << "Unknown field number " << field_number; Is ...
8 years ago (2012-12-10 23:49:37 UTC) #5
akalin
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 sync/engine/syncer_proto_util.cc:334: NOTREACHED() << "Unknown field number " << field_number; On ...
8 years ago (2012-12-11 00:15:50 UTC) #6
tim (not reviewing)
OK. On Mon, Dec 10, 2012 at 4:15 PM, <akalin@chromium.org> wrote: > > https://codereview.chromium.**org/11490018/diff/1/sync/** > ...
8 years ago (2012-12-11 01:19:46 UTC) #7
commit-bot: I haz the power
List of reviewers changed. tim@chromium.org did a drive-by without LGTM'ing!
8 years ago (2012-12-11 01:55:02 UTC) #8
tim (not reviewing)
On 2012/12/11 01:55:02, I haz the power (commit-bot) wrote: > List of reviewers changed. mailto:tim@chromium.org ...
8 years ago (2012-12-11 02:02:40 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/akalin@chromium.org/11490018/1
8 years ago (2012-12-11 02:03:20 UTC) #10
commit-bot: I haz the power
8 years ago (2012-12-11 02:05:08 UTC) #11
Message was sent while issue was closed.
Change committed as 172232

Powered by Google App Engine
This is Rietveld 408576698