STR: 1. from <obj-path>/_tests/testing/mochitest, run python runtests.py --test-path=modules/plugin/test/test_bug539565-2.html and wait for the test to finish. 2. Scroll to the right and slowly back to the left using the horizontal scrollbar thumb. This seems to be a regression from bug 564991, but reproducing is a bit temperamental (seems to depend on window size) so perhaps there ways to produce this before that bug landed.

Note that as well as unpainted lines that cover the entire embed, there are also unpainted lines that are only in the blue div.

Karl, are you still seeing this?

(In reply to comment #2) > Karl, are you still seeing this? Still seeing this with 9d6b04fbec36. The unpainted lines now seem to show uninitialized pixels.

I can

That test is pretty broken for me; scrolling around just fails to repaint the area where the plugin was.

-- nsDisplayTransform::GetBounds/IsOpaque are wrong. IsOpaque returns true when the list reports as being opaque, but that doesn't mean the whole overflow area for the frame is opaque. The overflow area for the frame might not be covered by stuff in the list (even just because we had to take the union of rectangles to get the overflow area). So, compute the transform bounds as the transform of the list bounds. -- PrintDisplayListTo needs to be more careful when printing the stored list for an nsDisplayTransform. We need to check whether the list DidComputeVisibility. -- nsDisplayPlugin::GetBounds is wrong for plugins with layers, in two ways: -- When using a plugin with a layer, we need to set the size of the bounds to the size of the layer (as well as adjusting the offset to center the plugin layer within the plugin rect). -- If the plugin layer ImageContainer has no image, the plugin hasn't painted yet. We should leave the bounds as the size of the plugin content area instead of setting them to the layer image size (which will be 0,0 since we have no image). If we allow the size of the plugin content area to be set to 0,0, BuildLayer will not actually be called and the plugin never gets around to displaying properly.

Benjamin, you might want this patch for your Windows work.

Comment on attachment 478632 [details] [diff] [review] fix hmm, the last paragraph of comment #6 may be wrong. We do need to ensure NotifyPainted is triggered, but it's not a good idea to lie about the size of what we're going to paint. Probably we should move the call to NotifyPainted to nsObjectFrame::BuildDisplayList.

Addresses previous comment. When we have no image, we allow the plugin display item to be 0x0 (it will actually be removed during ComputeVisibility and BuildLayer will never be called). But we move the code to call NotifyPaintWaiter and NotifyPainted from BuildLayer to BuildDisplayList, so it gets called anyway.

Comment on attachment 478634 [details] [diff] [review] fix v2 this patch breaking == 580160-1.html 580160-1-ref.html reftest

Comment on attachment 478634 [details] [diff] [review] fix v2 r=me, of course you'll have to fix the reftest fail.

Comment on attachment 478634 [details] [diff] [review] fix v2 r-, due to reftest failure... btw roc why do you ask superreview for this patch?

The 580160-1.html testcase works for me if I load it and call doTest() manually. I presume we're taking the snapshot before the plugin has resized and repainted its layer.

My patch here actually causes a test failure in layout/reftests/transform/compound-1-ref.html (maybe in combination with other patches in my queue). The problem actually already exists and is nontrivial. In FrameLayerBuilder in a couple of places we do nsIntRect itemVisibleRect = aItem->GetVisibleRect().ToNearestPixels(appUnitsPerDevPixel); This code is assuming that when the item draws, whatever it draws will be snapped to pixel boundaries. But this assumption is overoptimistic for a couple of reasons: -- Some content, such as an element transformed by rotation, simply doesn't snap its drawing. -- If the context has a tricky transform on it when we come to draw the content, we might not snap. Using ToOutsidePixels here would be the conservative approach and it would give correct results. However it would mean that for content that *does* snap, but is not already aligned at pixel boundaries, the visible rect is bigger than the area that actually gets drawn, so a layer containing only that content could no longer be treated as opaque. That could be bad. In BasicLayers we have a couple of places where we do gfxUtils::ClipToRegionSnapped that have related issues.

Filed bug 615794 on that.

The 580160-1 reftest failure is an intermittent failure on trunk already. Bug 615417 has my explanation of the failure (or at least, one cause of the failure).

Even with bug 615417 fixed I'm still seeing failures in 580160-1.html with my patch stack. The problem seems to be in the reftest harness; we're not correctly waiting to get the correct-size plugin image before taking the snapshot. The code in reftest.js controlling how a test waits for completion has become a mess. There are really three different conditions we can be waiting for: 'reftest-wait' to be removed from the root element's class, all paints to be flushed, and all plugin "explicit waiters" to be removed. Right now we wait for one of those conditions at a time and transition from waiting for one to waiting for another. A simpler and more robust design would be to keep checking all three conditions at once in one place and stop when they're all complete simultaneously. We register listeners for a change in each condition that all just call back to that single CheckTestEnd function.

This is simpler, and I hope clearer, and seems to fix the failures I've been seeing.

Comment on attachment 495465 [details] [diff] [review] Part 2: fix harness No, this needs some work.

I'll move the reftest harness changes to a new bug --- bug 617152

OK, turns out the test here fails on D3D10 because of bug 617242 (fix in bug).

Comment on attachment 478634 [details] [diff] [review] fix v2 Patch a bit outdated (some parts are landed)... do we still need it? I'm ok with this patch, if it is pass plugins reftests

Some of this landed in bug 617512. I think the rest is ready to land.