Bug 141645

Summary: ASSERTION FAILED: !length.isUndefined() in WebCore::GridLength::GridLength
Product: WebKit Reporter: Renata Hodovan <rhodovan.u-szeged>
Component: Layout and RenderingAssignee: Sergio Villar Senin <svillar>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, cdumez, darin, jfernandez, kling, rego, simon.fraser, svillar
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 116980    
Attachments:
Description Flags
Test case
none
Patch cdumez: review+

Renata Hodovan
Reported 2015-02-16 07:16:29 PST
Created attachment 246651 [details] Test case Load this with debug WK: <!DOCTYPE html> <style> title { -webkit-grid: dense row 755ex; } </style> <title> Backtrace: ASSERTION FAILED: !length.isUndefined() ../../Source/WebCore/rendering/style/GridLength.h(51) : WebCore::GridLength::GridLength(const WebCore::Length&) Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7fff94ed1700 (LWP 10281)] 0x00007fffed74709f in WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:321 321 *(int *)(uintptr_t)0xbbadbeef = 0; #0 0x00007fffed74709f in WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:321 #1 0x00007ffff2d08544 in WebCore::GridLength::GridLength (this=0x7fffffff5390, length=...) at ../../Source/WebCore/rendering/style/GridLength.h:51 #2 0x00007ffff3e01f9c in WebCore::StyleBuilderConverter::createGridTrackBreadth (primitiveValue=..., styleResolver=...) at ../../Source/WebCore/css/StyleBuilderConverter.h:769 #3 0x00007ffff3e02062 in WebCore::StyleBuilderConverter::createGridTrackSize (value=..., styleResolver=...) at ../../Source/WebCore/css/StyleBuilderConverter.h:775 #4 0x00007ffff3e02d21 in WebCore::StyleBuilderConverter::convertGridTrackSize (styleResolver=..., value=...) at ../../Source/WebCore/css/StyleBuilderConverter.h:886 #5 0x00007ffff3e1fa22 in WebCore::StyleBuilderFunctions::applyValueWebkitGridAutoColumns (styleResolver=..., value=...) at DerivedSources/WebCore/StyleBuilder.cpp:3902 #6 0x00007ffff3df399d in WebCore::StyleBuilder::applyProperty (property=WebCore::CSSPropertyWebkitGridAutoColumns, styleResolver=..., value=..., isInitial=false, isInherit=false) at DerivedSources/WebCore/StyleBuilder.cpp:7612 #7 0x00007ffff2e358f1 in WebCore::StyleResolver::applyProperty (this=0x7fffd97e5750, id=WebCore::CSSPropertyWebkitGridAutoColumns, value=0x7fffd9fc1138) at ../../Source/WebCore/css/StyleResolver.cpp:1958 #8 0x00007ffff2e38de5 in WebCore::StyleResolver::CascadedProperties::Property::apply (this=0x7fffffffb3a0, resolver=...) at ../../Source/WebCore/css/StyleResolver.cpp:2684 #9 0x00007ffff2e38f5a in WebCore::StyleResolver::applyCascadedProperties (this=0x7fffd97e5750, cascade=..., firstProperty=18, lastProperty=429) at ../../Source/WebCore/css/StyleResolver.cpp:2714 #10 0x00007ffff2e35352 in WebCore::StyleResolver::applyMatchedProperties (this=0x7fffd97e5750, matchResult=..., element=0x7fffd9ff9528, shouldUseMatchedPropertiesCache=WebCore::StyleResolver::UseMatchedPropertiesCache) at ../../Source/WebCore/css/StyleResolver.cpp:1795 #11 0x00007ffff2e305e2 in WebCore::StyleResolver::styleForElement (this=0x7fffd97e5750, element=0x7fffd9ff9528, defaultParent=0x7fffd9fe7c60, sharingBehavior=WebCore::AllowStyleSharing, matchingBehavior=WebCore::MatchAllRules, regionForStyling=0x0) at ../../Source/WebCore/css/StyleResolver.cpp:803 #12 0x00007ffff2ecf332 in WebCore::Document::styleForElementIgnoringPendingStylesheets (this=0x7fffd8040fc0, element=0x7fffd9ff9528) at ../../Source/WebCore/dom/Document.cpp:1886 #13 0x00007ffff2f396e7 in WebCore::Element::computedStyle (this=0x7fffd9ff9528, pseudoElementSpecifier=WebCore::NOPSEUDO) at ../../Source/WebCore/dom/Element.cpp:2242 #14 0x00007ffff31b3e42 in WebCore::HTMLTitleElement::textWithDirection (this=0x7fffd9ff9528) at ../../Source/WebCore/html/HTMLTitleElement.cpp:87 #15 0x00007ffff31b3d31 in WebCore::HTMLTitleElement::childrenChanged (this=0x7fffd9ff9528, change=...) at ../../Source/WebCore/html/HTMLTitleElement.cpp:70 #16 0x00007ffff2eac2c3 in WebCore::ContainerNode::notifyChildInserted (this=0x7fffd9ff9528, child=..., source=WebCore::ContainerNode::ChildChangeSourceParser) at ../../Source/WebCore/dom/ContainerNode.cpp:338 #17 0x00007ffff2eadfeb in WebCore::ContainerNode::parserAppendChild (this=0x7fffd9ff9528, newChild=...) at ../../Source/WebCore/dom/ContainerNode.cpp:753 #18 0x00007ffff324b54f in WebCore::insert (task=...) at ../../Source/WebCore/html/parser/HTMLConstructionSite.cpp:107 #19 0x00007ffff324b5de in WebCore::executeInsertTask (task=...) at ../../Source/WebCore/html/parser/HTMLConstructionSite.cpp:114 #20 0x00007ffff324b806 in WebCore::executeTask (task=...) at ../../Source/WebCore/html/parser/HTMLConstructionSite.cpp:152 #21 0x00007ffff324e50d in WebCore::HTMLConstructionSite::insertTextNode (this=0x7fffd97e74a0, characters=..., whitespaceMode=WebCore::WhitespaceUnknown) at ../../Source/WebCore/html/parser/HTMLConstructionSite.cpp:576 #22 0x00007ffff328775b in WebCore::HTMLTreeBuilder::processCharacterBuffer (this=0x7fffd97e7480, buffer=...) at ../../Source/WebCore/html/parser/HTMLTreeBuilder.cpp:2478 #23 0x00007ffff3287156 in WebCore::HTMLTreeBuilder::processCharacter (this=0x7fffd97e7480, token=...) at ../../Source/WebCore/html/parser/HTMLTreeBuilder.cpp:2272 #24 0x00007ffff327d183 in WebCore::HTMLTreeBuilder::processToken (this=0x7fffd97e7480, token=...) at ../../Source/WebCore/html/parser/HTMLTreeBuilder.cpp:382 #25 0x00007ffff327cf83 in WebCore::HTMLTreeBuilder::constructTree (this=0x7fffd97e7480, token=...) at ../../Source/WebCore/html/parser/HTMLTreeBuilder.cpp:341 #26 0x00007ffff3255048 in WebCore::HTMLDocumentParser::constructTreeFromHTMLToken (this=0x7fffd8017c80, rawToken=...) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:321 #27 0x00007ffff3254c77 in WebCore::HTMLDocumentParser::pumpTokenizer (this=0x7fffd8017c80, mode=WebCore::HTMLDocumentParser::AllowYield) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:276 #28 0x00007ffff3254597 in WebCore::HTMLDocumentParser::pumpTokenizerIfPossible (this=0x7fffd8017c80, mode=WebCore::HTMLDocumentParser::AllowYield) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:166 #29 0x00007ffff325558b in WebCore::HTMLDocumentParser::append (this=0x7fffd8017c80, inputSource=...) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:391 #30 0x00007ffff2ec47d9 in WebCore::DecodedDataDocumentParser::flush (this=0x7fffd8017c80, writer=...) at ../../Source/WebCore/dom/DecodedDataDocumentParser.cpp:60 #31 0x00007ffff33c1537 in WebCore::DocumentWriter::end (this=0x7fffd801ac20) at ../../Source/WebCore/loader/DocumentWriter.cpp:244 #32 0x00007ffff33acbd1 in WebCore::DocumentLoader::finishedLoading (this=0x7fffd801ab80, finishTime=0) at ../../Source/WebCore/loader/DocumentLoader.cpp:441 #33 0x00007ffff33ac93a in WebCore::DocumentLoader::notifyFinished (this=0x7fffd801ab80, resource=0x7fffd803ca40) at ../../Source/WebCore/loader/DocumentLoader.cpp:375 #34 0x00007ffff3461090 in WebCore::CachedResource::checkNotify (this=0x7fffd803ca40) at ../../Source/WebCore/loader/cache/CachedResource.cpp:293 #35 0x00007ffff346118e in WebCore::CachedResource::finishLoading (this=0x7fffd803ca40) at ../../Source/WebCore/loader/cache/CachedResource.cpp:309 #36 0x00007ffff345d6f5 in WebCore::CachedRawResource::finishLoading (this=0x7fffd803ca40, data=0x7fffd9fc8330) at ../../Source/WebCore/loader/cache/CachedRawResource.cpp:104 #37 0x00007ffff340feb9 in WebCore::SubresourceLoader::didFinishLoading (this=0x7fffd8011000, finishTime=0) at ../../Source/WebCore/loader/SubresourceLoader.cpp:364 #38 0x00007ffff340b7f3 in WebCore::ResourceLoader::didFinishLoading (this=0x7fffd8011000, finishTime=0) at ../../Source/WebCore/loader/ResourceLoader.cpp:542 #39 0x00007ffff3dbe10f in WebCore::readCallback (asyncResult=0x6b31f0, data=0x7fffd9fc4000) at ../../Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:1295 #40 0x00007fffeb27c7e6 in async_ready_callback_wrapper (source_object=0x7c72d0, res=0x6b31f0, user_data=user_data@entry=0x7fffd9fc4000) at ginputstream.c:523 #41 0x00007fffeb2a20e5 in g_task_return_now (task=0x6b31f0) at gtask.c:1077 #42 0x00007fffeb2a2109 in complete_in_idle_cb (task=0x6b31f0) at gtask.c:1086 #43 0x00007fffea55aa1d in g_main_dispatch (context=0x478b00) at gmain.c:3064 #44 g_main_context_dispatch (context=context@entry=0x478b00) at gmain.c:3663 #45 0x00007fffea55ad88 in g_main_context_iterate (context=0x478b00, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3734 #46 0x00007fffea55b04a in g_main_loop_run (loop=0x901d10) at gmain.c:3928 #47 0x00007ffff44b7750 in WTF::RunLoop::run () at ../../Source/WTF/wtf/gtk/RunLoopGtk.cpp:59 #48 0x00007ffff29a854c in WebKit::ChildProcessMain<WebKit::WebProcess, WebKit::WebProcessMain> (argc=2, argv=0x7fffffffd948) at ../../Source/WebKit2/Shared/unix/ChildProcessMain.h:61 #49 0x00007ffff29a83b1 in WebKit::WebProcessMainUnix (argc=2, argv=0x7fffffffd948) at ../../Source/WebKit2/WebProcess/gtk/WebProcessMainGtk.cpp:77 #50 0x00000000004008d1 in main (argc=2, argv=0x7fffffffd948) at ../../Source/WebKit2/WebProcess/EntryPoint/unix/WebProcessMain.cpp:44
Attachments
Test case (86 bytes, text/html)
2015-02-16 07:16 PST, Renata Hodovan 		no flags
Details
Patch (5.52 KB, patch)
2015-02-17 08:57 PST, Sergio Villar Senin 		cdumez: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Sergio Villar Senin
Comment 1 2015-02-17 01:21:37 PST
I'll take a look
Sergio Villar Senin
Comment 2 2015-02-17 08:57:00 PST
Created attachment 246742 [details] Patch
Sergio Villar Senin
Comment 3 2015-02-24 08:03:41 PST
Ping reviewers.
Chris Dumez
Comment 4 2015-02-24 08:45:48 PST
Comment on attachment 246742 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=246742&action=review > Source/WebCore/css/CSSPrimitiveValueMappings.h:4590 > + if ((supported & (FixedIntegerConversion | FixedFloatConversion)) && isFontRelativeLength() && convertFontRelativeLengthToLengthRequiresNotNullStyle() && !conversionData.style()) Would "&& !isLength()" be identical? If so, I would prefer this rather than adding than adding a new method with a rather obscure check.
Sergio Villar Senin
Comment 5 2015-02-25 04:12:57 PST
Comment on attachment 246742 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=246742&action=review >> Source/WebCore/css/CSSPrimitiveValueMappings.h:4590 >> + if ((supported & (FixedIntegerConversion | FixedFloatConversion)) && isFontRelativeLength() && convertFontRelativeLengthToLengthRequiresNotNullStyle() && !conversionData.style()) > > Would "&& !isLength()" be identical? If so, I would prefer this rather than adding than adding a new method with a rather obscure check. No it wouldn't because for a CSS_EMS the result of !isLength() would be false while the new method would return true. The point is that the last check "!conversionData.style()" only makes sense if the relative length is not CSS_REMS, because the conversion from CSS_REMS to double does not require accessing the style().
Chris Dumez
Comment 6 2015-02-25 11:14:00 PST
Comment on attachment 246742 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=246742&action=review r=me with nits. > Source/WebCore/ChangeLog:8 > + This bug has been here since r110848 but was uncovered by Are you sure about this revision number? It looks unrelated. > Source/WebCore/css/CSSPrimitiveValue.h:294 > + bool convertFontRelativeLengthToLengthRequiresNotNullStyle() const How about moving all the checks to this utility function? Also, I would move this outside the class and mark it as inline. e.g. inline bool CSSPrimitiveValue::convertingToLengthRequiresNonNullStyle(int lengthConversion) const { // This matches the implementation in CSSPrimitiveValue::computeLengthDouble(). switch (m_primitiveUnitType) { case CSS_EMS: case CSS_EXS: case CSS_CHS: return lengthConversion & (FixedIntegerConversion | FixedFloatConversion); default: return false; } } >>> Source/WebCore/css/CSSPrimitiveValueMappings.h:4590 >>> + if ((supported & (FixedIntegerConversion | FixedFloatConversion)) && isFontRelativeLength() && convertFontRelativeLengthToLengthRequiresNotNullStyle() && !conversionData.style()) >> >> Would "&& !isLength()" be identical? If so, I would prefer this rather than adding than adding a new method with a rather obscure check. > > No it wouldn't because for a CSS_EMS the result of !isLength() would be false while the new method would return true. The point is that the last check "!conversionData.style()" only makes sense if the relative length is not CSS_REMS, because the conversion from CSS_REMS to double does not require accessing the style(). Right, I understand now. I still think this should be refactored somehow to make this less error prone but I don't have a better proposal right now so let's land this with minor changes for now. However, if you update the method above as suggested. This would be: if (convertingToLengthRequiresNonNullStyle(supported) && !conversionData.style()) return Length(Undefined);
Sergio Villar Senin
Comment 7 2015-02-26 00:38:33 PST
(In reply to comment #6) > Comment on attachment 246742 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=246742&action=review > > r=me with nits. > > > Source/WebCore/ChangeLog:8 > > + This bug has been here since r110848 but was uncovered by > > Are you sure about this revision number? It looks unrelated. Indeed it's 110484, the 4 and the 8 were dancing apparently :) > > Source/WebCore/css/CSSPrimitiveValue.h:294 > > + bool convertFontRelativeLengthToLengthRequiresNotNullStyle() const > > How about moving all the checks to this utility function? Also, I would move > this outside the class and mark it as inline. > > e.g. OK makes sense.
Sergio Villar Senin
Comment 8 2015-02-26 02:58:39 PST
Committed r180669: <http://trac.webkit.org/changeset/180669>
Note You need to log in before you can comment on or make changes to this bug.