New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Jan 2014
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment
link

Issue 311465: Viewport units in -webkit-filter will assert/crash

Reported by pdr@chromium.org, Oct 25 2013 Project Member

Issue description

The following will assert:
<!DOCTYPE html>
<html>
<div style="-webkit-filter: drop-shadow(1vh 1vw black)"></div>
</html>

This is likely easy to fix: I think we're just missing CSS_VW and friends in the switch in CSSPrimitiveValue.cpp.

The stacktrace is listed below:
../../third_party/WebKit/Source/core/css/CSSPrimitiveValue.cpp(613) : double WebCore::CSSPrimitiveValue::computeLengthDouble(const WebCore::RenderStyle *, const WebCore::RenderStyle *, float, bool)
1   0xe7384ce WebCore::CSSPrimitiveValue::computeLengthDouble(WebCore::RenderStyle const*, WebCore::RenderStyle const*, float, bool)
2   0xe738003 int WebCore::CSSPrimitiveValue::computeLength<int>(WebCore::RenderStyle const*, WebCore::RenderStyle const*, float, bool)
3   0xe829afd WebCore::FilterOperationResolver::createFilterOperations(WebCore::CSSValue*, WebCore::RenderStyle const*, WebCore::RenderStyle const*, WebCore::FilterOperations&, WebCore::StyleResolverState&)
4   0xe858b12 WebCore::StyleBuilder::oldApplyProperty(WebCore::CSSPropertyID, WebCore::StyleResolverState&, WebCore::CSSValue*, bool, bool)
5   0xe855d16 WebCore::StyleBuilder::applyProperty(WebCore::CSSPropertyID, WebCore::StyleResolverState&, WebCore::CSSValue*)
6   0xe884297 void WebCore::StyleResolver::applyProperties<(WebCore::StyleResolver::StyleApplicationPass)3>(WebCore::StyleResolverState&, WebCore::StylePropertySet const*, WebCore::StyleRule*, bool, bool, WebCore::PropertyWhitelistType)
7   0xe87b822 void WebCore::StyleResolver::applyMatchedProperties<(WebCore::StyleResolver::StyleApplicationPass)3>(WebCore::StyleResolverState&, WebCore::MatchResult const&, bool, int, int, bool)
8   0xe8729fe WebCore::StyleResolver::applyMatchedProperties(WebCore::StyleResolverState&, WebCore::MatchResult const&)
9   0xe86ef68 WebCore::StyleResolver::styleForElement(WebCore::Element*, WebCore::RenderStyle*, WebCore::StyleSharingBehavior, WebCore::RuleMatchingBehavior, WebCore::RenderRegion*)
10  0xdc8ab4d WebCore::Element::originalStyleForRenderer()
11  0xdc8aa9d WebCore::Element::styleForRenderer()
12  0xdd112c6 WebCore::NodeRenderingContext::createRendererForElementIfNeeded()
13  0xdc89c89 WebCore::Element::attach(WebCore::Node::AttachContext const&)
14  0xdbecfe3 WebCore::ContainerNode::attachChildren(WebCore::Node::AttachContext const&)
15  0xdbe91ac WebCore::ContainerNode::attach(WebCore::Node::AttachContext const&)
16  0xdc89d61 WebCore::Element::attach(WebCore::Node::AttachContext const&)
17  0xdbecfe3 WebCore::ContainerNode::attachChildren(WebCore::Node::AttachContext const&)
 

Comment 1 by senorblanco@chromium.org, Oct 26 2013

That definitely fixes the assert, but I have no clue what the factor should be for these units. Any idea?

Comment 2 by bugdroid1@chromium.org, Jan 3 2014

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=164448

------------------------------------------------------------------------
r164448 | timloh@chromium.org | 2014-01-03T08:28:33.534806Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/css3/viewport-percentage-lengths/css3-viewport-percentage-lengths-getStyle.html?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RootInlineBox.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/masking/parsing-clip-path-shape-expected.txt?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSPrimitiveValue.h?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/resolver/StyleResolverState.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/virtual/legacy-animations-engine/animations/interpolation/line-height-interpolation-expected.txt?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderMenuList.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/animation/AnimatableLengthTest.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/resolver/StyleResolver.h?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/style/RenderStyle.h?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/Length.h?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderView.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/computed-offset-with-zoom-expected.txt?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSCalculationValueTest.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/win/virtual/legacy-animations-engine/animations/interpolation/line-height-interpolation-expected.txt?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/html/HTMLMetaElement-in.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/linux/virtual/legacy-animations-engine/animations/interpolation/line-height-interpolation-expected.txt?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/masking/parsing-clip-path-shape.html?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderGrid.cpp?r1=164448&r2=164447&pathrev=164448
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/css3/viewport-percentage-lengths/viewport-percentage-lengths-page-zoom-expected.txt?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderTable.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSPrimitiveValue.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/resolver/FontBuilder.h?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSLengthFunctions.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/web/PageScaleConstraintsSet.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/computed-offset-with-zoom.html?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/svg/RenderSVGRoot.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/frame/FrameView.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/MediaQueryEvaluator.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderBR.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSToLengthConversionData.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderScrollbarPart.cpp?r1=164448&r2=164447&pathrev=164448
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/css3/viewport-percentage-lengths/viewport-percentage-lengths-page-zoom.html?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/shapes/parsing/parsing-shape-lengths-expected.txt?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSGradientValue.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderView.h?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderReplaced.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/resolver/MatchedPropertiesCache.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/resolver/FontBuilder.cpp?r1=164448&r2=164447&pathrev=164448
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/css3/viewport-percentage-lengths/viewport-percentage-lengths-resize-expected.txt?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSComputedStyleDeclaration.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderBoxModelObject.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/TreeScope.h?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/animations/interpolation/line-height-interpolation-expected.txt?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/web/WebViewImpl.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/shapes/parsing/parsing-shape-lengths.html?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSLengthFunctions.h?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderFlexibleBox.h?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/svg/SVGSVGElement.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSToLengthConversionData.h?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderThemeChromiumMac.mm?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/resolver/StyleBuilderCustom.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderInline.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSCalculationValue.cpp?r1=164448&r2=164447&pathrev=164448
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/css3/viewport-percentage-lengths/viewport-percentage-lengths-resize.html?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Document.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/resolver/StyleBuilderConverter.h?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/ViewportDescription.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/resolver/MatchedPropertiesCache.h?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderBox.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSMatrix.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/LengthFunctions.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderText.h?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderBlock.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/TreeScope.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderTableCell.h?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSPrimitiveValueMappings.h?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/html/HTMLAreaElement.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/resolver/ViewportStyleResolver.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/resolver/StyleResolver.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/style/RenderStyle.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/css3/viewport-percentage-lengths/css3-viewport-percentage-lengths-getStyle-expected.txt?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderFlexibleBox.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderObject.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderTableSection.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/animation/css/CSSAnimatableValueFactory.cpp?r1=164448&r2=164447&pathrev=164448
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Document.h?r1=164448&r2=164447&pathrev=164448

Move viewport unit resolution to style recalc time

This patch moves the resolution of viewport units to style recalc time.
Currently viewport units are left unresolved during style recalcs which
leads to several problems (see linked bugs). Moving the resolution will
fix these problems, as well as reduce the plumbing that goes on. 

This patch is touches a lot of files since the valueForLength functions
now no longer need a RenderView, but the interesting changes are in:

- CSSToLengthConversionData -> CSSPrimitiveValue, for moving resolution
to style recalc time
- Length / LengthFunction, no longer needs to know about viewport units
- FrameView -> Document -> MatchedPropertiesCache, for scheduling style
recalcs upon resize

Viewport Lengths are currently also used in viewport descriptions (that
is, @viewport and the viewport meta tag). I've added to Length two more
enum values (DeviceWidth and DeviceHeight), alongside ExtendToZoom.

Note that getComputedStyle will now do the right thing and return pixel
values when viewport units are used. This behaviour matches Firefox and
the css3-cascade spec.

I've added a test for the most starred bug (124331) and for the various
other issues fixed by this patch I'll make test-only patches.

Note that calc integration with viewport units will be fixed separately
to simplify reviews.

TBR=dglazkov@chromium.org
BUG= 124331 , 137617 , 125709 , 261298 , 173407 , 322365 , 310874 , 311465 

Review URL: https://codereview.chromium.org/82083002
------------------------------------------------------------------------

Comment 3 by timloh@chromium.org, Jan 6 2014

Status: Fixed

Comment 4 by suzyh@chromium.org, Apr 5 2017

Components: -Blink>CSS>Filters Blink>Compositing>Filters

Sign in to add a comment