Convert DatabaseTracker and DatabaseMessageFilter to use TaskScheduler |
|||||
Issue descriptionCurrently, the DatabaseTracker (browser-side support for WebSQL) uses the FILE thread. It should use TaskScheduler. * Have DatabaseTracker mint a sequenced task runner. * Convert DBMF over to use it * ... * Profit! --------------- Current status (2017-09-05) * DatabaseTracker and DatabaseMessageFilter are moved to a dedicated sequence minted in the DatabaseTracker constructor - that part is done. * Unit tests inject a task runner which overrides this, via DatabaseTracker:: set_task_runner_for_testing() - this should change to the tests using the normal runner and doing work on the appropriate sequence instead.
,
Jul 12 2017
The unittest problem sounds familiar, this could be a matter of a missing base::test::ScopedTaskEnvironment? See the following CL/bug for example. https://chromium-review.googlesource.com/c/567396/5
,
Jul 12 2017
It's got a TestBrowserThreadBundle which (per gab@) gives you a ScopedTaskEnvironment "for free"
,
Jul 12 2017
(looking at the bug/CL...) Interesting. Again, per gab@ I wouldn't expect you'd need both. I'll keep an eye on that, maybe give it a try.
,
Jul 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/607cb1475d625041f23200d3002c448e06239570 commit 607cb1475d625041f23200d3002c448e06239570 Author: Joshua Bell <jsbell@chromium.org> Date: Mon Jul 24 19:17:16 2017 Move DatabaseTracker and DatabaseMessageFilter off of FILE thread As part of the Great TaskScheduler Migration, move 'Database' (i.e. WebSQL) operations in the browser off of the FILE thread to a dedicated task runner. Unit tests still override the task runner; that will be tackled in a follow-up. Bug: 739945 Change-Id: I7308c18fa6ff60686a3cffa62d35eb9719935477 Reviewed-on: https://chromium-review.googlesource.com/563649 Commit-Queue: Joshua Bell <jsbell@chromium.org> Reviewed-by: Michael Nordman <michaeln@chromium.org> Reviewed-by: Victor Costan <pwnall@chromium.org> Reviewed-by: John Abd-El-Malek <jam@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Cr-Commit-Position: refs/heads/master@{#489043} [modify] https://crrev.com/607cb1475d625041f23200d3002c448e06239570/chrome/browser/browsing_data/browsing_data_database_helper.cc [modify] https://crrev.com/607cb1475d625041f23200d3002c448e06239570/chrome/browser/browsing_data/browsing_data_database_helper.h [modify] https://crrev.com/607cb1475d625041f23200d3002c448e06239570/chrome/browser/browsing_data/mock_browsing_data_database_helper.cc [modify] https://crrev.com/607cb1475d625041f23200d3002c448e06239570/chrome/browser/browsing_data/mock_browsing_data_database_helper.h [modify] https://crrev.com/607cb1475d625041f23200d3002c448e06239570/chrome/browser/extensions/extension_service_unittest.cc [modify] https://crrev.com/607cb1475d625041f23200d3002c448e06239570/content/browser/browser_context.cc [modify] https://crrev.com/607cb1475d625041f23200d3002c448e06239570/content/browser/renderer_host/database_message_filter.cc [modify] https://crrev.com/607cb1475d625041f23200d3002c448e06239570/content/browser/renderer_host/database_message_filter.h [modify] https://crrev.com/607cb1475d625041f23200d3002c448e06239570/content/browser/storage_partition_impl.cc [modify] https://crrev.com/607cb1475d625041f23200d3002c448e06239570/content/public/test/browsing_data_remover_test_util.cc [modify] https://crrev.com/607cb1475d625041f23200d3002c448e06239570/content/shell/browser/layout_test/layout_test_message_filter.cc [modify] https://crrev.com/607cb1475d625041f23200d3002c448e06239570/content/shell/browser/layout_test/layout_test_message_filter.h [modify] https://crrev.com/607cb1475d625041f23200d3002c448e06239570/sql/connection.cc [modify] https://crrev.com/607cb1475d625041f23200d3002c448e06239570/storage/browser/database/database_quota_client.cc [modify] https://crrev.com/607cb1475d625041f23200d3002c448e06239570/storage/browser/database/database_quota_client.h [modify] https://crrev.com/607cb1475d625041f23200d3002c448e06239570/storage/browser/database/database_quota_client_unittest.cc [modify] https://crrev.com/607cb1475d625041f23200d3002c448e06239570/storage/browser/database/database_tracker.cc [modify] https://crrev.com/607cb1475d625041f23200d3002c448e06239570/storage/browser/database/database_tracker.h [modify] https://crrev.com/607cb1475d625041f23200d3002c448e06239570/storage/browser/database/database_tracker_unittest.cc
,
Jul 25 2017
Not closing yet since tests still inject a task runner.
,
Jul 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bdfd33d478b95ae1bb831ad089c3698b0645beaa commit bdfd33d478b95ae1bb831ad089c3698b0645beaa Author: Joshua Bell <jsbell@chromium.org> Date: Tue Jul 25 21:49:44 2017 WebSQL: Move database deletion retries to correct sequence Follow up to https://chromium-review.googlesource.com/c/563649/ DatabaseMessageFilter and DatabaseTracker were moved off the FILE thread to a dedicated sequence, but a case involving retries on I/O error was missed. Fix it. Bug: 739945 Change-Id: I2703680e1797142766f9fa8cc1a5cc8b19cb24c1 Reviewed-on: https://chromium-review.googlesource.com/585369 Reviewed-by: Gabriel Charette <gab@chromium.org> Commit-Queue: Joshua Bell <jsbell@chromium.org> Cr-Commit-Position: refs/heads/master@{#489450} [modify] https://crrev.com/bdfd33d478b95ae1bb831ad089c3698b0645beaa/content/browser/renderer_host/database_message_filter.cc
,
Sep 5 2017
Moving back to available for the test updates, since I'm not working on it at the moment.
,
Sep 5 2017
#8: While this is still on your mind, can you please update the bug description to reflect what was done / what needs to be done?
,
Sep 5 2017
,
Sep 6
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 6
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by jsb...@chromium.org
, Jul 12 2017