Bug 37048 – transitions/cancel-transition.html crashed on Leopard Debug Bot

• Component: Tools / Tests
• Status: REOPENED

[Eric Seidel (no email)]

transitions/cancel-transition.html crashed on Leopard Debug Bot

http://build.webkit.org/results/Leopard%20Intel%20Debug%20(Tests)/r57025%20(12365)/transitions/cancel-transition-stderr.txt
ASSERTION FAILED: newOutlineBox == renderer()->outlineBoundsForRepaint(repaintContainer, 0)
(/Volumes/Big/WebKit-BuildSlave/leopard-intel-debug/build/WebCore/rendering/RenderLayer.cpp:321 void WebCore::RenderLayer::updateLayerPositions(unsigned int, WebCore::IntPoint*))

http://trac.webkit.org/browser/trunk/LayoutTests/transitions/cancel-transition.html

The ASSERT was added by James Robinson:
http://trac.webkit.org/browser/trunk/WebCore/rendering/RenderLayer.cpp?annotate=blame&rev=57000#L321

[Eric Seidel (no email)]

It's possible this could be related to bug 28461.

[Simon Fraser (smfr)]

Ah, yes, it crashes if accelerated compositing is disabled.

[Simon Fraser (smfr)]

Where "crash" is an assertion. We're computing a repaint rect on some animating transform whose value is time-dependent.

[James Robinson]

If the ASSERT fails it means that the outline box generated using the cached offset differs from the outline box using the slow path (walking up the container hierarchy to generate the offset).  If the offset is time-dependent than this makes sense, the cache offset might be correct at time X but incorrect at time X+1ms.

Should I take out the ASSERT()?  IMHO the cleanest solution is for all time-dependent values in animations to be fixed when painting a single frame.

[Simon Fraser (smfr)]

> Should I take out the ASSERT()?

For now, yes.

> IMHO the cleanest solution is for all time-dependent values in animations to be fixed when painting a single frame.

I think this is the correct solution, yes. Let's keep this bug open to do that (or file a new one).

[James Robinson]

Created attachment 52463 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 52463 [details]
Patch

I'd like to see a //FIXME comment there that references this bug.
r=me

[James Robinson]

Committed r57033: <http://trac.webkit.org/changeset/57033>

[James Robinson]

The ASSERT() is gone but I don't feel that the underlying bug is fixed.

[Eric Seidel (no email)]

Attachment 52463 [details] was posted by a committer and has review+, assigning to James Robinson for commit.

[Eric Seidel (no email)]

Comment on attachment 52463 [details]
Patch

Obsoleting patch since this was committed.