Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1)

Issue 2877343003: Switch browser/extensions/api/file_system/file_system_api.[h|cc] to the TaskScheduler API (Closed)

Created:
7 months ago by Sébastien Marchand
Modified:
7 months ago
Reviewers:
mtomasz, fdoray, benwells
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, tzik, kinuko+fileapi, nhiroki, extensions-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Switch browser/extensions/api/file_system/file_system_api.[h|cc] to the TaskScheduler API With this CL, tasks posted from this file can run in parallel. Previously, they were all posted to the file thread so that was not possible. Please verify that this is ok. Also, please verify that appropriate traits are used to post task https://cs.chromium.org/chromium/src/base/task_scheduler/task_traits.h BUG=689520 Review-Url: https://codereview.chromium.org/2877343003 Cr-Commit-Position: refs/heads/master@{#472785} Committed: https://chromium.googlesource.com/chromium/src/+/8bf05bb205b2693eae180c0b69c7aa5385a3592f

Patch Set 1 : Switch FileSystemRetainEntryFunction to the TaskScheduler API. #

Patch Set 2 : Fixes. #

Total comments: 6

Patch Set 3 : Address Francois' comments. #

Total comments: 2

Patch Set 4 : Address Ben's comments. #

Messages

Total messages: 47 (37 generated)
Sébastien Marchand
PTAL
7 months ago (2017-05-16 14:08:19 UTC) #14
fdoray
https://codereview.chromium.org/2877343003/diff/100001/chrome/browser/extensions/api/file_system/file_system_api.cc File chrome/browser/extensions/api/file_system/file_system_api.cc (right): https://codereview.chromium.org/2877343003/diff/100001/chrome/browser/extensions/api/file_system/file_system_api.cc#newcode596 chrome/browser/extensions/api/file_system/file_system_api.cc:596: task_runner_->PostTaskAndReply( Unless RunAsync() is invoked multiple times on the ...
7 months ago (2017-05-16 15:23:26 UTC) #15
Sébastien Marchand
Thanks, PTAnL. https://codereview.chromium.org/2877343003/diff/100001/chrome/browser/extensions/api/file_system/file_system_api.cc File chrome/browser/extensions/api/file_system/file_system_api.cc (right): https://codereview.chromium.org/2877343003/diff/100001/chrome/browser/extensions/api/file_system/file_system_api.cc#newcode596 chrome/browser/extensions/api/file_system/file_system_api.cc:596: task_runner_->PostTaskAndReply( On 2017/05/16 15:23:26, fdoray wrote: > ...
7 months ago (2017-05-16 15:53:09 UTC) #20
fdoray
lgtm Possible message for owner: With this CL, tasks posted from this file can run ...
7 months ago (2017-05-16 19:56:10 UTC) #26
Sébastien Marchand
Thanks, +benwells@, PTAL.
7 months ago (2017-05-16 20:02:17 UTC) #29
benwells
I think this is OK but would like to get a second opinion. +mtomasz: I ...
7 months ago (2017-05-17 01:03:52 UTC) #31
Sébastien Marchand
Thanks, waiting for mtomasz@'s review. https://codereview.chromium.org/2877343003/diff/120001/chrome/browser/extensions/api/file_system/file_system_api.cc File chrome/browser/extensions/api/file_system/file_system_api.cc (right): https://codereview.chromium.org/2877343003/diff/120001/chrome/browser/extensions/api/file_system/file_system_api.cc#newcode883 chrome/browser/extensions/api/file_system/file_system_api.cc:883: &FileSystemChooseEntryFunction::ConfirmDirectoryAccessOnFileThread, On 2017/05/17 01:03:52, ...
7 months ago (2017-05-17 18:43:01 UTC) #36
mtomasz
Please fix the CL title as this CL fixes different function that the title says. ...
7 months ago (2017-05-18 03:35:32 UTC) #40
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/2877343003/160001
7 months ago (2017-05-18 13:06:18 UTC) #44
commit-bot: I haz the power
7 months ago (2017-05-18 13:12:06 UTC) #47
Message was sent while issue was closed.
Committed patchset #4 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/8bf05bb205b2693eae180c0b69c7...

Powered by Google App Engine
This is Rietveld 0eb02b776