Bug 37048 - transitions/cancel-transition.html crashed on Leopard Debug Bot
Summary: transitions/cancel-transition.html crashed on Leopard Debug Bot
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: James Robinson
URL:
Keywords:
Depends on: 28461
Blocks:
  Show dependency treegraph
 
Reported: 2010-04-02 15:30 PDT by Eric Seidel (no email)
Modified: 2010-05-17 00:34 PDT (History)
3 users (show)

See Also:


Attachments
Patch (1.56 KB, patch)
2010-04-02 16:30 PDT, James Robinson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2010-04-02 15:30:12 PDT
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
Comment 1 Eric Seidel (no email) 2010-04-02 15:30:50 PDT
It's possible this could be related to bug 28461.
Comment 2 Simon Fraser (smfr) 2010-04-02 15:38:09 PDT
Ah, yes, it crashes if accelerated compositing is disabled.
Comment 3 Simon Fraser (smfr) 2010-04-02 15:47:01 PDT
Where "crash" is an assertion. We're computing a repaint rect on some animating transform whose value is time-dependent.
Comment 4 James Robinson 2010-04-02 16:00:26 PDT
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.
Comment 5 Simon Fraser (smfr) 2010-04-02 16:14:51 PDT
> 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).
Comment 6 James Robinson 2010-04-02 16:30:39 PDT
Created attachment 52463 [details]
Patch
Comment 7 Simon Fraser (smfr) 2010-04-02 16:37:18 PDT
Comment on attachment 52463 [details]
Patch

I'd like to see a //FIXME comment there that references this bug.
r=me
Comment 8 James Robinson 2010-04-02 16:52:37 PDT
Committed r57033: <http://trac.webkit.org/changeset/57033>
Comment 9 James Robinson 2010-04-02 16:54:10 PDT
The ASSERT() is gone but I don't feel that the underlying bug is fixed.
Comment 10 Eric Seidel (no email) 2010-04-06 23:46:59 PDT
Attachment 52463 [details] was posted by a committer and has review+, assigning to James Robinson for commit.
Comment 11 Eric Seidel (no email) 2010-05-17 00:34:56 PDT
Comment on attachment 52463 [details]
Patch

Obsoleting patch since this was committed.