1018381 - In B2G OOP "reftest-wait"/MozReftestInvalidate based reftests, the snapshots are usually missing some of the test's dynamic changes (e.g. a few px at the bottom or right)

Status: RESOLVED DUPLICATE of bug 1000722

(Core :: Graphics, defect)

Description

[Daniel Holbert [:dholbert]]

It looks to me like MozReftestInvalidate-based reftests mostly fail on B2G + OOP, at least based on the flexbox reftests that were marked as "fails-if(B2G&&browserIsRemote)" in bug 973835, in this cset:
 http://hg.mozilla.org/mozilla-central/rev/f05fbe6261ac#l27.2

However, it seems like they don't *reliably* fail -- occasionally, the tests' async JS runs in time for the reftest snapshot, and the test passes.

Since these reftests are all tagged with "reftest-wait", the harness is *supposed* to delay the reftest snapshot until the "reftest-wait" class is removed (and pending paints have been flushed).   However, that's apparently not reliably happening right now, in the B2G + OOP configuration.

(Not sure what component this belongs in. Putting in Graphics for now, since that's where bug 973835 lives, which this blocks.)

Comment 1

[Daniel Holbert [:dholbert]]

As I noted in bug 981477 comment 51: presumably, these test-failures mean either that invalidation is broken on B2G + OOP, or that the reftest harness's "reftest-wait" snapshot is happening too early in this configuration (before paints have been flushed or something).

Comment 2

[Andrew Halberstadt [:ahal]]

We are running most of the tests in layout/reftests/invalidation.. though maybe a quarter are marked as failures on OOP. Would be good to see a failing log with verbosity enabled.

Comment 3

[Daniel Holbert [:dholbert]]

Interesting. The first test in that folder, table-repaint-a.html, does look like a test that I'd expect to fail if my hypothesis were correct. (It has a MozReftestInvalidate-triggered dynamic change + reftest-wait-removal, and its initial rendering does not look like its reference case.)

I wonder why that test passes when the others fail. Maybe there's a bit of a raciness based on complexity of the test (& load-time), or something.

Comment 4

[Andrew Halberstadt [:ahal]]

Here's a try run with Vincent's manifest patch backed out and verbosity enabled:
https://tbpl.mozilla.org/?tree=Try&rev=f3f42ff3a5ee

Comment 5

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #3)
> Interesting. The first test in that folder, table-repaint-a.html, does look
> like a test that I'd expect to fail if my hypothesis were correct.

(meant to say: "but it's *not* currently marked as failing or random on b2g + OOP")

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: pruned reftest-8 failure log (showing 1-2px failing to be invalidated at bottom and/or right edge of changing region)

Looking at the output of the "reftest-8" failures from comment 4, it looks like most of the failures are from failing to repaint a 1-2px wide strip at the bottom and/or right edge of the dynamically-changed region in the testcase.

That not-repainted region is where the mismatch lies that causes the test failure.

I'm attaching the reftest output (suitable for reftest-analyzer) for a few tests where this is particularly obvious:
 - flexbox-invalidation-1.html
 - to-number-from-other-type-unthemed-1.html
 - from-number-to-other-type-unthemed-1.html
 - to-range-from-other-type-unthemed-1.html
 - from-range-to-other-type-unthemed-1.html
 - value-prop-unthemed.html
 - max-prop.html
 - setvalue-framereconstruction-1.html

This looks like a pretty serious graphics bug for this platform. (Might be some form of rounding error?)

(Note: I'm ignoring the font-inflation test failures, as those seem to be more legitimate test-failures and hence aren't interesting from the perspective of this bug here.)

Comment 7

[David Baron :dbaron:]

Does a try push with https://hg.mozilla.org/mozilla-central/rev/33d397b3b008 reverted (or perhaps a more sensible change to revert it, like "if (true)") make these failures go away?

In other words, is this the same bug as bug 1000722, and does bug 1000722 comment 3 help?

Comment 8

[Daniel Holbert [:dholbert]]

(In reply to David Baron [:dbaron] (busy May 31-June 15) (UTC+2) from comment #7)
> In other words, is this the same bug as bug 1000722, and does bug 1000722
> comment 3 help?

Possibly. The "failures around the edge of the box that moved" at the end of that comment sounds like what I'm seeing here. (in comment 6)

> Does a try push with https://hg.mozilla.org/mozilla-central/rev/33d397b3b008
> reverted (or perhaps a more sensible change to revert it, like "if (true)")
> make these failures go away?

Yes.

I did two Try runs, testing each of those options (one with the cset reverted, one with "if (true)"):
 * reverted: https://tbpl.mozilla.org/?tree=Try&rev=690f9bac4fa9
 * if(true): https://tbpl.mozilla.org/?tree=Try&rev=1a9e5643b29e

The latter Try run has completed R8 (the reftest batch that I've been focusing on here), and it only shows [unrelated] font-inflation failures.  The other tests mentioned in comment 6 all seem to pass.

Comment 9

[Daniel Holbert [:dholbert]]

Just to complete the half-reported results @ end of comment 8: both Try runs had the R8 test-failures gone (aside from the font-inflation ones).

Marking this as depending on bug 1000722, though perhaps we should just dupe it.

Comment 10

[u459114]

Take it

Comment 11

[u459114]

Hi Daniel,
Would you mind apply attachment 8442002 [details] [diff] [review] and do a try run for the fail ones again while available? Thanks

Comment 12

[u459114]

(In reply to Daniel Holbert [:dholbert] from comment #6)
> Created attachment 8432225 [details]
> pruned reftest-8 failure log (showing 1-2px failing to be invalidated at
> bottom and/or right edge of changing region)
> 
> Looking at the output of the "reftest-8" failures from comment 4, it looks
> like most of the failures are from failing to repaint a 1-2px wide strip at
> the bottom and/or right edge of the dynamically-changed region in the
> testcase.
> 
> That not-repainted region is where the mismatch lies that causes the test
> failure.
> 
> I'm attaching the reftest output (suitable for reftest-analyzer) for a few
> tests where this is particularly obvious:
>  - flexbox-invalidation-1.html
>  - to-number-from-other-type-unthemed-1.html
>  - from-number-to-other-type-unthemed-1.html
>  - to-range-from-other-type-unthemed-1.html
>  - from-range-to-other-type-unthemed-1.html
>  - value-prop-unthemed.html
>  - max-prop.html
>  - setvalue-framereconstruction-1.html
Confirmed fixed by attachment 8442612 [details] [diff] [review]. Dup it