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

Issue 719041 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 701032



Sign in to add a comment

[Sync] Modify sync machinery to correctly commit USER_EVENTS/CommitOnly types

Project Member Reported by s...@chromium.org, May 5 2017

Issue description

Right now things have progress markers, too many fields, etc. Types are always included in both commit and getupdates. We need to change some assumptions to make user events work.
 

Comment 1 by s...@chromium.org, May 5 2017

Blocking: 701032
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 6 2017

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

commit c15b61bdf8133f14d7f715ee6e0dcc5ab65db57c
Author: skym <skym@chromium.org>
Date: Tue Jun 06 18:38:53 2017

[Sync] Support commit only types.

The main differences of commit only types is that they do not have a
progress marker, they use very few metadata fields of SyncEntity, they
do not perform GetUpdates, and they delete themselves after commit
confirmation. Also only supposed on USS/NonBlocking path.

This CL updates internal sync logic to facilitate commit only types.
With these changes user events should be entirely functional on the
client. Can specify the flag --enable-features=SyncUserEvents and
existing translate integration will start emitting.

The difference between using the static CommitOnlyTypes.Has(...), vs a
private boolean in worker vs processor is unfortunate. I think the
CommitOnlyTypes.Has(...) approach is better whenever something has the
model type, however, current unit tests are tightly coupled with
PREFERENCES, which is why I created the |commit_only_| field.

BUG= 719041 

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

[modify] https://crrev.com/c15b61bdf8133f14d7f715ee6e0dcc5ab65db57c/components/sync/base/model_type.h
[modify] https://crrev.com/c15b61bdf8133f14d7f715ee6e0dcc5ab65db57c/components/sync/driver/data_type_manager_impl.cc
[modify] https://crrev.com/c15b61bdf8133f14d7f715ee6e0dcc5ab65db57c/components/sync/engine_impl/cycle/nudge_tracker.cc
[modify] https://crrev.com/c15b61bdf8133f14d7f715ee6e0dcc5ab65db57c/components/sync/engine_impl/model_type_worker.cc
[modify] https://crrev.com/c15b61bdf8133f14d7f715ee6e0dcc5ab65db57c/components/sync/engine_impl/model_type_worker_unittest.cc
[modify] https://crrev.com/c15b61bdf8133f14d7f715ee6e0dcc5ab65db57c/components/sync/engine_impl/non_blocking_type_commit_contribution.cc
[modify] https://crrev.com/c15b61bdf8133f14d7f715ee6e0dcc5ab65db57c/components/sync/engine_impl/non_blocking_type_commit_contribution.h
[modify] https://crrev.com/c15b61bdf8133f14d7f715ee6e0dcc5ab65db57c/components/sync/engine_impl/syncer.cc
[modify] https://crrev.com/c15b61bdf8133f14d7f715ee6e0dcc5ab65db57c/components/sync/engine_impl/syncer_unittest.cc
[modify] https://crrev.com/c15b61bdf8133f14d7f715ee6e0dcc5ab65db57c/components/sync/model/model_type_change_processor.cc
[modify] https://crrev.com/c15b61bdf8133f14d7f715ee6e0dcc5ab65db57c/components/sync/model_impl/shared_model_type_processor.cc
[modify] https://crrev.com/c15b61bdf8133f14d7f715ee6e0dcc5ab65db57c/components/sync/model_impl/shared_model_type_processor.h
[modify] https://crrev.com/c15b61bdf8133f14d7f715ee6e0dcc5ab65db57c/components/sync/model_impl/shared_model_type_processor_unittest.cc
[modify] https://crrev.com/c15b61bdf8133f14d7f715ee6e0dcc5ab65db57c/components/sync/syncable/model_type.cc
[modify] https://crrev.com/c15b61bdf8133f14d7f715ee6e0dcc5ab65db57c/components/sync/test/engine/mock_connection_manager.cc
[modify] https://crrev.com/c15b61bdf8133f14d7f715ee6e0dcc5ab65db57c/components/sync/user_events/user_event_sync_bridge.cc
[modify] https://crrev.com/c15b61bdf8133f14d7f715ee6e0dcc5ab65db57c/components/sync/user_events/user_event_sync_bridge_unittest.cc

Comment 3 by s...@chromium.org, Jun 8 2017

Status: Fixed (was: Assigned)

Sign in to add a comment