Use MainThread on EmptyFrameScheduler

Use MainThread on EmptyFrameScheduler

The dummy Frame on Workers has EmptyFrameScheduler, and it returns a
WebTaskRunner for the current thread. However, since all Frame should
belong to the main thread, the resulting WebTaskRunner should run on
the main thread even when the task runner is requested on a non-main
thread.
This CL ensures EmptyFrameScheduler to use the main thread task runner.

Committed: https://crrev.com/8e40d35a47b2bbdf70d9a4d13c3978678a5d9628
Cr-Commit-Position: refs/heads/master@{#436265}

[tzik, 2016-12-02 13:12:42]

The CQ bit was checked by tzik@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-02 13:12:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-02 14:57:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-02 14:57:35]

Dry run: This issue passed the CQ dry run.

[tzik, 2016-12-05 06:10:51]

Description was changed from

==========
Use MainThread on EmptyFrameScheduler

BUG=
==========

to

==========
Use MainThread on EmptyFrameScheduler

The dummy Frame on Workers has EmptyFrameScheduler, and it returns a
WebTaskRunner for the current thread. As the result, TaskRunnerHelper::get()
on the dummy Document returns non-main thread, OTOH, its
ExecutionContext::postTask posts the task to the main thread, which is
inconsistent.

This CL forces EmptyFrameScheduler to return the main thread task runner as a
workaround.
==========

[tzik, 2016-12-05 06:10:51]

tzik@chromium.org changed reviewers:
+ nhiroki@chromium.org, yhirano@chromium.org

[tzik, 2016-12-05 06:11:01]

PTAL

[yhirano, 2016-12-05 06:26:45]

> OTOH, its ExecutionContext::postTask posts the task to the main thread

What is "its ExecutionContext::postTask"?

[tzik, 2016-12-05 06:29:21]

On 2016/12/05 06:26:45, yhirano wrote:
> > OTOH, its ExecutionContext::postTask posts the task to the main thread
> 
> What is "its ExecutionContext::postTask"?

I meant Document::postTask() that overrides ExecutionContext::postTask().

[nhiroki, 2016-12-05 06:37:34]

Can you provide a concrete example/callpath when a task is accidentally posted
to a worker's thread?

[yhirano, 2016-12-05 07:03:23]

Are you saying that EmptyFrameScheduler is always attached to an object
belonging to the main thread? If so, Can you add DCHECK(isMainThread()) in the
EmptyFrameScheduler constructor?

[tzik, 2016-12-05 07:10:10]

On 2016/12/05 06:37:34, nhiroki wrote:
> Can you provide a concrete example/callpath when a task is accidentally posted
> to a worker's thread?

Here is not-yet-landed failing case:
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...

The call path is:
WorkerThreadableLoader::start()
WorkerLoaderProxy::postTaskToLoader()
WebEmbeddedWorkerImpl::postTaskToLoader()
Where postTaskToLoader() calls Document::postTask().

[tzik, 2016-12-05 07:17:34]

The CQ bit was checked by tzik@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-05 07:17:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tzik, 2016-12-05 07:17:49]

On 2016/12/05 07:03:23, yhirano wrote:
> Are you saying that EmptyFrameScheduler is always attached to an object
> belonging to the main thread? If so, Can you add DCHECK(isMainThread()) in the
> EmptyFrameScheduler constructor?

Yes. IIUC, all of Page, Frame and Document are always on the main thread.

> Can you add DCHECK(isMainThread()) in the
> EmptyFrameScheduler constructor?

Done

[tzik, 2016-12-05 07:24:48]

The CQ bit was checked by tzik@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-05 07:25:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[nhiroki, 2016-12-05 08:34:53]

lgtm

[commit-bot: I haz the power, 2016-12-05 09:11:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-05 09:11:30]

Dry run: This issue passed the CQ dry run.

[yhirano, 2016-12-05 09:13:24]

code LGTM, but Modifying the issue description would be helpful.

[tzik, 2016-12-05 12:03:45]

Description was changed from

==========
Use MainThread on EmptyFrameScheduler

The dummy Frame on Workers has EmptyFrameScheduler, and it returns a
WebTaskRunner for the current thread. As the result, TaskRunnerHelper::get()
on the dummy Document returns non-main thread, OTOH, its
ExecutionContext::postTask posts the task to the main thread, which is
inconsistent.

This CL forces EmptyFrameScheduler to return the main thread task runner as a
workaround.
==========

to

==========
Use MainThread on EmptyFrameScheduler

The dummy Frame on Workers has EmptyFrameScheduler, and it returns a
WebTaskRunner for the current thread. However, since all Frame should
belong to the main thread, the resulting WebTaskRunner should run on
the main thread even when the task runner is requested on a non-main
thread.
This CL ensures EmptyFrameScheduler to use the main thread task runner.
subject: Use MainThread on EmptyFrameScheduler
==========

[tzik, 2016-12-05 12:03:49]

The CQ bit was checked by tzik@chromium.org

[commit-bot: I haz the power, 2016-12-05 12:03:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tzik, 2016-12-05 12:06:02]

The CQ bit was unchecked by tzik@chromium.org

[tzik, 2016-12-05 12:06:14]

Description was changed from

==========
Use MainThread on EmptyFrameScheduler

The dummy Frame on Workers has EmptyFrameScheduler, and it returns a
WebTaskRunner for the current thread. However, since all Frame should
belong to the main thread, the resulting WebTaskRunner should run on
the main thread even when the task runner is requested on a non-main
thread.
This CL ensures EmptyFrameScheduler to use the main thread task runner.
subject: Use MainThread on EmptyFrameScheduler
==========

to

==========
Use MainThread on EmptyFrameScheduler

The dummy Frame on Workers has EmptyFrameScheduler, and it returns a
WebTaskRunner for the current thread. However, since all Frame should
belong to the main thread, the resulting WebTaskRunner should run on
the main thread even when the task runner is requested on a non-main
thread.
This CL ensures EmptyFrameScheduler to use the main thread task runner.
==========

[tzik, 2016-12-05 12:06:21]

The CQ bit was checked by tzik@chromium.org

[commit-bot: I haz the power, 2016-12-05 12:06:30]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-05 12:10:17]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1480939581573250,
"parent_rev": "4497961c272ebf60cc5cffb1dbbb1bf167c697d8", "commit_rev":
"7b1d9cdfcf1a2b9471f1cdbd116a2970fb962757"}

[commit-bot: I haz the power, 2016-12-05 12:10:41]

Description was changed from

==========
Use MainThread on EmptyFrameScheduler

The dummy Frame on Workers has EmptyFrameScheduler, and it returns a
WebTaskRunner for the current thread. However, since all Frame should
belong to the main thread, the resulting WebTaskRunner should run on
the main thread even when the task runner is requested on a non-main
thread.
This CL ensures EmptyFrameScheduler to use the main thread task runner.
==========

to

==========
Use MainThread on EmptyFrameScheduler

The dummy Frame on Workers has EmptyFrameScheduler, and it returns a
WebTaskRunner for the current thread. However, since all Frame should
belong to the main thread, the resulting WebTaskRunner should run on
the main thread even when the task runner is requested on a non-main
thread.
This CL ensures EmptyFrameScheduler to use the main thread task runner.
==========

[commit-bot: I haz the power, 2016-12-05 12:10:42]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-05 12:12:15]

Description was changed from

==========
Use MainThread on EmptyFrameScheduler

The dummy Frame on Workers has EmptyFrameScheduler, and it returns a
WebTaskRunner for the current thread. However, since all Frame should
belong to the main thread, the resulting WebTaskRunner should run on
the main thread even when the task runner is requested on a non-main
thread.
This CL ensures EmptyFrameScheduler to use the main thread task runner.
==========

to

==========
Use MainThread on EmptyFrameScheduler

The dummy Frame on Workers has EmptyFrameScheduler, and it returns a
WebTaskRunner for the current thread. However, since all Frame should
belong to the main thread, the resulting WebTaskRunner should run on
the main thread even when the task runner is requested on a non-main
thread.
This CL ensures EmptyFrameScheduler to use the main thread task runner.

Committed: https://crrev.com/8e40d35a47b2bbdf70d9a4d13c3978678a5d9628
Cr-Commit-Position: refs/heads/master@{#436265}
==========

[commit-bot: I haz the power, 2016-12-05 12:12:16]

Patchset 3 (id:??) landed as
https://crrev.com/8e40d35a47b2bbdf70d9a4d13c3978678a5d9628
Cr-Commit-Position: refs/heads/master@{#436265}