Closed Bug 589396 Opened 14 years ago Closed 12 years ago

Tgfx is broken and 130078 exposes it

Categories

(Testing :: Talos, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: tnikkel, Unassigned)

References

Details

Attachments

(1 file)

Tgfx_nochrome absolutely blows up with bug 130078, see the links in bug 588663. There are 10000%+ increases. The pageloader extension in pageloader.js picks a window with this code in runRenderTest if (browserWindow) win = content.contentWindow; else win = window; and then calls nsIDOMWindowUtils::Redraw 500 times on win. For Tgfx_chrome the if condition is always true, for Tgfx_nochrome it is always false due to how the test is setup. In the Tgfx_chrome case win contains the pages we are measuring the performance of (ie borders-dashed.html). In the Tgfx_nochrome case win is the parent chrome window (ie pageloader.xul) which contains a child document that loads the pages we are testing. Now, all nsIDOMWindowUtils::Redraw does is get the root frame of the window and call InvalidateWithFlags(r, nsIFrame::INVALIDATE_IMMEDIATE) on it where r is the size of the root frame. nsIFrame::InvalidateWithFlags checks if painting is suppressed in the presshell, and if it is returns and doesn't do anything. The 500 nsIDOMWindowUtils::Redraw calls happen off of the load event for the child content document. DocumentViewerImpl::LoadComplete is where the load event is dispatched, and it unsuppresses painting after it dispatches the load event. So in Tgfx_chrome case the 500 nsIDOMWindowUtils::Redraw calls happen off of the load event of the child content document, and are targeted at the child content document. So when they happen painting is suppressed and they all do nothing. In the Tgfx_nochrome case the 500 nsIDOMWindowUtils::Redraw calls happen off of the load event of the child content document, but are targeted at the root chrome document. The root chrome document has not just loaded so its painting is not suppressed. But there is nothing to paint in the root chrome document, so it is still quick. This explains why Tgfx_nochrome takes longer than Tgfx_chrome. When we add the patches from bug 130078 the view managers for the chrome and content documents are linked, so the paint events targeted at the root chrome document also paint the child content document (we don't currently suppress painting when entering a subdocument that has painting suppressed). And this takes a lot longer since it is actually doing some work.
Clearly we need to fix Tgfx, but I don't think this should block 130078 landing.
For a quick "fix" we can just restore the broken behaviour so the numbers stay about the same by making the window we send the Redraw's to be the content window in the nochrome case as well.
Is this a version of bug 493050 ?
Closely related. If we don't change anything then bug 130078 is going to massively regress the Tgfx_nochrome numbers. Bug 130078 needs to land soon. So we need to do something about that. We can either disable the test. Or we can make a small change to the testing harness so that it continues to be bogus, but gives approximately the same numbers. We can decide what we want to do with Tgfx longer term in bug 493050, but we need a quick solution here.
I think spending any time at all making Tgfx results look sane is time wasted. Send an email to dev.tree-management saying "please ignore this apparent Tgfx regression" and move on. We can work out what to do with Tgfx in bug 493050.
I would also suggest that we should disable tgfx till it actually does something useful - right now we are just wasting cpu time on running it for every branch on every OS for every push.
Attached patch Disable tgfx (deleted) — Splinter Review
Attachment #471348 - Flags: review?(anodelman)
Attachment #471348 - Flags: review?(anodelman) → review+
Comment on attachment 471348 [details] [diff] [review] Disable tgfx Checked in as changeset: 2932:c0b16bcdc30b, and masters reconfigured.
Should we track fixing Tgfx in bug 493050 and mark this resolved?
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: