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

Issue 740757 link

Starred by 2 users
Status: Fixed
Owner:
mamir@chromium.org
Closed: Jul 4
Cc:
mastiz@chromium.org
█ tschumann@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature



Sign in to add a comment

Switch ModelTypeWorker to pull model with regards to local changes

Project Member Reported by pav...@chromium.org, Jul 10 2017 
Currently SharedModelTypeProcessor pushes local changes to ModelTypeWorker where they are buffered until they are committed to server. This bug tracks work to change ModelTypeWorker to pull model: ModelTypeWorker requests local changes right before syncing with server.

Here is the doc that describes proposed change: http://docs/document/d/1X4UDqW1xE9ynaMh9stXdCdZtYoUrEb1A-75rNXNWQZ8/edit#heading=h.7lbk8osf0imh
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 12 2017 

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

commit d88c624336115ba5a068b4b83538b5a4cdc42651
Author: Pavel Yatsuk <pavely@chromium.org>
Date: Wed Jul 12 02:24:34 2017

[Sync] Only register for cancellation signal during blocking operation

Cancellation signal signals from UI thread that sync engine is about to be shut
dowm. Implementation of CancelationSignal only allows single observer at a time.

Currently ServerConnectionManager registers for cancellation signal for its
lifetime. It performs blocking HTTP requests to server.

In the future I would also like to issue blocking requests to model types. This
requires ModelTypeWorkter to register with CancelationSignal as well. I want to
move to a model where whoever performs blocking call registers for cancellation
signal for the duration of that call.

In this CL I'm making SyncBridgedConnection register for cancellation signal
only for the duration of HTTP request.

BUG= 740757 
R=pnoland@chromium.org

Change-Id: I18a99de29aa2cf3a27e2f3ba6b3cdb02670c4fc6
Reviewed-on: https://chromium-review.googlesource.com/566029
Reviewed-by: Patrick Noland <pnoland@chromium.org>
Commit-Queue: Pavel Yatsuk <pavely@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485801}
[modify] https://crrev.com/d88c624336115ba5a068b4b83538b5a4cdc42651/components/sync/engine_impl/net/server_connection_manager.cc
[modify] https://crrev.com/d88c624336115ba5a068b4b83538b5a4cdc42651/components/sync/engine_impl/net/server_connection_manager.h
[modify] https://crrev.com/d88c624336115ba5a068b4b83538b5a4cdc42651/components/sync/engine_impl/net/sync_server_connection_manager.cc
[modify] https://crrev.com/d88c624336115ba5a068b4b83538b5a4cdc42651/components/sync/engine_impl/net/sync_server_connection_manager.h
[modify] https://crrev.com/d88c624336115ba5a068b4b83538b5a4cdc42651/components/sync/engine_impl/syncer_proto_util_unittest.cc
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 25 2017 

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

commit b30b47daa93efb9b1ea5a1b34be286dd04ae889f
Author: Pavel Yatsuk <pavely@chromium.org>
Date: Tue Jul 25 02:11:39 2017

[Sync] Introduce ModelTypeProcessor::GetLocalChanges

This change introduces ModelTypeProcessor::GetLocalChanges. Sync engine posts to
model thread to call this function and blocks until model type calls provided
callback passing results back. Blocking wait on sync thread can be cancelled by
sync engine shutdown.

In this change:
- Wired Cancelation signal through registry and worker to blocking request.
- Added NudgeForCommit and GetLocalChanges functions.
- Added a class that holds results of GetLocalChanges call and blocks sync
  thread.

BUG= 740757 
R=pnoland@chromium.org

Change-Id: I327aa503de931edde6dd6f21fa90dacb48597fe9
Reviewed-on: https://chromium-review.googlesource.com/580627
Commit-Queue: Patrick Noland <pnoland@chromium.org>
Commit-Queue: Pavel Yatsuk <pavely@chromium.org>
Reviewed-by: Patrick Noland <pnoland@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489187}
[modify] https://crrev.com/b30b47daa93efb9b1ea5a1b34be286dd04ae889f/components/sync/engine/commit_queue.h
[modify] https://crrev.com/b30b47daa93efb9b1ea5a1b34be286dd04ae889f/components/sync/engine/fake_model_type_processor.cc
[modify] https://crrev.com/b30b47daa93efb9b1ea5a1b34be286dd04ae889f/components/sync/engine/fake_model_type_processor.h
[modify] https://crrev.com/b30b47daa93efb9b1ea5a1b34be286dd04ae889f/components/sync/engine/model_type_processor.h
[modify] https://crrev.com/b30b47daa93efb9b1ea5a1b34be286dd04ae889f/components/sync/engine/model_type_processor_proxy.cc
[modify] https://crrev.com/b30b47daa93efb9b1ea5a1b34be286dd04ae889f/components/sync/engine/model_type_processor_proxy.h
[modify] https://crrev.com/b30b47daa93efb9b1ea5a1b34be286dd04ae889f/components/sync/engine_impl/model_type_registry.cc
[modify] https://crrev.com/b30b47daa93efb9b1ea5a1b34be286dd04ae889f/components/sync/engine_impl/model_type_registry.h
[modify] https://crrev.com/b30b47daa93efb9b1ea5a1b34be286dd04ae889f/components/sync/engine_impl/model_type_registry_unittest.cc
[modify] https://crrev.com/b30b47daa93efb9b1ea5a1b34be286dd04ae889f/components/sync/engine_impl/model_type_worker.cc
[modify] https://crrev.com/b30b47daa93efb9b1ea5a1b34be286dd04ae889f/components/sync/engine_impl/model_type_worker.h
[modify] https://crrev.com/b30b47daa93efb9b1ea5a1b34be286dd04ae889f/components/sync/engine_impl/model_type_worker_unittest.cc
[modify] https://crrev.com/b30b47daa93efb9b1ea5a1b34be286dd04ae889f/components/sync/engine_impl/sync_manager_impl.cc
[modify] https://crrev.com/b30b47daa93efb9b1ea5a1b34be286dd04ae889f/components/sync/engine_impl/sync_scheduler_impl_unittest.cc
[modify] https://crrev.com/b30b47daa93efb9b1ea5a1b34be286dd04ae889f/components/sync/engine_impl/syncer_unittest.cc
[modify] https://crrev.com/b30b47daa93efb9b1ea5a1b34be286dd04ae889f/components/sync/engine_impl/uss_migrator_unittest.cc
[modify] https://crrev.com/b30b47daa93efb9b1ea5a1b34be286dd04ae889f/components/sync/model_impl/shared_model_type_processor.cc
[modify] https://crrev.com/b30b47daa93efb9b1ea5a1b34be286dd04ae889f/components/sync/model_impl/shared_model_type_processor.h
[modify] https://crrev.com/b30b47daa93efb9b1ea5a1b34be286dd04ae889f/components/sync/test/engine/mock_model_type_processor.cc
[modify] https://crrev.com/b30b47daa93efb9b1ea5a1b34be286dd04ae889f/components/sync/test/engine/mock_model_type_processor.h
[modify] https://crrev.com/b30b47daa93efb9b1ea5a1b34be286dd04ae889f/components/sync/test/engine/mock_model_type_worker.cc
[modify] https://crrev.com/b30b47daa93efb9b1ea5a1b34be286dd04ae889f/components/sync/test/engine/mock_model_type_worker.h
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 24 2017 

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

commit d7bc39eb98168882cd7afe17a1086bf146fa7e44
Author: Pavel Yatsuk <pavely@chromium.org>
Date: Tue Oct 24 21:07:17 2017

[Sync] Switch processor/worker to pull model

This change makes worker use results of GetLocalChanges when preparing commit
contribution. EnqueueForCommit is not used and therefore removed.

With this change the expectation is that ModelTypeWorker::entities_ only
accumulate entities in two scenarios:
- Between calls to ProcessGetUpdatesResponse and ApplyUpdates remote updates are
  accumulated
- Between calls to GetContribution and OnCommit response local changes are
  maintained

Encryption is still done at ModelTypeWorker. There are a couple of important
scenarios related to encryption:
- After processor nudges worker about local changes worker should remember about
  that nudge in case cryptographer is not ready. When cryptographer becomes
  ready worker should nudge a NudgeHandler, processor is not going to reissue
  the nudge.
- When worker receives corrupt encrypted update it should skip the update and
  continue with the rest of entities in the batch. It shouldn't block datatype.

Another important scenario concerns commit failures. When commit fails on the
server NonBlockingTypeCommitContribution would not include corresponding
CommitResponseData in the response_list. Wth this change worker is not
retaining/recommitting entities so processor should do that.

This CL includes bunch of random small cleanups I stumbled upon while reviewing
implementation.

BUG= 740757 
R=skym@chromium.org

Change-Id: I3f072d78a8e5782ee9e44da783051964eaa7d62e
Reviewed-on: https://chromium-review.googlesource.com/729520
Commit-Queue: Pavel Yatsuk <pavely@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511258}
[modify] https://crrev.com/d7bc39eb98168882cd7afe17a1086bf146fa7e44/components/sync/engine/commit_queue.h
[modify] https://crrev.com/d7bc39eb98168882cd7afe17a1086bf146fa7e44/components/sync/engine_impl/commit.cc
[modify] https://crrev.com/d7bc39eb98168882cd7afe17a1086bf146fa7e44/components/sync/engine_impl/commit.h
[modify] https://crrev.com/d7bc39eb98168882cd7afe17a1086bf146fa7e44/components/sync/engine_impl/model_type_registry.cc
[modify] https://crrev.com/d7bc39eb98168882cd7afe17a1086bf146fa7e44/components/sync/engine_impl/model_type_worker.cc
[modify] https://crrev.com/d7bc39eb98168882cd7afe17a1086bf146fa7e44/components/sync/engine_impl/model_type_worker.h
[modify] https://crrev.com/d7bc39eb98168882cd7afe17a1086bf146fa7e44/components/sync/engine_impl/model_type_worker_unittest.cc
[modify] https://crrev.com/d7bc39eb98168882cd7afe17a1086bf146fa7e44/components/sync/engine_impl/non_blocking_type_commit_contribution.cc
[modify] https://crrev.com/d7bc39eb98168882cd7afe17a1086bf146fa7e44/components/sync/model_impl/shared_model_type_processor.cc
[modify] https://crrev.com/d7bc39eb98168882cd7afe17a1086bf146fa7e44/components/sync/model_impl/shared_model_type_processor.h
[modify] https://crrev.com/d7bc39eb98168882cd7afe17a1086bf146fa7e44/components/sync/model_impl/shared_model_type_processor_unittest.cc
[modify] https://crrev.com/d7bc39eb98168882cd7afe17a1086bf146fa7e44/components/sync/test/engine/mock_model_type_processor.cc
[modify] https://crrev.com/d7bc39eb98168882cd7afe17a1086bf146fa7e44/components/sync/test/engine/mock_model_type_processor.h
[modify] https://crrev.com/d7bc39eb98168882cd7afe17a1086bf146fa7e44/components/sync/test/engine/mock_model_type_worker.cc
[modify] https://crrev.com/d7bc39eb98168882cd7afe17a1086bf146fa7e44/components/sync/test/engine/mock_model_type_worker.h
[modify] https://crrev.com/d7bc39eb98168882cd7afe17a1086bf146fa7e44/components/sync/test/engine/single_type_mock_server.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 26 2017 

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

commit 17d66c3cdf6776b5eb531e334db50643a43a3543
Author: Pavel Yatsuk <pavely@chromium.org>
Date: Thu Oct 26 05:34:10 2017

Revert "[Sync] Switch processor/worker to pull model"

This reverts commit d7bc39eb98168882cd7afe17a1086bf146fa7e44.

Reason for revert: The change caused multiple crashes on dev channel. https://crbug.com/778177

Original change's description:
> [Sync] Switch processor/worker to pull model
> 
> This change makes worker use results of GetLocalChanges when preparing commit
> contribution. EnqueueForCommit is not used and therefore removed.
> 
> With this change the expectation is that ModelTypeWorker::entities_ only
> accumulate entities in two scenarios:
> - Between calls to ProcessGetUpdatesResponse and ApplyUpdates remote updates are
>   accumulated
> - Between calls to GetContribution and OnCommit response local changes are
>   maintained
> 
> Encryption is still done at ModelTypeWorker. There are a couple of important
> scenarios related to encryption:
> - After processor nudges worker about local changes worker should remember about
>   that nudge in case cryptographer is not ready. When cryptographer becomes
>   ready worker should nudge a NudgeHandler, processor is not going to reissue
>   the nudge.
> - When worker receives corrupt encrypted update it should skip the update and
>   continue with the rest of entities in the batch. It shouldn't block datatype.
> 
> Another important scenario concerns commit failures. When commit fails on the
> server NonBlockingTypeCommitContribution would not include corresponding
> CommitResponseData in the response_list. Wth this change worker is not
> retaining/recommitting entities so processor should do that.
> 
> This CL includes bunch of random small cleanups I stumbled upon while reviewing
> implementation.
> 
> BUG= 740757 
> R=​skym@chromium.org
> 
> Change-Id: I3f072d78a8e5782ee9e44da783051964eaa7d62e
> Reviewed-on: https://chromium-review.googlesource.com/729520
> Commit-Queue: Pavel Yatsuk <pavely@chromium.org>
> Reviewed-by: Sky Malice <skym@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#511258}

TBR=pavely@chromium.org,skym@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  740757 
Change-Id: Id47c7e8d4b6228aff724681fab2f66b5c43e77ba
Reviewed-on: https://chromium-review.googlesource.com/738617
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Commit-Queue: Pavel Yatsuk <pavely@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511747}
[modify] https://crrev.com/17d66c3cdf6776b5eb531e334db50643a43a3543/components/sync/engine/commit_queue.h
[modify] https://crrev.com/17d66c3cdf6776b5eb531e334db50643a43a3543/components/sync/engine_impl/commit.cc
[modify] https://crrev.com/17d66c3cdf6776b5eb531e334db50643a43a3543/components/sync/engine_impl/commit.h
[modify] https://crrev.com/17d66c3cdf6776b5eb531e334db50643a43a3543/components/sync/engine_impl/model_type_registry.cc
[modify] https://crrev.com/17d66c3cdf6776b5eb531e334db50643a43a3543/components/sync/engine_impl/model_type_worker.cc
[modify] https://crrev.com/17d66c3cdf6776b5eb531e334db50643a43a3543/components/sync/engine_impl/model_type_worker.h
[modify] https://crrev.com/17d66c3cdf6776b5eb531e334db50643a43a3543/components/sync/engine_impl/model_type_worker_unittest.cc
[modify] https://crrev.com/17d66c3cdf6776b5eb531e334db50643a43a3543/components/sync/engine_impl/non_blocking_type_commit_contribution.cc
[modify] https://crrev.com/17d66c3cdf6776b5eb531e334db50643a43a3543/components/sync/model_impl/shared_model_type_processor.cc
[modify] https://crrev.com/17d66c3cdf6776b5eb531e334db50643a43a3543/components/sync/model_impl/shared_model_type_processor.h
[modify] https://crrev.com/17d66c3cdf6776b5eb531e334db50643a43a3543/components/sync/model_impl/shared_model_type_processor_unittest.cc
[modify] https://crrev.com/17d66c3cdf6776b5eb531e334db50643a43a3543/components/sync/test/engine/mock_model_type_processor.cc
[modify] https://crrev.com/17d66c3cdf6776b5eb531e334db50643a43a3543/components/sync/test/engine/mock_model_type_processor.h
[modify] https://crrev.com/17d66c3cdf6776b5eb531e334db50643a43a3543/components/sync/test/engine/mock_model_type_worker.cc
[modify] https://crrev.com/17d66c3cdf6776b5eb531e334db50643a43a3543/components/sync/test/engine/mock_model_type_worker.h
[modify] https://crrev.com/17d66c3cdf6776b5eb531e334db50643a43a3543/components/sync/test/engine/single_type_mock_server.cc
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 14 2017 

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

commit 7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf
Author: Pavel Yatsuk <pavely@chromium.org>
Date: Tue Nov 14 17:10:43 2017

Reland: [Sync] Switch processor/worker to pull model

This change makes worker use results of GetLocalChanges when preparing commit
contribution. EnqueueForCommit is not used and therefore removed.

With this change the expectation is that ModelTypeWorker::entities_ only
accumulate entities in two scenarios:
- Between calls to ProcessGetUpdatesResponse and ApplyUpdates remote updates are
  accumulated
- Between calls to GetContribution and OnCommit response local changes are
  maintained

Encryption is still done at ModelTypeWorker. There are a couple of important
scenarios related to encryption:
- After processor nudges worker about local changes worker should remember about
  that nudge in case cryptographer is not ready. When cryptographer becomes
  ready worker should nudge a NudgeHandler, processor is not going to reissue
  the nudge.
- When worker receives corrupt encrypted update it should skip the update and
  continue with the rest of entities in the batch. It shouldn't block datatype.

Another important scenario concerns commit failures. When commit fails on the
server NonBlockingTypeCommitContribution would not include corresponding
CommitResponseData in the response_list. Wth this change worker is not
retaining/recommitting entities so processor should do that.

This CL includes bunch of random small cleanups I stumbled upon while reviewing
implementation.

BUG= 740757 
R=skym@chromium.org

Reviewed-on: https://chromium-review.googlesource.com/729520
Commit-Queue: Pavel Yatsuk <pavely@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#511258}
Change-Id: I753173b6638d802c2057472f7a612e1b8f876d4c
Reviewed-on: https://chromium-review.googlesource.com/753983
Cr-Commit-Position: refs/heads/master@{#516333}
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/engine/commit_queue.h
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/engine_impl/commit.cc
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/engine_impl/commit.h
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/engine_impl/model_type_registry.cc
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/engine_impl/model_type_worker.cc
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/engine_impl/model_type_worker.h
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/engine_impl/model_type_worker_unittest.cc
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/engine_impl/non_blocking_type_commit_contribution.cc
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/engine_impl/syncer_util.cc
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/engine_impl/syncer_util.h
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/engine_impl/worker_entity_tracker.cc
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/model/entity_data.cc
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/model/entity_data.h
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/model_impl/processor_entity_tracker.cc
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/model_impl/processor_entity_tracker.h
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/model_impl/processor_entity_tracker_unittest.cc
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/model_impl/shared_model_type_processor.cc
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/model_impl/shared_model_type_processor.h
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/model_impl/shared_model_type_processor_unittest.cc
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/test/engine/mock_model_type_processor.cc
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/test/engine/mock_model_type_processor.h
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/test/engine/mock_model_type_worker.cc
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/test/engine/mock_model_type_worker.h
[modify] https://crrev.com/7e0a44b102cbc1c2da37196f3f83b19dd6c67bcf/components/sync/test/engine/single_type_mock_server.cc
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 17 2017 

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

commit f904ab78d11fb2549bd4d1e528335a0354c2959c
Author: Pavel Yatsuk <pavely@chromium.org>
Date: Fri Nov 17 03:49:35 2017

[Sync] Don't use WorkerEntityTracker for commit path in worker.

This is a step towards worker not relying on client tag hash to identify entity.
It is needed to implement bookmarks migration as bookmarks code doesn't populate
client tag hash.

In this CL I stop caching CommitRequestData in WorkerEntityTracker, instead I
pass CommitRequestDataList to CommitContributor. When commit response arrives,
records are matched with corresponding request objects in CommitContributor
based on position. All the logic related to populating and adjusting SyncEntity
is moved there too.

BUG= 740757 
R=skym@chromium.org

Change-Id: I46012c48e506bfdd23f393536dcf2f911d0efad4
Reviewed-on: https://chromium-review.googlesource.com/775773
Commit-Queue: Pavel Yatsuk <pavely@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517287}
[modify] https://crrev.com/f904ab78d11fb2549bd4d1e528335a0354c2959c/components/sync/engine_impl/model_type_worker.cc
[modify] https://crrev.com/f904ab78d11fb2549bd4d1e528335a0354c2959c/components/sync/engine_impl/model_type_worker.h
[modify] https://crrev.com/f904ab78d11fb2549bd4d1e528335a0354c2959c/components/sync/engine_impl/non_blocking_type_commit_contribution.cc
[modify] https://crrev.com/f904ab78d11fb2549bd4d1e528335a0354c2959c/components/sync/engine_impl/non_blocking_type_commit_contribution.h
[modify] https://crrev.com/f904ab78d11fb2549bd4d1e528335a0354c2959c/components/sync/engine_impl/worker_entity_tracker.cc
[modify] https://crrev.com/f904ab78d11fb2549bd4d1e528335a0354c2959c/components/sync/engine_impl/worker_entity_tracker.h
[modify] https://crrev.com/f904ab78d11fb2549bd4d1e528335a0354c2959c/components/sync/engine_impl/worker_entity_tracker_unittest.cc
[modify] https://crrev.com/f904ab78d11fb2549bd4d1e528335a0354c2959c/components/sync/model_impl/shared_model_type_processor.cc

Comment 7 by pav...@chromium.org, Jan 17 2018

Labels: SyncStarter2018
Owner: mamir@chromium.org
Mohamed, your current change is part of this work.

Comment 8 by pav...@chromium.org, Jan 17 2018

Labels: SyncHandoff2018

Comment 9 by zea@chromium.org, Jan 17 2018

Labels: -SyncStarter2018 Hotlist-GoodFirstBug
Project Member

Comment 10 by bugdroid1@chromium.org, Apr 11 2018 

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

commit fdcd3fc28f874510aa16baa4e8a31773a305512c
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Wed Apr 11 07:58:15 2018

[Sync USS] Load entities in CTBMTP when the Sync engine is ready

Before this CL:
Data are loaded in ClientTagBasedModelTypeProcessor on startup.
Data are kept in memory until they are committed which can
never happen (if the client is offline for example).

After this CL:
Only metadata are loaded at startup, and only after the Sync engine
asks for local changes, data are loaded in memory. As a result,
GetLocalChanges method in the processor is called asynchronously
because the required data are not necessarily available in memory.

Bug:  740757 
Change-Id: I8a11ddc0c8136775f0075697ba6b8ee3975685fa
Reviewed-on: https://chromium-review.googlesource.com/825562
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549815}
[modify] https://crrev.com/fdcd3fc28f874510aa16baa4e8a31773a305512c/components/sync/engine/model_type_processor.h
[modify] https://crrev.com/fdcd3fc28f874510aa16baa4e8a31773a305512c/components/sync/engine_impl/model_type_worker.cc
[modify] https://crrev.com/fdcd3fc28f874510aa16baa4e8a31773a305512c/components/sync/model_impl/client_tag_based_model_type_processor.cc
[modify] https://crrev.com/fdcd3fc28f874510aa16baa4e8a31773a305512c/components/sync/model_impl/client_tag_based_model_type_processor.h
[modify] https://crrev.com/fdcd3fc28f874510aa16baa4e8a31773a305512c/components/sync/model_impl/client_tag_based_model_type_processor_unittest.cc
[modify] https://crrev.com/fdcd3fc28f874510aa16baa4e8a31773a305512c/components/sync/model_impl/processor_entity_tracker.cc
[modify] https://crrev.com/fdcd3fc28f874510aa16baa4e8a31773a305512c/components/sync/test/engine/mock_model_type_worker.cc
[modify] https://crrev.com/fdcd3fc28f874510aa16baa4e8a31773a305512c/components/sync/test/engine/mock_model_type_worker.h

Comment 11 by mastiz@chromium.org, Jul 4 

Can we mark this as fixed?

Comment 12 by mamir@chromium.org, Jul 4

Status: Fixed (was: Assigned)
As discussed offline, yes.
►

Sign in to add a comment