Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1776)

Issue 2610503002: Use TaskScheduler instead of WorkerPool in cache_util.cc. (Closed)

Created:
3 years, 11 months ago by fdoray
Modified:
3 years, 11 months ago
Reviewers:
gavinp
CC:
chromium-reviews, cbentzel+watch_chromium.org, gavinp+disk_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Use TaskScheduler instead of WorkerPool in cache_util.cc. The following traits are used: Priority: BACKGROUND User won't notice if this task takes an arbitrarily long time to complete. Shutdown behavior: CONTINUE_ON_SHUTDOWN Tasks posted with this mode which have not started executing before shutdown is initiated will never run. Tasks with this mode running at shutdown will be ignored (the worker will not be joined). Note: Tasks that were previously posted to base::WorkerPool should use this shutdown behavior because this is how base::WorkerPool handles all its tasks. May Block: Tasks posted with MayBlock() may block. This includes but is not limited to tasks that wait on synchronous file I/O operations: read or write a file from disk, interact with a pipe or a socket, rename or delete a file, enumerate files in a directory, etc. This trait isn't required for the mere use of locks. BUG=659191 Review-Url: https://codereview.chromium.org/2610503002 Cr-Commit-Position: refs/heads/master@{#442985} Committed: https://chromium.googlesource.com/chromium/src/+/8d1a5a1e352ee6b2aee40b42d9fdc4b6e0c97fed

Patch Set 1 #

Patch Set 2 : Fix build error #

Patch Set 3 : add ScopedTaskScheduler #

Unified diffs Side-by-side diffs Delta from patch set Stats (+17 lines, -3 lines) Patch
M net/disk_cache/backend_unittest.cc View 1 2 4 chunks +9 lines, -0 lines 0 comments Download
M net/disk_cache/cache_util.cc View 1 2 chunks +8 lines, -3 lines 0 comments Download

Messages

Total messages: 22 (16 generated)
fdoray
PTAL
3 years, 11 months ago (2017-01-09 21:17:55 UTC) #14
gavinp
lgtm
3 years, 11 months ago (2017-01-11 18:32:39 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2610503002/40001
3 years, 11 months ago (2017-01-11 18:35:12 UTC) #17
commit-bot: I haz the power
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/8d1a5a1e352ee6b2aee40b42d9fdc4b6e0c97fed
3 years, 11 months ago (2017-01-11 19:46:10 UTC) #20
mikecase (-- gone --)
A revert of this CL (patchset #3 id:40001) has been created in https://codereview.chromium.org/2624073004/ by mikecase@chromium.org. ...
3 years, 11 months ago (2017-01-11 22:11:53 UTC) #21
dschuyler
3 years, 11 months ago (2017-01-11 22:12:06 UTC) #22
Message was sent while issue was closed.
A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/2626163002/ by dschuyler@chromium.org.

The reason for reverting is: It looks like this CL may be related to a unit test
failure on this build:
https://build.chromium.org/p/chromium.win/builders/Win10%20Tests%20x64/builds...
- Sheriff.

Powered by Google App Engine
This is Rietveld 408576698