New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 714828 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Feature

Blocking:
issue 701032
issue 716199



Sign in to add a comment

[Sync] Remove required from SyncEntity proto

Project Member Reported by s...@chromium.org, Apr 24 2017

Issue description

We currently have two required fields in SyncEntity: name and version. When emitting user events, we want to be as compact as possible, and being forced to set empty fields conflicts with this.

It should be possible to update the client and server definitions to simply treat these fields as optional going forward. Old clients will still think they're required, and it's important that they always receive values for these. On non-commit only model types we should still be setting these for the foreseeable future.

We could also add a safety check to the server side to make sure old clients always see these fields as well, to prevent a regression.

 

Comment 1 by s...@chromium.org, Apr 27 2017

Blocking: 716199
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f9bf5ca85577c34702afe733dcb6d1d98ed728a0

commit f9bf5ca85577c34702afe733dcb6d1d98ed728a0
Author: skym <skym@chromium.org>
Date: Fri Apr 28 20:06:39 2017

[Sync] Remove required fields from SyncEntity and obsolete ThrottleParameters.

This in preparation for USER_EVENTS, which is a commit only type and
some fields like version and not useful. We want to save as much
bandwidth as possible, and switching to optional saves us ~2 bytes each
before compression.

BUG= 714828 

Review-Url: https://codereview.chromium.org/2837993002
Cr-Commit-Position: refs/heads/master@{#468104}

[modify] https://crrev.com/f9bf5ca85577c34702afe733dcb6d1d98ed728a0/components/sync/protocol/sync.proto

Comment 3 by s...@chromium.org, Apr 28 2017

Status: Fixed (was: Started)

Sign in to add a comment