Bug 21011 – Need to clean up fix in PropertyWrapperGetter::equals a bit (and add test case)

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Chris Marrin]

The fix in changelist 36703 has one remaining problem. The test:

    if ((!a || !b) && b != a)

will fall through in the case where both a and b are null, causing a crash in the deref on the next line. There is no evidence that it is possible for this to happen, but I think the patch I have submitted is a bit safer. The patch also includes a testcase for the crash.

[Chris Marrin]

Created attachment 23681 [details]
Patch, including LayoutTest file

[Eric Seidel (no email)]

Comment on attachment 23681 [details]
Patch, including LayoutTest file

if (!a && !b || a == b) is redundant.

if (!a && !b)
should be sufficient.

Another way to write this woudl be:

if (!a || !b)
    return (a == b);

but I think the two ifs as you've written it is more clear (After removing the redundancy).

[Simon Fraser (smfr)]

Committed r37076
	M	WebCore/ChangeLog
	M	WebCore/page/animation/CompositeAnimation.cpp
	M	WebCore/page/animation/AnimationBase.cpp
	M	LayoutTests/ChangeLog
	A	LayoutTests/transitions/override-transition-crash-expected.txt
	A	LayoutTests/transitions/override-transition-crash.html
r37076 = ff91302a8b84e30eec65022e68b48301e3dac305 (trunk)