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)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month, 2 weeks ago by Sébastien Marchand
Modified:
1 month, 1 week 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
1 month, 1 week 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 ...
1 month, 1 week 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: > ...
1 month, 1 week 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 ...
1 month, 1 week ago (2017-05-16 19:56:10 UTC) #26
Sébastien Marchand
Thanks, +benwells@, PTAL.
1 month, 1 week 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 ...
1 month, 1 week 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, ...
1 month, 1 week 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. ...
1 month, 1 week 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
1 month, 1 week ago (2017-05-18 13:06:18 UTC) #44
commit-bot: I haz the power
1 month, 1 week 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...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 23e94e589