Move two shape related properties into CSSPropertyParser

third_party/WebKit/Source/core/css/CSSBasicShapeValues.cpp

 /*
  * Copyright (C) 2011 Adobe Systems Incorporated. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1. Redistributions of source code must retain the above
  *    copyright notice, this list of conditions and the following
  *    disclaimer.
@@ skipping 70 matching lines @@
         side = toCSSPrimitiveValue(toCSSValuePair(*offset).first()).getValueID();
         amount = &toCSSPrimitiveValue(toCSSValuePair(*offset).second());
     } else {
         amount = toCSSPrimitiveValue(offset.get());
     }
 
     if (side == CSSValueCenter) {
         side = defaultSide;
         amount = cssValuePool().createValue(50, CSSPrimitiveValue::UnitType::Percentage);
     } else if ((side == CSSValueRight || side == CSSValueBottom)
-        && amount->isPercentage()) {
+        && amount && amount->isPercentage()) {

[Timothy Loh, 2016/01/27 00:44:03]

What changed to make us need this?

[rwlbuis, 2016/01/27 20:02:39]

parseFillPosition has a special ResolveValuesAsPercent mode, that
consumePosition does not have. So in the case of positions of length 1 or 2 this
mode was used, primitive values with percentages were created instead of value
ids, and we end up with a non-null value for amount.

But consumePosition does not have this mode, so we just have a single side value
and null amount for standalone left/right/bottom/top values, causing crashes in
the two else blocks if nothing is changed.

In my latest code I at least moved the middle block up, since the if check can
only succeed with a CSSValuePair, this way we at least get rid of one amount
null check.

         side = defaultSide;
         amount = cssValuePool().createValue(100 - amount->getFloatValue(), CSSPrimitiveValue::UnitType::Percentage);
-    } else if (amount->isLength() && !amount->getFloatValue()) {
+    } else if (!amount || (amount->isLength() && !amount->getFloatValue())) {
         if (side == CSSValueRight || side == CSSValueBottom)
             amount = cssValuePool().createValue(100, CSSPrimitiveValue::UnitType::Percentage);
         else
             amount = cssValuePool().createValue(0, CSSPrimitiveValue::UnitType::Percentage);
         side = defaultSide;
     }
 
     return CSSValuePair::create(cssValuePool().createIdentifierValue(side), amount.release(), CSSValuePair::KeepIdenticalValues);
 }
 
@@ skipping 293 matching lines @@
     visitor->trace(m_left);
     visitor->trace(m_topLeftRadius);
     visitor->trace(m_topRightRadius);
     visitor->trace(m_bottomRightRadius);
     visitor->trace(m_bottomLeftRadius);
     CSSValue::traceAfterDispatch(visitor);
 }
 
 } // namespace blink