Migrate Sync to Task Scheduler API |
|||||
Issue descriptionThe goal of this is to remove dependence of Sync code on BrowserThreads and replace with per-model instances of SequencedTaskRunner. Perhaps we could do the migration in two steps. 1) Introduce one sequenced task runner singleton per current Sync group and have both native models and sync code use that. That will be used as a direct replacement for browser threads used in ChromeSyncClient::CreateModelWorkerForGroup. This should allow minimal changes in the sync code. 2) Refactor Sync to get rid of the group concept and move to 1:1 mapping of model specific sequenced task runner to model type in Sync code. History model already works this way so that is an example to follow. Assigning this to myself for now
,
Jun 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8caf9b2d9cc526400419885f23c373430a34a8f0 commit 8caf9b2d9cc526400419885f23c373430a34a8f0 Author: stanisc <stanisc@chromium.org> Date: Mon Jun 26 23:24:08 2017 Remove dependency on BrowserThread::FILE from Sync code This change moves definition of a task runner for a sync group to a native model code outside of Sync, in this case - to extensions code. This might be a bit controversial because syncer::GROUP_FILE could theoretically include sync types other than syncer::EXTENSION_SETTINGS and syncer::APP_SETTINGS, but in reality those are the only two possible sync types that currently sync on the FILE thread. And since we are migrating to the new TaskScheduler API and sequences, if we need to introduce a new sync type in the future it should run on a new, separate sequence and avoid sharing the FILE group. I added a new function that is supposed to return a singleton instance of SequencedTaskRunner - extensions::GetBackendTaskRunner(). For now the implementation of this function still uses BrowserThread::FILE but that should be changed to use LazySequencedTaskRunner as soon as the rest of extensions storage code is modified to share the same task runner. I noticed that extensions are unavailable of iOS and Android so I removed the syncer::GROUP_FILE option from iOS and Android specific code. To summarize, the next steps are: 1) Change extensions code to use extensions::GetBackendTaskRunner() everywhere the FILE thread is currently used. 2) Modify GetBackendTaskRunner() to use LazySequencedTaskRunner instead of the FILE thread task runner. BUG= 689520 , 731903 Review-Url: https://codereview.chromium.org/2944993002 Cr-Commit-Position: refs/heads/master@{#482455} [modify] https://crrev.com/8caf9b2d9cc526400419885f23c373430a34a8f0/chrome/browser/extensions/BUILD.gn [add] https://crrev.com/8caf9b2d9cc526400419885f23c373430a34a8f0/chrome/browser/extensions/api/storage/backend_task_runner.cc [add] https://crrev.com/8caf9b2d9cc526400419885f23c373430a34a8f0/chrome/browser/extensions/api/storage/backend_task_runner.h [modify] https://crrev.com/8caf9b2d9cc526400419885f23c373430a34a8f0/chrome/browser/sync/chrome_sync_client.cc [modify] https://crrev.com/8caf9b2d9cc526400419885f23c373430a34a8f0/chrome/browser/sync/glue/extension_setting_data_type_controller.cc [modify] https://crrev.com/8caf9b2d9cc526400419885f23c373430a34a8f0/chrome/browser/sync/test/integration/extension_settings_helper.cc [modify] https://crrev.com/8caf9b2d9cc526400419885f23c373430a34a8f0/ios/chrome/browser/sync/ios_chrome_sync_client.mm
,
Jun 28 2017
,
Jul 13 2017
pkasting@, assigning this to you since you took chrome_sync_client.cc and a few other files related to migration of Autofill. The approach that I used for migration of the sync FILE group is to make a singleton SequencedTaskRunner on Extensions side and use it from both Extensions and Sync code. I think you could use the same approach for migration of Autofill / Sync DB group.
,
Jul 13 2017
Sorry, I can't migrate all of sync. I'll be touching a few files in a few places, but I cannot own the larger bug here.
,
Sep 7 2017
Looks like this is now done. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by stanisc@chromium.org
, Jun 26 2017