The isolation property is part of the CSS Blending specification: http://dev.w3.org/fxtf/compositing-1/#isolation Parse and implement a prefixed version of the property. The allowed values for -webkit-isolation are isolate and auto(default).
Created attachment 224476 [details] Patch V1
Created attachment 224479 [details] Patch V2
Comment on attachment 224479 [details] Patch V2 View in context: https://bugs.webkit.org/attachment.cgi?id=224479&action=review > Source/WebCore/css/CSSPrimitiveValueMappings.h:4158 > +template<> inline CSSPrimitiveValue::CSSPrimitiveValue(Isolation i) Short variable names like "i" are not consistent with WebKit style. We tend to prefer e.g like "isolation". > Source/WebCore/css/CSSPrimitiveValueMappings.h:4169 > + switch (i) { > + case IsolationAuto: > + m_value.valueID = CSSValueAuto; > + break; > + case IsolationIsolate: > + m_value.valueID = CSSValueIsolate; > + break; > + } We should have a default case here with ASSERT_NOT_REACHED(), just like we do when converting in the other direction. > Source/WebCore/rendering/style/RenderStyle.h:974 > + void setIsolation(Isolation v) { rareNonInheritedData.access()->m_isolation = v; } Using rareNonInheritedData.access() like this will force the RenderStyle to create its own unique copy of the StyleRareNonInheritedData. If you look at other setters in this file, they use a macro (SET_VAR) that first checks if the value being written is identical to the existing one, and avoids a copy in that case.
Created attachment 224608 [details] Patch V3 Addressed issues highlighted by Andreas Kling.
Comment on attachment 224608 [details] Patch V3 View in context: https://bugs.webkit.org/attachment.cgi?id=224608&action=review > Source/WebCore/rendering/style/RenderStyle.h:975 > + bool hasIsolation() const { return rareNonInheritedData->m_isolation != IsolationAuto; } Won't this crash if rareNonInheritedData hasn't been allocated?
(In reply to comment #5) > (From update of attachment 224608 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=224608&action=review > > > Source/WebCore/rendering/style/RenderStyle.h:975 > > + bool hasIsolation() const { return rareNonInheritedData->m_isolation != IsolationAuto; } > > Won't this crash if rareNonInheritedData hasn't been allocated? All of RenderStyle's constructors initialise a valid rareNonInheritedData member, so I don't think this will crash.
Created attachment 225055 [details] Patch V4 Add SVG implementation. Add additional tests for HTML and SVG. Please have another look.
Comment on attachment 225055 [details] Patch V4 View in context: https://bugs.webkit.org/attachment.cgi?id=225055&action=review The patch looks good to me. Just two snippets. I give the other reviewers some time to review the patch as well before giving my r+. Maybe Simon has more comments. > Source/WebCore/rendering/style/RenderStyle.h:974 > + void setBlendMode(BlendMode v) { SET_VAR(rareNonInheritedData, m_effectiveBlendMode, v); } > bool hasBlendMode() const { return static_cast<BlendMode>(rareNonInheritedData->m_effectiveBlendMode) != BlendModeNormal; } > + > + Isolation isolation() const { return static_cast<Isolation>(rareNonInheritedData->m_isolation); } > + void setIsolation(Isolation v) { SET_VAR(rareNonInheritedData, m_isolation, v); } should still have longer names like blendMode and isolation instead of v. Regardless of the use elsewhere.
Created attachment 225068 [details] Patch V4. Addressed Dirk's comments
Comment on attachment 225068 [details] Patch V4. Addressed Dirk's comments LGTM. Since no further review comments came from Simon and kling: r=me
Comment on attachment 225068 [details] Patch V4. Addressed Dirk's comments Clearing flags on attachment: 225068 Committed r164795: <http://trac.webkit.org/changeset/164795>
All reviewed patches have been landed. Closing bug.
css3/compositing/isolation-isolate-blended-child.html test added in this patch fails every time on one of the bots. Filed bug 129468.