See also comment here, where this was brought up during code review of the WebThread refactor:

https://chromium-review.googlesource.com/c/chromium/src/+/1216663/15/components/proxy_config/ios/proxy_service_factory.cc#23

While I understand the premise of this discussion, if we take a step back and look at the documentation of base::PostTask : it guarantees that all tasks posted with the same traits will be scheduled in posted order (and this are guaranteed to run in posted order if the traits imply a single consuming thread).

As such, it's kind of irrelevant whether the task runners are the same or different instances (it's recounted anyways). What matters is that two task runners with the same traits will have the same property as base::PostTask w.r.t. ordering. One can think of it as independent task runners funnelled to a single thread which will be forced to keep ordering of tasks with the same traits per ordering being their only differentiating factor.

That's a good point -- the difference is just that the target thread requirement implies that tasks with the same traits will be scheduled and executed in sequence. Perhaps "Get" would make that more obvious than "Create" from the callers point of view?

"Get" would be clearer than "Create" indeed for the BrowserThread case, but it's wrong for the pool case (it is indeed creating an independent sequence).

My point in #5 however is that "Create" is not wrong in the BrowserThread case either.

We can think of the BrowserThread case as creating "independent" sequences which funnel to one thread anyways and for which incoming tasks are ordered by "post time" if no other trait differentiates them.

Maybe "Obtain" achieves both? It's like "Get" but doesn't preclude creating an independent one if it DNE?

This is tricky and I appreciate the comment in #5, I don't have a problem with base::PostTaskWithTraits.  The call sites for base::Create*TaskRunnerWithTraits and a well known thread look a bit odd.  Currently these are in the minority, but once the trait refactor lands I suspect will no longer be the case.  In fact it may be wrong for most call sites.

With that in mind, how do we be feel about using base::Get*TaskRunnerWithTraits and adding a new trait base::UniqueSequence to disambiguate? Alternatively we could have both base::Get*TaskRunnerWithTraits and base::Create*TaskRunnerWithTraits and require the former with well known threads.  We'd want some other (likely internal) API for actually creating per-frame sequences. 

I like the simplicity of "everything with identical traits is always scheduled in posted order". However to make that consistent with worker pool tasks I think we'd need some kind of unique sequence trait like Alex suggested (if we're going for the Get vs. Create semantics). I'm not sure about Obtain vs. Get vs. Create, but I think the main thing is making this crystal clear to the callers.

Per-frame sequences would still be identified by unique traits, right? As in PostTask({FrameId(1234), TaskType::kLoading}) vs. PostTask({FrameId(1234), TaskType::kIPC}). In other words, these could also work through the public API I think.

Yes per-frame sequences would still be identified by unique traits.  I suspect if we've not encountered (say) {FrameId(1234), TaskType::kIPC} before that would lazily construct the appropriate queue.  Ideally we'd also know when FrameId(1234) goes away so we can remove the queues.  

As an aside, the current FrameTreeNode observer API is a little awkward for that, ideally we'd have some global observer rather than having to register with every FrameTreeNode.

I'd strongly prefer not to mix "Get" and "Create" semantics, that'll be
most confusing IMO.

The way I see it "Create" is just creating a task runner which will post
tasks to run in order on the destination sequence. If the destination
sequence is shared (inferred by the traits), then ordering of tasks between
these task runners follows base::PostTask semantics (which already defines
"tasks with same traits are scheduled in posting order).

Whether we expose Get/Create we also have the issue of when does a task
posted to a task runner run w.r.t. a task posted to base::PostTask. Having
the two be documented as using the same ordering is what matters IMO. Then
Get/Create is just an impl detail of whether the task runner instance is
the same or a new one that points to the same shared queue and that's not
really relevant.

Or we could go a step further and say that if you *really* care about
ordering (not just mutual exclusion), then you should reuse the *same*
instance of the task runner (in which case Create makes even more sense as
it truly creates an independent sequence). In general that's proven to be
the right approach for TaskScheduler (the vast majority of components do
not have ordering constraints w.r.t. other components).

Le mer. 12 sept. 2018 07 h 00, alexclarke via monorail <
monorail+v2.2900467117@chromium.org> a écrit :

Let me repeat my 2 cents from this code review [1].

"...as a regular user, it's not obvious to me that this CreateFoo call actually returns a shared instance. Or maybe it doesn't? I will be immediately concerned when posting multiple tasks that they won't get to a single task runner and the order will be messed up..."

Let me provide a typical usecase. Many times I know that some methods MyClass::DoFoo{1,2,3} are called in a specific order, and each of them posts a task to, say, IO thread. Currently, I am 100% sure that IO thread will execute the tasks in the same order as DoFoo{x} were called. With new API, it's not obvious to me.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1235728/3/content/public/browser/browser_thread.h#73

Given the strong confusion by many reviewers, I think we need to do something here.

I guess Get*TaskRunnerWithTraits() is less confusing overall? When the traits don't identify an existing sequence, that may create one but that's okay..?

How about Obtain*TaskRunnerWithTraits()? That's not Get nor Create, so it'll get people to read the doc which will make it clear that this an existing sequence iff the traits identify an existing sequence?

I'd +1 "Obtain" for the reasons you mention. People may not expect "Get" to also sometimes create.

FWIW, there are a few other naming bits and pieces that we may want to discuss together with this rename, e.g. dropping "WithTraits". I've started to collect some of those here:

https://docs.google.com/document/d/1-rH9f_n65UTYogXAoeuHt6jsljooVW2IjqRTLI50N8Y/edit?usp=drivesdk

LGTM, thanks for writing this down

Le mar. 25 sept. 2018 02 h 25, eseckler via monorail <
monorail+v2.1164142555@chromium.org> a écrit :

Quoting gab@: We'd like to keep "Create" semantics after all as in the future it might be possible to dynamically adjust the traits of a task runner and that really should only apply to the task runner instance you own (not an underlying shared task runner). So while you will get a task runner which funnels to a shared queue today (and has ordering guarantees with other TRs with same traits), we don't want to guarantee that this is the *same* TaskRunner object as that would tie our hands for such future improvements.

We're adding some documentation in this regard to browser_task_traits.h.

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

commit e8b52905e9c7f996550eb3507669b741a50022be
Author: Eric Seckler <eseckler@chromium.org>
Date: Fri Sep 28 10:33:06 2018

content: Document BrowserThread task posting order guarantees.

Bug:  878356 , 882872
Change-Id: I16af203c203b82efc408d2153c43eb3cb6fcc7b9
Reviewed-on: https://chromium-review.googlesource.com/1246361
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Commit-Queue: Eric Seckler <eseckler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595043}
[modify] https://crrev.com/e8b52905e9c7f996550eb3507669b741a50022be/content/public/browser/browser_task_traits.h

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

commit 7ead3ef5225f3d87a85e632e6183d47a76ab4df1
Author: Eric Seckler <eseckler@chromium.org>
Date: Mon Oct 01 10:15:28 2018

ios: Document WebThread task posting order guarantees.

Replicating the equivalent BrowserThread documentation for ios, see
https://chromium-review.googlesource.com/c/chromium/src/+/1246361

Bug:  878356 , 882872
Change-Id: Ie9470baecbce1dbb5b6c45852836db238cffe3b7
Reviewed-on: https://chromium-review.googlesource.com/1253763
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595418}
[modify] https://crrev.com/7ead3ef5225f3d87a85e632e6183d47a76ab4df1/ios/web/public/web_task_traits.h