scheduler: Always create a real scheduler in unit tests

scheduler: Always create a real scheduler in unit tests

Previously we would create a dummy scheduler in any test which uses
TestBlinkWebUnitTestSupport without first initializing a message loop.
This causes problems because the dummy scheduler ignores all tasks it
is given.

This patch makes the tests more realistic by always creating a real
renderer scheduler regardless of whether we have a message loop or not.
This is achieved by lazily binding the scheduler to the message loop
the first time it is needed.

Longer term we would like to refactor these test suites to ensure Blink
always has a valid message loop when it is initialized, but this will
involve rewiring several tests.

BUG=463143, 495659
Committed: https://crrev.com/087644f1eab41927823a1a2fc2df08bd4e10fe18
Cr-Commit-Position: refs/heads/master@{#332685}

Committed: https://crrev.com/ae456cc035b8bb76c11d47f28b2efab3dea0c26d
Cr-Commit-Position: refs/heads/master@{#332818}

[Sami, 2015-06-02 15:54:07]

skyostil@chromium.org changed reviewers:
+ alexclarke@chromium.org, rmcilroy@chromium.org

[Sami, 2015-06-02 15:59:32]

skyostil@chromium.org changed reviewers:
+ sievers@chromium.org

[alex clarke (OOO till 29th), 2015-06-02 16:01:46]

lgtm

https://codereview.chromium.org/1152623008/diff/1/components/scheduler/render...
File components/scheduler/renderer/lazy_scheduler_message_loop_delegate.h
(right):

https://codereview.chromium.org/1152623008/diff/1/components/scheduler/render...
components/scheduler/renderer/lazy_scheduler_message_loop_delegate.h:38:
protected:
Is this needed?

https://codereview.chromium.org/1152623008/diff/1/components/scheduler/render...
components/scheduler/renderer/lazy_scheduler_message_loop_delegate.h:42:
LazySchedulerMessageLoopDelegate();
nit: blank line after constructor?

[Sami, 2015-06-02 16:08:39]

Thanks Alex.

https://codereview.chromium.org/1152623008/diff/1/components/scheduler/render...
File components/scheduler/renderer/lazy_scheduler_message_loop_delegate.h
(right):

https://codereview.chromium.org/1152623008/diff/1/components/scheduler/render...
components/scheduler/renderer/lazy_scheduler_message_loop_delegate.h:38:
protected:
On 2015/06/02 16:01:45, alexclarke1 wrote:
> Is this needed?

Right, made the destructor private.

https://codereview.chromium.org/1152623008/diff/1/components/scheduler/render...
components/scheduler/renderer/lazy_scheduler_message_loop_delegate.h:42:
LazySchedulerMessageLoopDelegate();
On 2015/06/02 16:01:45, alexclarke1 wrote:
> nit: blank line after constructor?

Done.

[rmcilroy, 2015-06-02 16:53:07]

Generally looks good, but I would prefer that this was test-only specific.

https://codereview.chromium.org/1152623008/diff/60001/components/scheduler/re...
File components/scheduler/renderer/lazy_scheduler_message_loop_delegate.h
(right):

https://codereview.chromium.org/1152623008/diff/60001/components/scheduler/re...
components/scheduler/renderer/lazy_scheduler_message_loop_delegate.h:21: class
SCHEDULER_EXPORT LazySchedulerMessageLoopDelegate
Maybe we should call this LazySchedulerMessageLoopDelegateForTests if this
should only be used in tests?

https://codereview.chromium.org/1152623008/diff/60001/components/scheduler/sc...
File components/scheduler/scheduler.gypi (right):

https://codereview.chromium.org/1152623008/diff/60001/components/scheduler/sc...
components/scheduler/scheduler.gypi:48:
'renderer/lazy_scheduler_message_loop_delegate.h',
Could we have this as a separate component (or something similar) which is only
depended on by content_tests to avoid compiling it into Chrome itself?

[Sami, 2015-06-02 17:26:21]

https://codereview.chromium.org/1152623008/diff/60001/components/scheduler/re...
File components/scheduler/renderer/lazy_scheduler_message_loop_delegate.h
(right):

https://codereview.chromium.org/1152623008/diff/60001/components/scheduler/re...
components/scheduler/renderer/lazy_scheduler_message_loop_delegate.h:21: class
SCHEDULER_EXPORT LazySchedulerMessageLoopDelegate
On 2015/06/02 16:53:07, rmcilroy wrote:
> Maybe we should call this LazySchedulerMessageLoopDelegateForTests if this
> should only be used in tests?

I thought about that but shied away because the include guards didn't fit into
80 chars anymore :) Done.

https://codereview.chromium.org/1152623008/diff/60001/components/scheduler/sc...
File components/scheduler/scheduler.gypi (right):

https://codereview.chromium.org/1152623008/diff/60001/components/scheduler/sc...
components/scheduler/scheduler.gypi:48:
'renderer/lazy_scheduler_message_loop_delegate.h',
On 2015/06/02 16:53:07, rmcilroy wrote:
> Could we have this as a separate component (or something similar) which is
only
> depended on by content_tests to avoid compiling it into Chrome itself?

Good idea! I've now made an attempt of doing that.

[no sievers, 2015-06-02 20:27:46]

content lgtm

[rmcilroy, 2015-06-02 20:47:19]

On 2015/06/02 20:27:46, sievers wrote:
> content lgtm

lgtm, thanks!

[Sami, 2015-06-03 11:41:43]

The CQ bit was checked by skyostil@chromium.org

[Sami, 2015-06-03 11:41:43]

The patchset sent to the CQ was uploaded after l-g-t-m from
alexclarke@chromium.org, sievers@chromium.org, rmcilroy@chromium.org
Link to the patchset: https://codereview.chromium.org/1152623008/#ps120001
(title: "Fix windows build.")

[commit-bot: I haz the power, 2015-06-03 11:42:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1152623008/120001

[commit-bot: I haz the power, 2015-06-03 12:50:22]

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

[commit-bot: I haz the power, 2015-06-03 12:50:22]

Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Sami, 2015-06-03 16:56:24]

The CQ bit was checked by skyostil@chromium.org

[Sami, 2015-06-03 16:56:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
sievers@chromium.org, rmcilroy@chromium.org, alexclarke@chromium.org
Link to the patchset: https://codereview.chromium.org/1152623008/#ps140001
(title: "Really fix the windows build.")

[Sami, 2015-06-03 16:56:47]

The CQ bit was unchecked by skyostil@chromium.org

[Sami, 2015-06-03 16:58:46]

The CQ bit was checked by skyostil@chromium.org

[Sami, 2015-06-03 16:58:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
sievers@chromium.org, rmcilroy@chromium.org, alexclarke@chromium.org
Link to the patchset: https://codereview.chromium.org/1152623008/#ps160001
(title: "DEPS typo.")

[commit-bot: I haz the power, 2015-06-03 17:00:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1152623008/160001

[commit-bot: I haz the power, 2015-06-03 17:38:36]

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

[commit-bot: I haz the power, 2015-06-03 17:38:38]

Try jobs failed on following builders:
  linux_chromium_gn_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Sami, 2015-06-03 18:13:25]

The CQ bit was checked by skyostil@chromium.org

[Sami, 2015-06-03 18:13:26]

The patchset sent to the CQ was uploaded after l-g-t-m from
sievers@chromium.org, rmcilroy@chromium.org, alexclarke@chromium.org
Link to the patchset: https://codereview.chromium.org/1152623008/#ps180001
(title: "Build a static library for GN too.")

[commit-bot: I haz the power, 2015-06-03 18:16:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1152623008/180001

[commit-bot: I haz the power, 2015-06-03 19:33:34]

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

[commit-bot: I haz the power, 2015-06-03 19:33:35]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Sami, 2015-06-03 20:03:17]

The CQ bit was checked by skyostil@chromium.org

[commit-bot: I haz the power, 2015-06-03 20:04:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1152623008/180001

[commit-bot: I haz the power, 2015-06-03 21:21:09]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-06-03 21:22:03]

Patchset 10 (id:??) landed as
https://crrev.com/087644f1eab41927823a1a2fc2df08bd4e10fe18
Cr-Commit-Position: refs/heads/master@{#332685}

[tkent, 2015-06-04 01:27:28]

A revert of this CL (patchset #10 id:180001) has been created in
https://codereview.chromium.org/1156463004/ by tkent@chromium.org.

The reason for reverting is: Caused crashes of webkit_unit_tests.
e.g.
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/48714
.

[alex clarke (OOO till 29th), 2015-06-04 10:46:08]

lgtm

Fix looks good.

[Sami, 2015-06-04 10:46:32]

The CQ bit was checked by skyostil@chromium.org

[Sami, 2015-06-04 10:46:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
sievers@chromium.org, rmcilroy@chromium.org
Link to the patchset: https://codereview.chromium.org/1152623008/#ps200001
(title: "Remember the message loop binding.")

[commit-bot: I haz the power, 2015-06-04 10:46:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1152623008/200001

[commit-bot: I haz the power, 2015-06-04 11:42:14]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2015-06-04 11:42:57]

Patchset 11 (id:??) landed as
https://crrev.com/ae456cc035b8bb76c11d47f28b2efab3dea0c26d
Cr-Commit-Position: refs/heads/master@{#332818}