563899 - ts harness calls document.write from an onload handler causing the document.write to be counted into shutdown time

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

Attachment: Remove the bad line

Steps to reproduce:
 1) Put a printf in nsHtmlParser::Parse(const nsAString&, void*, const nsACString&, PRBool, nsDTDMode) if html5.enable=true or nsParser::Parse(const nsAString&, void*, const nsACString&, PRBool, nsDTDMode) if html5.enable=false.
 2) Put a printf in nsAppStartup::Quit
 3) Build the app
 4) Run the ts benchmark with the new build.

Actual results:
nsIParser::Parse(const nsAString&, void*, const nsACString&, PRBool, nsDTDMode) is called after nsAppStartup::Quit.

Expected results:
Expected no call to nsIParser::Parse(const nsAString&, void*, const nsACString&, PRBool, nsDTDMode) after nsAppStartup::Quit.

See also the dev-tree-management thread at http://groups.google.com/group/mozilla.dev.tree-management/browse_thread/thread/4b3ed61fdb8287ac I suspect the document.write() after quit has something to do with the ts shutdown regression seen when the HTML5 parser was turned on by default.

Comment 1

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

s/nsHtmlParser/nsHtml5Parser/

Comment 2

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 443585 [details] [diff] [review]
Remove the bad line

sorry for the delay. looks good.

Comment 3

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

Thanks. I think I don't have access to land this where it needs to land.

Comment 4

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Comment on attachment 443585 [details] [diff] [review]
Remove the bad line

Found in triage, just need to land this r+'d patch.

Comment 5

[Chris AtLee [:catlee]]

Can the Talos folks test/land this patch?

Comment 6

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

I don't quite understand how the document.write is touching the shutdown time since the timestamp is taken after the document.write.

Comment 7

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

(In reply to comment #6)
> I don't quite understand how the document.write is touching the shutdown time
> since the timestamp is taken after the document.write.

It seems to me the time taken after the document.write is the end time stamp. Not sure if that get counted towards shutdown exactly, but the use of document.write here is very bogus in any case.

Comment 8

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

I believe that there is an alternate solution offered here:

bug 648749#c8

Which will let us keep the visual output for when doing manual debugging.

Comment 9

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

bug 648749 has been deployed, and I believe that it fixes this bug as well.

We now use:

    document.body.textContent = 'Startup time = ' + startupTime + ' ms';

and shutdown is now triggered asynchronously. 

Going to mark this fixed, please re-open if I'm missing something tricky.

Comment 10

[Ed Morley [:emorley]]

Removing checkin-needed due to comment 9.