This is a follow-up of bug 84920, currently RenderLayer z-index list dirtying is done in RenderBoxModelObject::styleWillChange. This has one downside: to tighten our code, we would rather be able to determine when we are switch from being a stacking context to a non-stacking context. We could add a pre-style change hook in RenderLayer but we already have a post-style change hook in RenderLayer::styleChanged. To avoid duplication, we should try to move the code there. As a side note, this would also make the layer handle its low-level operation instead of delegating that to the associated RenderBoxModelObject.
Created attachment 140623 [details] Proposed refactoring 1: Move code and added handling for the stacking context transition.
Comment on attachment 140623 [details] Proposed refactoring 1: Move code and added handling for the stacking context transition. View in context: https://bugs.webkit.org/attachment.cgi?id=140623&action=review > Source/WebCore/rendering/RenderLayer.cpp:195 > + m_zOrderListsDirty = isStackingContext(); This needs a “why” comment. > Source/WebCore/rendering/RenderLayer.cpp:4702 > + // FIXME: RenderLayer already handles visibility changes through our visiblity dirty bits. This logic could > + // likely be folded along with the rest. > + if (oldStyle->zIndex() != renderer()->style()->zIndex() || oldStyle->visibility() != renderer()->style()->visibility()) { > + dirtyStackingContextZOrderLists(); > + if (isStackingContext) > + dirtyZOrderLists(); > + } This work is redundant if isStackingContext != wasStackingContext since dirtyStackingContextZOrderLists and dirtyZOrderLists were already called as necessary. I think it would be better to structure the code so we don’t do it twice. Could be as simple as putting a return inside the earlier code. > Source/WebCore/rendering/RenderLayer.h:620 > + bool layerWithStyleIsStackingContext(const RenderStyle* style) const { return !style->hasAutoZIndex() || renderer()->isRenderView(); } The naming here is strange. I think that it could be named isStackingContext just like the public function and just be an overload that takes an explicit style argument.
Created attachment 140827 [details] Patch for landing
Comment on attachment 140827 [details] Patch for landing Clearing flags on attachment: 140827 Committed r116480: <http://trac.webkit.org/changeset/116480>
All reviewed patches have been landed. Closing bug.