See https://chromium-review.googlesource.com/c/565641/ for some more background and discussion for one use case for these threads in blink.

Not sure what the best approach would be to implement either of these though. For FILE thread maybe something like:

- Add CreateFileThread() method to blink::RendererScheduler, which returns a WebThread.
- RenderThreadImpl uses this (and its task runner) to implement its GetFileThreadTaskRunner method, replacing the custom thread creation in RenderThreadImpl.
- CreateFileThread under the hood call WebThreadBase::CreateWorkerThread (or maybe that's the wrong method?) to actually create the appropriate thread.
- Somehow actually make the newly created thread available to the rest of blink. Possibly add a new WebThread* FileThread() getter to Platform, similar to the existing CompositorThread getter? Which is then implemented to return the thread stored in RenderThreadImpl.

Should we instead expose base/task_runner/ to Blink since we're moving away from dedicated threads elsewhere in Chrome? Or if we don't want Blink to see the full interfaces yet, RenderThreadImpl could have a GetFileThreadTaskRunner which justs posts tasks with the correct task traits instead of using a dedicated thread.

Yes, if you're going to plumb to //base, use base/task_scheduler/post_task.h (even if you only create a single task runner under the hood -- which would be the equivalent of a base::Thread as far as running tasks is concerned but would benefit from all the task scheduling hooks).

Have Platform::CreateFileTaskRunner() which returns WebTaskRunner was what I was thinking about, as I imagined it might be easier for Blink Scheduler to come into play if necessary, but otherwise just let consumers use base/task_runner classes would be ok too.

I have one question though, I expect we could replace other similar use case, e.g. Web DB thread, with the new one, but if so we'll want to have GC support there-- then we still want to have something like WebThreadSupportingGC, and probably would still need map those task runners to a single task runner.

On the other hand for the initial usage that's being discussed in the code review (i.e. blob one) just using SequencedTaskRunner should be totally fine.

Even if you need a SingleThreadTaskRunner :
base::CreateSingleThreadTaskRunnerWithTraits() is much preferred to
base::Thread. It will make sure those tasks go through the main task funnel
in chrome and are thus gaining a bunch of instrumentation for free (all new
frameworks should be built on top of the TaskScheduler's TaskTracker).

Le lun. 24 juil. 2017 12:00, kinuko via monorail <
monorail+v2.913730398@chromium.org> a écrit :

I am not advocating the use of base::Thread at all, using base::CreateSingleThreadTaskRunnerWithTraits looks good and it doesn't sound it's incompatible with what I wrote.

My point was that uniformly using WebTaskRunner seemed to help us unify Blink's scheduling logic (which would be eventually working on Lucky Luke), but if scheduler folks think it's fine to directly expose base/task_runner (without going through Blink scheduler layer) to Blink it'd be probably just fine.  If that's the case does that mean Blink scheduler would be the one that schedule main-thread and worker-thread things, but not other tasks in Blink?

Yeah, I'm a bit confused.

I was thinking that Blink Scheduler wants to schedule all tasks in Blink so there's no option to expose //base/task_runner directly.

Or (as kinuko-san mentioned) do you mean that Blink Scheduler doesn't need to schedule tasks on IO and FILE threads?

It doesn't seem like blink scheduler scheduling all tasks in blink/the renderer, and using //base/task_scheduler directly need to be mutually exclusive? I.e. the blink scheduler could implmement base::TaskScheduler and call TaskScheduler::SetInstance to be responsible for scheduling all tasks in the renderer process (or whatever other integration between blink task scheduler and base/task_scheduler makes sense).

Although maybe potentially needing extra TaskTraits (like the "needs garbage collector" mentioned before) might indeed make it infeasible to use base/task_scheduler directly from blink code.

Currently Blink has a strong thread affinity in many places (including GC). I agree that we should make the task - thread mapping more dynamic but it is going to be a big project.

So at the moment I'd suggest moving this discussion forward with an assumption that WebTaskRunner is mapped to some specific WebThread.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8c2764154c196bf55a141141914255ff866b1c12

commit 8c2764154c196bf55a141141914255ff866b1c12
Author: Marijn Kruisselbrink <mek@chromium.org>
Date: Sat Jul 29 06:11:21 2017

Move BlobBytesProvider to a File task runner.

Move it off the main/worker thread to prevent deadlock when the main or
worker thread tries to synchronously read the blob (using XHR or
FileReaderSync). Additionally BlobBytesProvider might do blocking
file operations, which shouldn't be done on the main thread anyway, so
this moves the code to always run on the File thread. In the future it
might make sense to have part of BlobBytesProvider on the IO thread with
only the file operations on the File thread (matching the old IPC based
implementation), but for now doing all its work on the File thread should
work just fine.

This also required actually exposing a File task runner in blink::Platform.
This file task runner is not shared with the file thread in content, but
then the content one is only created on demand (for ppapi and MHTML saving)
anyway, so in most cases this should not result in extra threads.

Also add a virtual test suite to enable FileAPI (blob) layout and WPT tests
with mojo blobs, now that creating blobs works.

Bug:  740584 ,  744774 
Change-Id: Icc1e22f662dea895e1d1b2f1a1cc79c885e6706e
Reviewed-on: https://chromium-review.googlesource.com/565641
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490636}
[modify] https://crrev.com/8c2764154c196bf55a141141914255ff866b1c12/content/renderer/render_thread_impl.cc
[modify] https://crrev.com/8c2764154c196bf55a141141914255ff866b1c12/content/renderer/render_thread_impl.h
[modify] https://crrev.com/8c2764154c196bf55a141141914255ff866b1c12/third_party/WebKit/LayoutTests/NeverFixTests
[modify] https://crrev.com/8c2764154c196bf55a141141914255ff866b1c12/third_party/WebKit/LayoutTests/VirtualTestSuites
[add] https://crrev.com/8c2764154c196bf55a141141914255ff866b1c12/third_party/WebKit/LayoutTests/virtual/mojo-blobs/external/wpt/FileAPI/README.txt
[add] https://crrev.com/8c2764154c196bf55a141141914255ff866b1c12/third_party/WebKit/LayoutTests/virtual/mojo-blobs/fast/files/README.txt
[modify] https://crrev.com/8c2764154c196bf55a141141914255ff866b1c12/third_party/WebKit/Source/platform/CrossThreadCopier.h
[modify] https://crrev.com/8c2764154c196bf55a141141914255ff866b1c12/third_party/WebKit/Source/platform/blob/BlobBytesProvider.cpp
[modify] https://crrev.com/8c2764154c196bf55a141141914255ff866b1c12/third_party/WebKit/Source/platform/blob/BlobData.cpp
[modify] https://crrev.com/8c2764154c196bf55a141141914255ff866b1c12/third_party/WebKit/Source/platform/exported/Platform.cpp
[modify] https://crrev.com/8c2764154c196bf55a141141914255ff866b1c12/third_party/WebKit/public/platform/Platform.h

Apologies, applied the wrong component in bulk.

Setting Internals>Services>Storage to all children of issue 611935

Update: TaskScheduler APIs now can be used in Blink and the only place that used FileTaskRunner was converted (https://chromium-review.googlesource.com/c/chromium/src/+/976098). Now we need to delete Platform::FileTaskRunner ( issue 828364 ).

We'll revisit the decision to expose IO task runner when needed, marking as WontFix for now.