Bug 131480

Summary: REGRESSION (r166860): ASSERTION FAILED: !isCalculated() on fast/css/image-set-value-not-removed-crash.html
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: CSSAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, darin, esprehn+autocc, glenn, gyuyoung.kim, kling, macpherson, menard, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch kling: review+, kling: commit-queue-

Description Alexey Proskuryakov 2014-04-09 23:37:04 PDT
Starting with <http://trac.webkit.org/r166860>, debug WK1 bots are hitting an assertion almost every time:

ASSERTION FAILED: !isCalculated()
/Volumes/Data/slave/mountainlion-debug/build/Source/WebCore/platform/Length.h(246) : float WebCore::Length::value() const
1   0x10bccc0c0 WTFCrash
2   0x10d71b619 WebCore::Length::value() const
3   0x10d9cae02 WebCore::valueForNinePieceImageSlice(WebCore::NinePieceImage const&)
4   0x10d9c37d6 WebCore::ComputedStyleExtractor::propertyValue(WebCore::CSSPropertyID, WebCore::EUpdateLayout) const
5   0x10d9cd7fe WebCore::ComputedStyleExtractor::copyPropertiesInSet(WebCore::CSSPropertyID const*, unsigned int) const
6   0x10d9c6ef8 WebCore::ComputedStyleExtractor::copyProperties() const
7   0x10dd3da81 WebCore::copyPropertiesFromComputedStyle(WebCore::ComputedStyleExtractor&, WebCore::EditingStyle::PropertiesToInclude)
8   0x10dd3d345 WebCore::EditingStyle::init(WebCore::Node*, WebCore::EditingStyle::PropertiesToInclude)
9   0x10dd3d23b WebCore::EditingStyle::EditingStyle(WebCore::Node*, WebCore::EditingStyle::PropertiesToInclude)
10  0x10dd3d1d3 WebCore::EditingStyle::EditingStyle(WebCore::Node*, WebCore::EditingStyle::PropertiesToInclude)
11  0x10d7cc8ff WebCore::EditingStyle::create(WebCore::Node*, WebCore::EditingStyle::PropertiesToInclude)
12  0x10dd3f135 WebCore::EditingStyle::styleAtSelectionStart(WebCore::VisibleSelection const&, bool)
13  0x10dd500c8 WebCore::Editor::selectionStartHasStyle(WebCore::CSSPropertyID, WTF::String const&) const
14  0x10dd69d6d WebCore::executeToggleStyle(WebCore::Frame&, WebCore::EditorCommandSource, WebCore::EditAction, WebCore::CSSPropertyID, char const*, char const*)
Comment 1 Alexey Proskuryakov 2014-04-09 23:40:38 PDT
Skipped the test in debug builds in <http://trac.webkit.org/r167070>.
Comment 2 Darin Adler 2014-04-10 16:09:01 PDT
What’s new here is the assertion. The bug isn’t new. I’ll fix this.
Comment 3 Darin Adler 2014-04-11 10:41:47 PDT
I wonder why it’s *almost* every time instead of every time.
Comment 4 Darin Adler 2014-04-11 10:42:49 PDT
It’s bizarre; this test case does not involve calculated values. I wonder how a calculated value gets in there.
Comment 5 Alexey Proskuryakov 2014-04-11 10:49:20 PDT
FWIW, it's reproducible for me locally.
Comment 6 Alexey Proskuryakov 2014-04-11 10:50:56 PDT
Looking at the flakiness dashboard results more closely, I think that it's actually every time - some of the runs are shown as timeouts, but I'm pretty certain that it's a tools bug, and we just fail to detect a crash that happened.
Comment 7 Darin Adler 2014-04-12 10:07:37 PDT
The calculation here is created as part of animation. We are animating from the value 26829% to the value 2, so during the animation the current value of the property is a combination of both. The CSSComputedStyle code doesn’t know how to serialize something that is neither a percentage nor a number. I imagine there are a lot more cases like this in computed style.
Comment 8 Darin Adler 2014-04-12 10:08:04 PDT
Because the code depends on the timing of the animation, I imagine it won’t always crash the same way every time.
Comment 9 Darin Adler 2014-04-12 12:02:42 PDT
Created attachment 229209 [details]
Patch
Comment 10 Andreas Kling 2014-04-12 22:05:08 PDT
Comment on attachment 229209 [details]
Patch

r=me, but you should also unskip the test.
Comment 11 Darin Adler 2014-04-13 01:06:25 PDT
Committed r167192: <http://trac.webkit.org/changeset/167192>
Comment 12 Brent Fulgham 2015-12-08 13:57:36 PST
Part of the fix for CVE-2014-4410.