487750 - test_plugin_clipping.xhtml fails randomly

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Josh Aas]

OS X 10.5.2 mozilla-central unit test:

*** 43911 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/generic/test/test_plugin_clipping.xhtml | Matching regions: expected {100,100,200,200}, got {100,100,100,100}
43911 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/generic/test/test_plugin_clipping.xhtml | Matching regions: expected {100,100,200,200}, got {100,100,100,100}
make: *** [mochitest-plain] Error 1
Unknown Error: command finished with exit code: 2

I can reproduce this locally on Mac OS X 10.5 and backing out the patches that landed before the failure doesn't help, I think this is rando-orange.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1240242088.1240251603.1652.gz
OS X 10.5.2 mozilla-central unit test on 2009/04/20 08:41:28  

(bug 488961 appears in the same log)

Comment 2

[Matthew Gregan [:kinetik]]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1241499951.1241508444.8098.gz
OS X 10.5.2 mozilla-central unit test on 2009/05/04 22:05:51

Comment 3

[Robert Strong (they/them - no direct email)]

OS X 10.5.2 mozilla-central unit test on 2009/05/21 20:31:18
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1242963078.1242968392.17192.gz&fulltext=1

Comment 4

[Daniel Holbert [:dholbert]]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1242996699.1243001825.10343.gz
OS X 10.5.2 mozilla-central unit test on 2009/05/22 05:51:39

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1243006918.1243012055.3029.gz
OS X 10.5.2 mozilla-central unit test on 2009/05/22 08:41:58

Comment 5

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1243187188.1243192976.7733.gz
OS X 10.5.2 mozilla-central unit test on 2009/05/24 10:46:28

Comment 6

[(no longer active)]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1243222356.1243231703.4915.gz
OS X 10.5.2 mozilla-central unit test on 2009/05/24 20:32:36

Comment 7

[David Bolter [:davidb] (NeedInfo me for attention)]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1243570748.1243579301.928.gz
OS X 10.5.2 mozilla-central unit test on 2009/05/28 21:19:08

Comment 8

[Matthew Gregan [:kinetik]]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1243982231.1243987298.24702.gz
OS X 10.5.2 mozilla-central unit test on 2009/06/02 15:37:11

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1244073721.1244079666.31347.gz

Comment 10

[Ben Newman (:bnewman) (:benjamn)]

I can get this to fail consistently by inserting

    for (var start = new Date; new Date - start < 1000; );

anywhere such that it runs before window.onload fires (i.e., before runTests runs).

When the unmodified test case fails intermittently, it's almost always on the first run of the test (fresh cache?), when the child window stays open for a longer-than-usual amount of time.  Adding the busy loop just exaggerates the delay.

Comment 11

[Ben Newman (:bnewman) (:benjamn)]

Even with the busy loop, I can rescue the test from failure if I introduce a slight delay beyond window.onload:

    function runTests() {
        if (!runTests.repeat)
            return setTimeout(runTests.repeat = runTests, 1000);
        ...
    }
    for (var start = new Date; new Date - start < 1000; );
    window.addEventListener("load", runTests, false);

Any ideas?

Comment 12

[Boris Zbarsky [:bzbarsky]]

So... the test has this code:

  var p = document.getElementById("p1");
  if (p.getClipRegionRectEdge(0, 0) == p.getClipRegionRectEdge(0, 2)) {
    // plugin hasn't been updated yet. wait.
    setTimeout(runTests2, 100);
    return;
  }

Why exactly is it needed?  Why is it enough to test "p1" only?  If I remove that code, all the tests fail, and the failure for the fifth one is precisely the one for this bug....  Does the plug-in run some sort of initialization code in a separate thread, and is this waiting for that to finish or something?

Comment 13

[Boris Zbarsky [:bzbarsky]]

OK, looks like we update the clip region in the plug-in at random times (from painting, off a timer, etc, etc).  See nsPluginInstanceOwner::FixUpPluginWindow.

Which means that we have no reasonable way to know when it's safe to read the clip region data from the plug-in on mac, as far as I can tell.  The above setTimeout seems to try to deal with it, but it looks like there can be cases where we've set up the right clip rect on the parent doc but not yet on the plug-in in the iframe.  If that happens, we get fail.  At least that's my best guess.

Comment 14

[Ben Newman (:bnewman) (:benjamn)]

Attachment: waiting for all plugins to update

In particular, this guarantees that the <embed id="p5"> inside of <iframe id="f1"> updates before we check its clipping region.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 381612 [details] [diff] [review]
waiting for all plugins to update

I assume this did help?

Comment 16

[Ben Newman (:bnewman) (:benjamn)]

(In reply to comment #15)
> (From update of attachment 381612 [details] [diff] [review])
> I assume this did help?

By all appearances, yes.  Can't reproduce anymore.

Comment 17

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Thanks Ben!

Comment 18

[Ben Newman (:bnewman) (:benjamn)]

(In reply to comment #17)
> Thanks Ben!

No problem :)

http://hg.mozilla.org/mozilla-central/rev/b53be8993cca

Comment 19

[Daniel Holbert [:dholbert]]

FWIW, this test just failed on Linux:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1251926815.1251935132.23792.gz
Linux mozilla-central unit test on 2009/09/02 14:26:55

> 46779 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/generic/test/test_plugin_clipping.xhtml | Matching regions for 'p1': expected {0,0,200,200}, got
> 46794 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/generic/test/test_plugin_clipping.xhtml | Matching regions for 'p4': expected {100,100,200,200}, got {100,100,200,166}
> 46799 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/generic/test/test_plugin_clipping.xhtml | Matching regions for 'p5': expected {100,100,200,200}, got

I'm not 100% sure if it's sporadic, since it's still the most recent cycle.  But it looks like it -- the previous tinderbox cycle didn't have this test-failure (though it did have a different known-sporadic failure), and the only change between these two cycles was a patch from blassey in Windows-only widget code.

Assuming the failure doesn't stick, I'll file a new bug on this.

Comment 20

[Daniel Holbert [:dholbert]]

Yup, the next cycle went green -- filed Bug 514321 on the new Linux sporadic failure in this test.