Open Bug 481740 Opened 11 years ago Updated 1 year ago

test framework for interruptible reflow - trigger a reflow interrupt on textnode containing vertical tab

Categories

(Core :: General, enhancement)

enhancement
Not set

Tracking

()

People

(Reporter: njain, Unassigned)

References

Details

Attachments

(1 file, 4 obsolete files)

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1; GTB5; .NET CLR 1.1.4322; InfoPath.2; .NET CLR 2.0.50727)
Build Identifier: 

Need a setup to test the interruptible reflow code as well as possible. One approach is to trigger an interrupt when there is a textnode with a particular content (e.g. a textnode containing only a vertical tab) when certain environment variables are set.

This framework would allow us to easily write reftests containing a textnode with a vertical tab, that would trigger reflow interrupts at particular places at particular times.

Reproducible: Always
Created a patch which triggers a reflow interrupt for textnodes containg a vertical tab. This patch adds additional functionality for testing puposes to the patch from bug 67752.

The environment variables needed to trigger an interrupt are:
 export GECKO_REFLOW_INTERRUPT_MODE=vtab
 export GECKO_REFLOW_INTERRUPT_CHECKS_TO_SKIP=1
 
Sample test file to trigger an interrupt:
<!DOCTYPE html>
<html>
<body>
<div><pre>&#11;Text</pre></div>
</body>
</html>
Depends on: ireflow
Neeti, I just updated the patch in bug 67752.  Would you mind making an interdiff of your patch against that one?
Assignee: nobody → njain
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attaching interdiff of test framework patch against the patch in bug 67752 submitted on 03-29-09
Attachment #365753 - Attachment is obsolete: true
Updated interdiff patch against bug 67752(04-22-2009) and added tests to trigger interrupts
Attachment #370443 - Attachment is obsolete: true
Looks like these reftests never made it into the tree.  Are they valid and can they be checked in?
I don't know that anyone's checked that:

1)  The tests pass
2)  The tests actually test something

I suspect that without more changes elsewhere they probably don't.

The patch also enables the interruptible reflow debug spew, which is probably undesirable.
Attached patch updated patch (obsolete) — Splinter Review
I updated this patch so that it applies cleanly and compiles against mozilla-central.  Still need to investigate whether it does anything.
Attachment #375072 - Attachment is obsolete: true
Boris:  the tests all produce output similar to this when hitting the vtab:

*** Entering reflow event (time=1255647089021000)
*** DETECTED pending interrupt (time=1255647089021000)
mFramesToDirty.Count() == 6
*** Returning from reflow event (time=1255647089037000)

Does this qualify as "testing something"?  They also all pass.  The caveat is that these must all be set:

GECKO_REFLOW_INTERRUPT_MODE=vtab
GECKO_REFLOW_INTERRUPT_CHECKS_TO_SKIP=0
GECKO_REFLOW_MIN_NOINTERRUPT_DURATION=0

So, I would have to add something to reftest.js to allow these to be set from within a test.

Also, I had to make a small change in nsPresContext::CheckForInterrupt(), from 

  mHasPendingInterrupt =
    TimeStamp::Now() - mReflowStartTime > sInterruptTimeout &&
    HavePendingInputEvent() &&
    !IsChrome();

to

  mHasPendingInterrupt =
    TimeStamp::Now() - mReflowStartTime >= sInterruptTimeout &&
    HavePendingInputEvent() &&
    !IsChrome();

since for some tests, TimeStamp::Now() - mReflowStartTime = 0, when the vtab is hit, at least on my machine.
> Does this qualify as "testing something"?

Yeah; it means we interrupted the reflow.  We probably don't want to land the part of this patch that enables those printfs... 

I'd been using GECKO_REFLOW_MIN_NOINTERRUPT_DURATION=-1 to disable the timeout check.

As far as setting the env vars from reftest.js, we'd need to make prescontext notice the env var change or something.  I suppose we can hook up an observer notification to reset the prescontext state.  Alternately, we'd need to run these reftests as a separate test run from normal reftests.
Attached patch patch v3Splinter Review
I ended up not setting env vars from reftest.js, since doing so using nsIEnvironment causes a deliberate leak which is no good for tinderbox (see http://mxr.mozilla.org/mozilla-central/source/xpcom/threads/nsEnvironment.cpp#135).  So instead I'm just using an observer notification to tell nsPresContext to set/reset the ireflow parameters.  I'm sure there are other ways to do this; if you want something else, let me know.
Attachment #405550 - Attachment is obsolete: true
Attachment #416994 - Flags: review?(bzbarsky)
Attachment #416994 - Attachment description: path v3 → patch v3
Assignee: njain → jgriffin
Status: NEW → ASSIGNED
Attachment #416994 - Flags: review?(bzbarsky)
Assignee: jgriffin → nobody
No assignee, updating the status.
Status: ASSIGNED → NEW
No assignee, updating the status.
No assignee, updating the status.
No assignee, updating the status.
You need to log in before you can comment on or make changes to this bug.