379922 - Canvas frame's invalidation no longer correct

Status: RESOLVED FIXED

(Core :: Web Painting, defect)

Description

[Boris Zbarsky [:bzbarsky]]

The block in nsCanvasFrame::Reflow which does:

589     // If the child frame was just inserted, then we're responsible for making sure
590     // it repaints
591     if (kidDirty) {
592       // But we have a new child, which will affect our background, so
593       // invalidate our whole rect.
594       // Note: Even though we request to be sized to our child's size, our
595       // scroll frame ensures that we are always the size of the viewport.
596       // Also note: GetPosition() on a CanvasFrame is always going to return
597       // (0, 0). We only want to invalidate GetRect() since GetOverflowRect()
598       // could also include overflow to our top and left (out of the viewport)
599       // which doesn't need to be painted.
600       Invalidate(GetRect(), PR_FALSE);
601     }

is no longer correct.  In particular, the canvas frame is NOT the size of the viewport anymore.  Not sure when this changed.

This is most visible with the testcase in bug 71191 on Linux.  When I load it, the canvas background doesn't paint green until I move the mouse out of the window...

Comment 1

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

Changed in bug 192767, maybe?

Comment 2

[Boris Zbarsky [:bzbarsky]]

That's possible.. it was some scrollframe change that changed this, yes.

Comment 3

[Eli Friedman]

We shouldn't be depending on the canvas frame for background invalidation anyway... InvalidateCanvasIfNeeded is supposed to take care of that.  I think it's buggy, though.

Comment 4

[Eli Friedman]

Okay, so it looks like the canvas frame's invalidation is the only place we handle fixing the background after frame reconstruction.  So I guess we should fix it.

Comment 5

[Eli Friedman]

Attachment: Patch

Comment 6

[Eli Friedman]

Note that we don't have any testing framework for catching invalidation bugs.

Comment 7

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

Comment on attachment 269921 [details] [diff] [review]
Patch

r+sr=dbaron

Comment 8

[Eli Friedman]

Checked in.