Add assertion to EmbeddedTestServer creation method

Add assertion to EmbeddedTestServer creation method

When the EmbeddedTestServer is created on UiThread, the test would
hang forever because it will hang waiting for tasks to post to UI thread

BUG=720155
Review-Url: https://codereview.chromium.org/2860673005
Cr-Commit-Position: refs/heads/master@{#473310}
Committed: https://chromium.googlesource.com/chromium/src/+/508534a90769468c845e0f0438fbcb4f4fe38d51

[the real yoland, 2017-05-04 00:27:31]

yolandyan@chromium.org changed reviewers:
+ jbudorick@chromium.org, phajdan.jr@chromium.org

[the real yoland, 2017-05-04 00:27:41]

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

[commit-bot: I haz the power, 2017-05-04 00:28:19]

Dry run: CQ is trying da patch.

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

[jbudorick, 2017-05-04 01:10:15]

https://codereview.chromium.org/2860673005/diff/1/net/test/android/javatests/...
File
net/test/android/javatests/src/org/chromium/net/test/EmbeddedTestServer.java
(right):

https://codereview.chromium.org/2860673005/diff/1/net/test/android/javatests/...
net/test/android/javatests/src/org/chromium/net/test/EmbeddedTestServer.java:230:
assert (Looper.getMainLooper() != Looper.myLooper())
This should use a the junit Assert, not the regular java assert.

[the real yoland, 2017-05-04 01:23:39]

https://codereview.chromium.org/2860673005/diff/1/net/test/android/javatests/...
File
net/test/android/javatests/src/org/chromium/net/test/EmbeddedTestServer.java
(right):

https://codereview.chromium.org/2860673005/diff/1/net/test/android/javatests/...
net/test/android/javatests/src/org/chromium/net/test/EmbeddedTestServer.java:230:
assert (Looper.getMainLooper() != Looper.myLooper())
On 2017/05/04 at 01:10:15, jbudorick wrote:
> This should use a the junit Assert, not the regular java assert.

Done

[jbudorick, 2017-05-04 01:26:00]

also: do you have more context for this? Where did it come up? Is there a bug?

[the real yoland, 2017-05-04 01:58:04]

On 2017/05/04 at 01:26:00, jbudorick wrote:
> also: do you have more context for this? Where did it come up? Is there a bug?

Not really a bug, just came up in local run, when @UiThreadTest makes @Before
run on UiThread, and if a ETS is created in @Before, it hangs the test forever
and times out on host python side.
There are a couple of tryjobs I ran with exactly same situation

e.g. 
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...
In the stdout log
https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.chromium.andro...

The output just ends after NewTabPageTest#testSetSearchProviderHasLogo starts
because testSetSearchProviderHasLogo had @UiThreadTest annotation causing the
setUp and ETS creation to run on UiThread.

In my local run logcat, it just keeps sending "cr_TestServer: Still waiting for
EmbeddedTestServer service connection."

[jbudorick, 2017-05-04 14:59:46]

On 2017/05/04 01:58:04, the real yoland wrote:
> On 2017/05/04 at 01:26:00, jbudorick wrote:
> > also: do you have more context for this? Where did it come up? Is there a
bug?
> 
> Not really a bug, just came up in local run, when @UiThreadTest makes @Before
> run on UiThread, and if a ETS is created in @Before, it hangs the test forever
> and times out on host python side.
> There are a couple of tryjobs I ran with exactly same situation
> 
> e.g. 
>
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...
> In the stdout log
>
https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.chromium.andro...
> 
> The output just ends after NewTabPageTest#testSetSearchProviderHasLogo starts
> because testSetSearchProviderHasLogo had @UiThreadTest annotation causing the
> setUp and ETS creation to run on UiThread.
> 
> In my local run logcat, it just keeps sending "cr_TestServer: Still waiting
for
> EmbeddedTestServer service connection."

huh, nice find. Can you summarize this in a bug and attach it to this CL? lgtm
otherwise

[Paweł Hajdan Jr., 2017-05-05 17:28:55]

LGTM

[the real yoland, 2017-05-09 23:36:43]

Description was changed from

==========
Add assertion to EmbeddedTestServer creation method

When the EmbeddedTestServer is created on UiThread, the test would
hang forever waiting for the server to start.
==========

to

==========
Add assertion to EmbeddedTestServer creation method

When the EmbeddedTestServer is created on UiThread, the test would
hang forever waiting for the server to start.

BUG=720155
==========

[the real yoland, 2017-05-09 23:36:53]

Added Crbug

[the real yoland, 2017-05-09 23:36:55]

The CQ bit was checked by yolandyan@chromium.org

[commit-bot: I haz the power, 2017-05-09 23:38:26]

CQ is trying da patch.

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

[jbudorick, 2017-05-09 23:38:34]

On 2017/05/09 23:36:53, the real yoland wrote:
> Added Crbug

thanks :) lgtm

[commit-bot: I haz the power, 2017-05-09 23:55:09]

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

[commit-bot: I haz the power, 2017-05-09 23:55:10]

Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[the real yoland, 2017-05-10 00:40:25]

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

[commit-bot: I haz the power, 2017-05-10 00:41:31]

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-05-10 02:44:02]

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

[commit-bot: I haz the power, 2017-05-10 02:44:02]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has
not started yet; builder either lacks capacity or does not exist (misspelled?))
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet;
builder either lacks capacity or does not exist (misspelled?))
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does
not exist (misspelled?))
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, build
has not started yet; builder either lacks capacity or does not exist
(misspelled?))
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
build has not started yet; builder either lacks capacity or does not exist
(misspelled?))
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does
not exist (misspelled?))
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does
not exist (misspelled?))
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does
not exist (misspelled?))
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, build
has not started yet; builder either lacks capacity or does not exist
(misspelled?))
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
build has not started yet; builder either lacks capacity or does not exist
(misspelled?))

[the real yoland, 2017-05-10 21:02:11]

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

[the real yoland, 2017-05-10 21:02:34]

The CQ bit was unchecked by yolandyan@chromium.org

[the real yoland, 2017-05-10 21:02:38]

The CQ bit was checked by yolandyan@chromium.org

[the real yoland, 2017-05-10 21:02:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
jbudorick@chromium.org, phajdan.jr@chromium.org
Link to the patchset: https://codereview.chromium.org/2860673005/#ps40001
(title: "Add deps to gn")

[commit-bot: I haz the power, 2017-05-10 21:05:16]

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-05-10 22:08:54]

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

[commit-bot: I haz the power, 2017-05-10 22:08:55]

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

[the real yoland, 2017-05-10 22:11:00]

yolandyan@chromium.org changed reviewers:
+ rsleevi@chromium.org

[the real yoland, 2017-05-10 22:11:01]

+rsleevi for net/android/BUILD.gn OWNER stamp

[the real yoland, 2017-05-15 19:01:20]

yolandyan@chromium.org changed reviewers:
+ pauljensen@chromium.org
- rsleevi@chromium.org

[the real yoland, 2017-05-15 19:01:21]

+pauljensen for OWNER stamp on src/net/android/BUILD.gn

[pauljensen, 2017-05-16 12:05:09]

The assert message and commit description could really use some
justification/motivation/explanation, like "...because it will hang waiting for
tasks to post to UI thread" or whatever the reason is.

[the real yoland, 2017-05-17 23:24:04]

Description was changed from

==========
Add assertion to EmbeddedTestServer creation method

When the EmbeddedTestServer is created on UiThread, the test would
hang forever waiting for the server to start.

BUG=720155
==========

to

==========
Add assertion to EmbeddedTestServer creation method

When the EmbeddedTestServer is created on UiThread, the test would
hang forever because it will hang waiting for tasks to post to UI thread

BUG=720155
==========

[the real yoland, 2017-05-17 23:25:09]

On 2017/05/16 at 12:05:09, pauljensen wrote:
> The assert message and commit description could really use some
justification/motivation/explanation, like "...because it will hang waiting for
tasks to post to UI thread" or whatever the reason is.

Done

[pauljensen, 2017-05-17 23:52:31]

lgtm

[the real yoland, 2017-05-18 00:03:22]

The CQ bit was checked by yolandyan@chromium.org

[the real yoland, 2017-05-18 00:03:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
jbudorick@chromium.org, phajdan.jr@chromium.org
Link to the patchset: https://codereview.chromium.org/2860673005/#ps60001
(title: "address comments")

[commit-bot: I haz the power, 2017-05-18 00:05:54]

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-05-18 02:19:10]

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

[commit-bot: I haz the power, 2017-05-18 02:19:11]

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

[the real yoland, 2017-05-19 18:17:08]

The CQ bit was checked by yolandyan@chromium.org

[commit-bot: I haz the power, 2017-05-19 18:17:50]

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-05-19 21:00:05]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1495217828613510,
"parent_rev": "50612fa00dfc8e79fe5bc92e2f235eee645739e9", "commit_rev":
"d9918bdea291b2648d5984382c3e55d01d8db4d3"}

[commit-bot: I haz the power, 2017-05-19 21:00:32]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1495217828613510,
"parent_rev": "8ac79a0a4825252d1eb801526ffc45f2ff4ba1a4", "commit_rev":
"508534a90769468c845e0f0438fbcb4f4fe38d51"}

[commit-bot: I haz the power, 2017-05-19 21:00:42]

Description was changed from

==========
Add assertion to EmbeddedTestServer creation method

When the EmbeddedTestServer is created on UiThread, the test would
hang forever because it will hang waiting for tasks to post to UI thread

BUG=720155
==========

to

==========
Add assertion to EmbeddedTestServer creation method

When the EmbeddedTestServer is created on UiThread, the test would
hang forever because it will hang waiting for tasks to post to UI thread

BUG=720155

Review-Url: https://codereview.chromium.org/2860673005
Cr-Commit-Position: refs/heads/master@{#473310}
Committed:
https://chromium.googlesource.com/chromium/src/+/508534a90769468c845e0f0438fb...
==========

[commit-bot: I haz the power, 2017-05-19 21:00:44]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/508534a90769468c845e0f0438fb...