541040 - The v8 test suite depends on timing and buffer boundaries

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Henri Sivonen (:hsivonen)]

The v8 talos suite doesn't finish with html5.enable=true

qm-pubuntu-try16: 
		Started Wed, 20 Jan 2010 08:21:35
Running test v8: 
		Started Wed, 20 Jan 2010 08:21:35
	Screen width/height:1280/1024
	colorDepth:24
	Browser inner width/height: 1024/620
	Browser outer width/height: 1024/768
	Screen width/height:1280/1024
	colorDepth:24
	Browser inner width/height: 1024/620
	Browser outer width/height: 1024/768
NOISE: Cycle 1: loaded file:///home/mozqa/talos-slave/talos-data/talos/page_load_test/v8/run-richards.html (next: file:///home/mozqa/talos-slave/talos-data/talos/page_load_test/v8/run-deltablue.html)
NOISE: 
NOISE: __FAILbrowser frozen__FAIL
NOISE: Cycle 1: loaded file:///home/mozqa/talos-slave/talos-data/talos/page_load_test/v8/run-richards.html (next: file:///home/mozqa/talos-slave/talos-data/talos/page_load_test/v8/run-deltablue.html)
NOISE: 
NOISE: __FAILbrowser frozen__FAIL
Failed v8: 
		Stopped Wed, 20 Jan 2010 08:32:22
FAIL: Busted: v8
FAIL: browser frozen
Completed test v8: 
		Stopped Wed, 20 Jan 2010 08:32:22
RETURN: cycle time: 00:10:46<br>
qm-pubuntu-try16: 
		Stopped Wed, 20 Jan 2010 08:32:22
Sending results: 
		Started Wed, 20 Jan 2010 08:32:22
RETURN: new graph links
RETURN:<br>
RETURN:<p style="font-size:smaller;">Details:<br>|</p>
Completed sending results: 
		Stopped Wed, 20 Jan 2010 08:32:22
program finished with exit code 0

Comment 1

[Henri Sivonen (:hsivonen)]

The test suite does a bogus thing here. It assumes that if it calls setTimeout from an inline script, the page will be parsed to completion without spinning the event loop so the timeout doesn't have a chance to fire before the page has been parsed.

The planned fix for bug 543458 will most likely hide this problem.

However, to make the test suite truly robust, instead of having <div id="msg" /> after the scripts, there should instead be a <div id="msg"></div> above the scripts or, alternatively, the scripts should start from the onload handler.

Comment 2

[Henri Sivonen (:hsivonen)]

The fix for bug 543458 makes this no longer a blocker for turning on the HTML5 parser on by default, but I think it would still be proper to fix the test cases, too.

Comment 3

[Henri Sivonen (:hsivonen)]

Attachment: Move the message div above the script and make it have a proper end tag

Comment 4

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 431870 [details] [diff] [review]
Move the message div above the script and make it have a proper end tag

looks good.

Comment 5

[Nick Thomas [:nthomas] (UTC+12)]

Can we upstream this change ? Seems likely that we'll lose it again if we take an update to v8.

Comment 6

[Henri Sivonen (:hsivonen)]

(In reply to comment #4)
> (From update of attachment 431870 [details] [diff] [review])
> looks good.

Marking checkin-needed, since I think I don't have access to land the patch where it needs to go.

(In reply to comment #5)
> Can we upstream this change ? Seems likely that we'll lose it again if we take
> an update to v8.

Looking at the upstream code, it seems that they have very different HTML around the .js files than we have. 

lsblakk, Do the .js files get updated from upstream but the HTML harness-files are Mozilla-only?

Comment 7

[[e-mail] (:bhearsum)]

I'll care of landing this whenever the next downtime is

Comment 8

[[e-mail] (:bhearsum)]

Comment on attachment 431870 [details] [diff] [review]
Move the message div above the script and make it have a proper end tag

Checking in page_load_test/v8/run-crypto.html;
/cvsroot/mozilla/testing/performance/talos/page_load_test/v8/run-crypto.html,v  <--  run-crypto.html
new revision: 1.2; previous revision: 1.1
done
Checking in page_load_test/v8/run-deltablue.html;
/cvsroot/mozilla/testing/performance/talos/page_load_test/v8/run-deltablue.html,v  <--  run-deltablue.html
new revision: 1.2; previous revision: 1.1
done
Checking in page_load_test/v8/run-earley-boyer.html;
/cvsroot/mozilla/testing/performance/talos/page_load_test/v8/run-earley-boyer.html,v  <--  run-earley-boyer.html
new revision: 1.2; previous revision: 1.1
done
Checking in page_load_test/v8/run-raytrace.html;
/cvsroot/mozilla/testing/performance/talos/page_load_test/v8/run-raytrace.html,v  <--  run-raytrace.html
new revision: 1.2; previous revision: 1.1
done
Checking in page_load_test/v8/run-regexp.html;
/cvsroot/mozilla/testing/performance/talos/page_load_test/v8/run-regexp.html,v  <--  run-regexp.html
new revision: 1.2; previous revision: 1.1
done
Checking in page_load_test/v8/run-richards.html;
/cvsroot/mozilla/testing/performance/talos/page_load_test/v8/run-richards.html,v  <--  run-richards.html
new revision: 1.2; previous revision: 1.1
done
Checking in page_load_test/v8/run-splay.html;
/cvsroot/mozilla/testing/performance/talos/page_load_test/v8/run-splay.html,v  <--  run-splay.html
new revision: 1.2; previous revision: 1.1
done

Comment 9

[[e-mail] (:bhearsum)]

Landed this, assuming it is fixed.

Comment 10

[alice nodelman [:alice] [:anode]]

Removing checkin-needed tag.

Comment 11

[alice nodelman [:alice] [:anode]]

Can we replace this v8 test with the v8 tests that are a part of the dromaeo suites that we currently run?