Rewrite HTML5 workers ui_tests to browser_tests. Compared to ui_tests, browser_tests are faster, le…

Rewrite HTML5 workers ui_tests to browser_tests. Compared to ui_tests, browser_tests are faster, less flaky and sharded. They're also portable to content_browsertests once we have it.

A bunch of these tests have bitrotted because they've been disabled for so long, so there are a number of fixes throughout (some were checked into WebKit). I haven't seen any flakiness when running them now dozens of times on my Win/Mac/Linux, so I'm renabling all disabled tests (36!). I'll keep a close eye on the tree and flakiness dashboard afterwards.

BUG=98716, 48148, 69881, 70861, 59786, 36800, 35965, 36555, 101996, 34996, 16934, 22599
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=129641

[jam, 2012-03-28 15:08:07]

dgrogan: layout_browsertest.*. I basically made it more like layout_ui_test.* to
support the more advanced needs of worker tests. In followup changes I'll
convert the rest of the ui_tests to browser_tests and then delete
layout_ui_test.

atwilson: the rest. I added the change to expected results here so you can see
them inline; I'll send those through WebKit now.

[jam, 2012-03-28 19:50:30]

hmm try servers still can't patch this in, even though it's applying for me when
I try on my mac/linux. I'm running these tests locally there.

Drew: you might want to look at patchset 4 to see the diff from ui to
browser_test

[jam, 2012-03-28 21:27:36]

btw this depends on this webkit patch: http://codereview.chromium.org/9802025/

[jam, 2012-03-28 21:35:59]

oops I meant to paste https://bugs.webkit.org/show_bug.cgi?id=82499

[jam, 2012-03-28 22:00:48]

missed another WebKit revision, so that'll be r112447

[dgrogan, 2012-03-28 22:08:13]

lgtm for
content/browser/in_process_webkit/indexed_db_layout_browsertest.cc
content/test/layout_browsertest.*

http://codereview.chromium.org/9802025/diff/3016/content/test/layout_browsert...
File content/test/layout_browsertest.cc (right):

http://codereview.chromium.org/9802025/diff/3016/content/test/layout_browsert...
content/test/layout_browsertest.cc:113: ASSERT_TRUE(file_util::CreateDirectory(
I think you can coalesce the CreateDirectory lines on 112 and 113.

I was hoping this would let you get rid of test_parent_dir_ altogether, but it
seems necessary to pass third_party/WebKit/LayoutTests/http/tests (or its
content/test/data counterpart) to LayoutTestHttpServer instead of
third_party/WebKit/LayoutTests/http/tests/workers.

[Andrew T Wilson (Slow), 2012-03-28 22:44:09]

http://codereview.chromium.org/9802025/diff/3016/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (left):

http://codereview.chromium.org/9802025/diff/3016/chrome/chrome_tests.gypi#old...
chrome/chrome_tests.gypi:838: '../content/worker/test/worker_uitest.cc',
Yay!

http://codereview.chromium.org/9802025/diff/3016/chrome/test/data/workers/ter...
File chrome/test/data/workers/terminate_queued_workers.html (left):

http://codereview.chromium.org/9802025/diff/3016/chrome/test/data/workers/ter...
chrome/test/data/workers/terminate_queued_workers.html:22: // limit.
Why did we get rid of this check? Was it race-y?

http://codereview.chromium.org/9802025/diff/3016/chrome/test/data/workers/wor...
File chrome/test/data/workers/worker_utils.js (right):

http://codereview.chromium.org/9802025/diff/3016/chrome/test/data/workers/wor...
chrome/test/data/workers/worker_utils.js:36: document.title = result;
This is clever.

http://codereview.chromium.org/9802025/diff/3016/content/browser/worker_host/...
File content/browser/worker_host/test/worker_browsertest.cc (right):

http://codereview.chromium.org/9802025/diff/3016/content/browser/worker_host/...
content/browser/worker_host/test/worker_browsertest.cc:43: //
http://crbug.com/101996 (started flaking with WebKit roll 98537:98582).
Are these tests no longer flaky now that we're using browser tests? Should we
modify the comments appropriately/mark bugs fixed? Or do we want to wait to see
what happens in practice when this hits the tree?

[jam, 2012-03-28 23:01:27]

http://codereview.chromium.org/9802025/diff/3016/chrome/test/data/workers/ter...
File chrome/test/data/workers/terminate_queued_workers.html (left):

http://codereview.chromium.org/9802025/diff/3016/chrome/test/data/workers/ter...
chrome/test/data/workers/terminate_queued_workers.html:22: // limit.
On 2012/03/28 22:44:09, Andrew T Wilson wrote:
> Why did we get rid of this check? Was it race-y?

now we can run more than 16 workers with isolates

http://codereview.chromium.org/9802025/diff/3016/chrome/test/data/workers/wor...
File chrome/test/data/workers/worker_utils.js (right):

http://codereview.chromium.org/9802025/diff/3016/chrome/test/data/workers/wor...
chrome/test/data/workers/worker_utils.js:36: document.title = result;
On 2012/03/28 22:44:09, Andrew T Wilson wrote:
> This is clever.

David came up with this for the indexed db tests :)

http://codereview.chromium.org/9802025/diff/3016/content/browser/worker_host/...
File content/browser/worker_host/test/worker_browsertest.cc (right):

http://codereview.chromium.org/9802025/diff/3016/content/browser/worker_host/...
content/browser/worker_host/test/worker_browsertest.cc:43: //
http://crbug.com/101996 (started flaking with WebKit roll 98537:98582).
On 2012/03/28 22:44:09, Andrew T Wilson wrote:
> Are these tests no longer flaky now that we're using browser tests? Should we
> modify the comments appropriately/mark bugs fixed? Or do we want to wait to
see
> what happens in practice when this hits the tree?

I haven't seen any flakiness in running these tests dozens of times on my
win/linux/mac machines. I hope that by switching them they'll be less flaky on
the bots as well. Of course I have no idea if that'll be the case, and
unfortunately this patch won't apply on the bots. So my plan is to check them in
with these comments so that if tests are flaky, the same bugs are reopened.

The one issue is on windows sometimes the http server won't start, but that's
separate from browser_tests vs ui_tests. I'm investigating that.

http://codereview.chromium.org/9802025/diff/3016/content/test/layout_browsert...
File content/test/layout_browsertest.cc (right):

http://codereview.chromium.org/9802025/diff/3016/content/test/layout_browsert...
content/test/layout_browsertest.cc:113: ASSERT_TRUE(file_util::CreateDirectory(
On 2012/03/28 22:08:13, dgrogan wrote:
> I think you can coalesce the CreateDirectory lines on 112 and 113.
> 
> I was hoping this would let you get rid of test_parent_dir_ altogether, but it
> seems necessary to pass third_party/WebKit/LayoutTests/http/tests (or its
> content/test/data counterpart) to LayoutTestHttpServer instead of
> third_party/WebKit/LayoutTests/http/tests/workers.

Done.

[Andrew T Wilson (Slow), 2012-03-28 23:08:52]

LGTM

[Andrew T Wilson (Slow), 2012-03-28 23:49:26]

http://codereview.chromium.org/9802025/diff/3016/chrome/test/data/workers/ter...
File chrome/test/data/workers/terminate_queued_workers.html (left):

http://codereview.chromium.org/9802025/diff/3016/chrome/test/data/workers/ter...
chrome/test/data/workers/terminate_queued_workers.html:22: // limit.
Note that now this test doesn't actually do much useful if we can't cause queued
workers. Doesn't hurt to keep it around, but it doesn't actually do what it
claims (queue up workers, then terminate them before they can get started).

[jam, 2012-03-29 01:03:41]

http://codereview.chromium.org/9802025/diff/3016/chrome/test/data/workers/ter...
File chrome/test/data/workers/terminate_queued_workers.html (left):

http://codereview.chromium.org/9802025/diff/3016/chrome/test/data/workers/ter...
chrome/test/data/workers/terminate_queued_workers.html:22: // limit.
On 2012/03/28 23:49:27, Andrew T Wilson wrote:
> Note that now this test doesn't actually do much useful if we can't cause
queued
> workers. Doesn't hurt to keep it around, but it doesn't actually do what it
> claims (queue up workers, then terminate them before they can get started).

good point. I tried to switch it to a SharedWorker but didn't see a way to
terminate it. I tried removing a reference to it but then I think this would
just be racy on when it dies, so it's not that useful (correct me if I'm wrong).
deleted the test

[Andrew T Wilson (Slow), 2012-03-29 01:10:17]

Yeah, shared workers don't have a terminate() function. So that's probably the
right call.