Add missing TaskScheduler on tests using RendereWidgetHostImpl

Add missing TaskScheduler on tests using RendereWidgetHostImpl

RenderWidgetHostImpl constructor requires an instance of TaskScheduler
on Windows. Its absence causes DCHECK failure or null ptr access
failure only on Windows, which is hard to find for non-windows devs.

This CL adds a DCHECK to detect it on non-Windows build, so that we
notice the failure earlier, and also fixes the failure detected by
the DCHECK.

BUG=696919
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2785883004
Cr-Commit-Position: refs/heads/master@{#461134}
Committed: https://chromium.googlesource.com/chromium/src/+/5d6c4a51e01d9f6d37e24b1964bb482421808b73

[tzik, 2017-03-31 04:14:04]

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

[commit-bot: I haz the power, 2017-03-31 04:14:27]

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

[tzik, 2017-03-31 04:15:04]

Description was changed from

==========
Add DCHECK to RendereWidgetHostImpl to ensure having TaskScheduler instance

RenderWidgetHostImpl constructor requires an instance of TaskScheduler
on Windows. If its absence causes DCHECK failure or null ptr access
failure only on Windows, which is hard to find for non-windows devs.

This CL adds a DCHECK to detect it on non-Windows build, so that we
notice the failure earlier.
==========

to

==========
Add DCHECK to RendereWidgetHostImpl to ensure having TaskScheduler instance

RenderWidgetHostImpl constructor requires an instance of TaskScheduler
on Windows. If its absence causes DCHECK failure or null ptr access
failure only on Windows, which is hard to find for non-windows devs.

This CL adds a DCHECK to detect it on non-Windows build, so that we
notice the failure earlier.

BUG=696919
==========

[tzik, 2017-03-31 04:15:04]

tzik@chromium.org changed reviewers:
+ kinuko@chromium.org

[tzik, 2017-03-31 04:15:12]

PTAL

[commit-bot: I haz the power, 2017-03-31 04:38:30]

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

[commit-bot: I haz the power, 2017-03-31 04:38:31]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[kinuko, 2017-03-31 04:41:54]

Looks like some tests are failing on DCHECK?

[tzik, 2017-03-31 04:47:38]

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

[commit-bot: I haz the power, 2017-03-31 04:48:00]

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

[tzik, 2017-03-31 04:49:17]

Description was changed from

==========
Add DCHECK to RendereWidgetHostImpl to ensure having TaskScheduler instance

RenderWidgetHostImpl constructor requires an instance of TaskScheduler
on Windows. If its absence causes DCHECK failure or null ptr access
failure only on Windows, which is hard to find for non-windows devs.

This CL adds a DCHECK to detect it on non-Windows build, so that we
notice the failure earlier.

BUG=696919
==========

to

==========
Add DCHECK to RendereWidgetHostImpl to ensure having TaskScheduler instance

RenderWidgetHostImpl constructor requires an instance of TaskScheduler
on Windows. If its absence causes DCHECK failure or null ptr access
failure only on Windows, which is hard to find for non-windows devs.

This CL adds a DCHECK to detect it on non-Windows build, so that we
notice the failure earlier.

BUG=696919
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[tzik, 2017-03-31 04:49:37]

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

[commit-bot: I haz the power, 2017-03-31 04:50:01]

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

[tzik, 2017-03-31 04:51:31]

On 2017/03/31 04:41:54, kinuko wrote:
> Looks like some tests are failing on DCHECK?

Updated the CL to fix it. Let's see whether it works.

[tzik, 2017-03-31 05:13:15]

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

[commit-bot: I haz the power, 2017-03-31 05:13:30]

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

[kinuko, 2017-03-31 05:22:29]

lgtm if it goes green

[tzik, 2017-03-31 05:56:17]

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

[commit-bot: I haz the power, 2017-03-31 05:56:35]

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

[tzik, 2017-03-31 05:59:28]

The CQ bit was unchecked by tzik@chromium.org

[tzik, 2017-03-31 06:06:24]

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

[commit-bot: I haz the power, 2017-03-31 06:06:41]

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, 2017-03-31 06:51:36]

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

[commit-bot: I haz the power, 2017-03-31 06:51:37]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tzik, 2017-03-31 11:10:22]

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

[commit-bot: I haz the power, 2017-03-31 11:10:37]

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

[tzik, 2017-03-31 11:17:24]

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

[commit-bot: I haz the power, 2017-03-31 11:17:34]

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

[tzik, 2017-03-31 11:22:48]

kinuko: I updated text_input_client_mac_unittest.mm for a test fix. Could you
take another look to it?

[kinuko, 2017-03-31 11:37:17]

One test's still failing for PS7, is that fixed?

[tzik, 2017-03-31 11:49:52]

On 2017/03/31 11:37:17, kinuko wrote:
> One test's still failing for PS7, is that fixed?

😨

[commit-bot: I haz the power, 2017-03-31 12:01:46]

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

[commit-bot: I haz the power, 2017-03-31 12:01:47]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tzik, 2017-03-31 12:02:31]

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

[commit-bot: I haz the power, 2017-03-31 12:02:55]

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

[tzik, 2017-03-31 12:03:16]

On 2017/03/31 11:37:17, kinuko wrote:
> One test's still failing for PS7, is that fixed?

That should be fixed on PS9.

[commit-bot: I haz the power, 2017-03-31 13:25:33]

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

[commit-bot: I haz the power, 2017-03-31 13:25:34]

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

[tzik, 2017-03-31 13:42:55]

On 2017/03/31 12:03:16, tzik wrote:
> On 2017/03/31 11:37:17, kinuko wrote:
> > One test's still failing for PS7, is that fixed?
> 
> That should be fixed on PS9.

Renaming the CL, as this is no longer just adding DCHECK.

[tzik, 2017-03-31 13:45:22]

Description was changed from

==========
Add DCHECK to RendereWidgetHostImpl to ensure having TaskScheduler instance

RenderWidgetHostImpl constructor requires an instance of TaskScheduler
on Windows. If its absence causes DCHECK failure or null ptr access
failure only on Windows, which is hard to find for non-windows devs.

This CL adds a DCHECK to detect it on non-Windows build, so that we
notice the failure earlier.

BUG=696919
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add missing TaskScheduler on tests using RendereWidgetHostImpl

RenderWidgetHostImpl constructor requires an instance of TaskScheduler
on Windows. Its absence causes DCHECK failure or null ptr access
failure only on Windows, which is hard to find for non-windows devs.

This CL adds a DCHECK to detect it on non-Windows build, so that we
notice the failure earlier, and also fixes the failure detected by
the DCHECK.

BUG=696919
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[kinuko, 2017-03-31 14:26:55]

still lgtm

https://codereview.chromium.org/2785883004/diff/160001/content/browser/render...
File content/browser/renderer_host/text_input_client_mac_unittest.mm (right):

https://codereview.chromium.org/2785883004/diff/160001/content/browser/render...
content/browser/renderer_host/text_input_client_mac_unittest.mm:65: // deletes
the MRPH before scheduled MRPH deletion is invoked.
Whoa.

[tzik, 2017-03-31 15:21:30]

The CQ bit was checked by tzik@chromium.org

[commit-bot: I haz the power, 2017-03-31 15:21:57]

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, 2017-03-31 15:53:50]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1490973690754410,
"parent_rev": "8ba8d7142ef4449775d3d75b6f17b80d24d07f77", "commit_rev":
"5d6c4a51e01d9f6d37e24b1964bb482421808b73"}

[commit-bot: I haz the power, 2017-03-31 15:54:41]

Description was changed from

==========
Add missing TaskScheduler on tests using RendereWidgetHostImpl

RenderWidgetHostImpl constructor requires an instance of TaskScheduler
on Windows. Its absence causes DCHECK failure or null ptr access
failure only on Windows, which is hard to find for non-windows devs.

This CL adds a DCHECK to detect it on non-Windows build, so that we
notice the failure earlier, and also fixes the failure detected by
the DCHECK.

BUG=696919
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add missing TaskScheduler on tests using RendereWidgetHostImpl

RenderWidgetHostImpl constructor requires an instance of TaskScheduler
on Windows. Its absence causes DCHECK failure or null ptr access
failure only on Windows, which is hard to find for non-windows devs.

This CL adds a DCHECK to detect it on non-Windows build, so that we
notice the failure earlier, and also fixes the failure detected by
the DCHECK.

BUG=696919
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2785883004
Cr-Commit-Position: refs/heads/master@{#461134}
Committed:
https://chromium.googlesource.com/chromium/src/+/5d6c4a51e01d9f6d37e24b1964bb...
==========

[commit-bot: I haz the power, 2017-03-31 15:54:42]

Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/5d6c4a51e01d9f6d37e24b1964bb...

[gab, 2017-03-31 18:06:58]

gab@chromium.org changed reviewers:
+ gab@chromium.org

[gab, 2017-03-31 18:06:59]

drive-by comment about ScopedTaskScheduler :)

https://codereview.chromium.org/2785883004/diff/160001/content/browser/render...
File
content/browser/renderer_host/render_widget_host_view_mac_editcommand_helper_unittest.mm
(right):

https://codereview.chromium.org/2785883004/diff/160001/content/browser/render...
content/browser/renderer_host/render_widget_host_view_mac_editcommand_helper_unittest.mm:136:
base::test::ScopedTaskScheduler task_scheduler(&message_loop);
I think you can merge the two lines above (ScopedTaskScheduler comes with an
implicit MessageLoop if not handed one and no callers below use |message_loop|
explicitly.

PS: Yeah, I know, this is kind of a mess... Aimed to be solved by
https://docs.google.com/document/d/1QabRo8c7D9LsYY3cEcaPQbOCLo8Tu-6VLykYXyl3P...