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
I'll take a look
Created attachment 246742 [details] Patch
Ping reviewers.
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.
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().
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);
(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.
Committed r180669: <http://trac.webkit.org/changeset/180669>