New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 872236 link

Starred by 1 user
Status: Available
Owner: ----
Cc:
leszeks@chromium.org
altimin@chromium.org
█ alexclarke@chromium.org
ksolt@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

Improve API for posting continuations

Project Member Reported by skyos...@chromium.org, Aug 8 
Working with task continuations is currently a little cumbersome because:

1) In Blink, there's no way to get the task queue you're currently on to post a continuation.

2) It's not obvious how to decide whether posting a continuation is necessary or not.

3) There's no handy utility API for "run immediately or post a continuation" which might be a useful pattern to encourage.

I think we should address these issues in stages:

1) Add a getter for the currently running task's TaskType. This unblocks[1] by letting the streaming code check whether it's already running on the LoadingControl task queue or not.

2) Make sure ThreadTaskRunnerHandle and SequencedTaskRunnerHandle return the current task queue's runner while the task is executing.

3) Design a utility API for the "post or run this task" case. It should use the ShouldYield() API internally in order to allow the scheduler to interject if necessary.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1165547

Comment 1 by altimin@chromium.org, Aug 8 

I think that there is an extra step between 1) and 2) — change the semantics of RunsTasksInCurrentSequence to account for different task queues.

The big problem with ThreadTaskRunnerHandle::Get is that it is used extensively now and  it will be very hard to distinguish between "a valid continuation task" and "legacy call, LocalFrame::GetTaskRunner()->PostTask() should be used". A particular concern is that a task might be incorrectly posted to the current task runner.

A solution to that problem might be to ban running javascript on the tasks posted through ThreadTaskRunnerHandle::Get.

Comment 2 by skyos...@chromium.org, Aug 8 

Ah, right, forgot about RunsTasksInCurrentSequence. Essentially it should become conditional on the currently running task queue too.

ThreadTaskRunnerHandle::Get is indeed tricky. However I don't think SequencedTaskRunnerHandle should behave differently from it since it would be very hard to explain why that's the case. I'm tempted to just make this change and see what breaks to figure out the constraints here. Another option here would be to keep the current behavior and steer people toward the API from step 3.

Comment 3 by altimin@chromium.org, Aug 8 

The problem is that breakage from this change can be very subtle (and have security implications with cooperative scheduling if we fail to freeze a task from the frame). Probably jumping straight to the 3) might be much easier (and we'll tackle portions of 2) independently).

Comment 4 by leszeks@chromium.org, Aug 8 

So the plan would be 1) followed by 3)?

Comment 5 by skyos...@chromium.org, Aug 8 

Yes, I think that makes sense.

Comment 6 by skyos...@chromium.org, Aug 15

Cc: ksolt@chromium.org

Comment 7 by altimin@chromium.org, Aug 16

Labels: -Type-Bug Type-Feature
►

Sign in to add a comment