Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
RenderElement::m_style should be a Ref.
<https://webkit.org/b/123401>

Source/WebCore:

Made RenderElement::m_style a Ref. This codifies the fact that it
can never be null after construction.

Removed a couple of unnecessary null checks that were exposed as
compilation failures.

Reviewed by Antti Koivisto.

Source/WTF:

Added a Ref::replace() so we can Indiana Jones the new style in
RenderElement::setStyle() while keeping a handle on the old style
for a while longer.

Reviewed by Antti Koivisto.

Canonical link: https://commits.webkit.org/141513@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@158111 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Andreas Kling committed Oct 28, 2013
1 parent 84875f7 commit 06b651c
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 18 deletions.
11 changes: 11 additions & 0 deletions Source/WTF/ChangeLog
@@ -1,3 +1,14 @@
2013-10-28 Andreas Kling <akling@apple.com>

RenderElement::m_style should be a Ref.
<https://webkit.org/b/123401>

Added a Ref::replace() so we can Indiana Jones the new style in
RenderElement::setStyle() while keeping a handle on the old style
for a while longer.

Reviewed by Antti Koivisto.

2013-10-28 Carlos Garcia Campos <cgarcia@igalia.com>

Unreviewed. Fix make distcheck.
Expand Down
9 changes: 9 additions & 0 deletions Source/WTF/wtf/Ref.h
Expand Up @@ -60,10 +60,19 @@ template<typename T> class Ref {
const T& get() const { return *m_ptr; }
T& get() { return *m_ptr; }

template<typename U> PassRef<T> replace(PassRef<U>) WARN_UNUSED_RETURN;

private:
T* m_ptr;
};

template<typename T> template<typename U> inline PassRef<T> Ref<T>::replace(PassRef<U> reference)
{
auto oldReference = adoptRef(*m_ptr);
m_ptr = &reference.leakRef();
return oldReference;
}

} // namespace WTF

using WTF::Ref;
Expand Down
13 changes: 13 additions & 0 deletions Source/WebCore/ChangeLog
@@ -1,3 +1,16 @@
2013-10-28 Andreas Kling <akling@apple.com>

RenderElement::m_style should be a Ref.
<https://webkit.org/b/123401>

Made RenderElement::m_style a Ref. This codifies the fact that it
can never be null after construction.

Removed a couple of unnecessary null checks that were exposed as
compilation failures.

Reviewed by Antti Koivisto.

2013-10-28 Bastien Nocera <hadess@hadess.net>

Name all the GLib timeout sources
Expand Down
31 changes: 15 additions & 16 deletions Source/WebCore/rendering/RenderElement.cpp
Expand Up @@ -392,7 +392,7 @@ void RenderElement::initializeStyle()

void RenderElement::setStyle(PassRef<RenderStyle> style)
{
if (m_style == &style.get()) {
if (&m_style.get() == &style.get()) {
#if USE(ACCELERATED_COMPOSITING)
// We need to run through adjustStyleDifference() for iframes, plugins, and canvas so
// style sharing is disabled for them. That should ensure that we never hit this code path.
Expand All @@ -410,19 +410,18 @@ void RenderElement::setStyle(PassRef<RenderStyle> style)
diff = adjustStyleDifference(diff, contextSensitiveProperties);

styleWillChange(diff, style.get());

RefPtr<RenderStyle> oldStyle = m_style.release();
m_style = std::move(style);

updateFillImages(oldStyle ? oldStyle->backgroundLayers() : nullptr, m_style->backgroundLayers());
updateFillImages(oldStyle ? oldStyle->maskLayers() : nullptr, m_style->maskLayers());
Ref<RenderStyle> oldStyle(m_style.replace(std::move(style)));

updateFillImages(oldStyle.get().backgroundLayers(), m_style->backgroundLayers());
updateFillImages(oldStyle.get().maskLayers(), m_style->maskLayers());

updateImage(oldStyle ? oldStyle->borderImage().image() : nullptr, m_style->borderImage().image());
updateImage(oldStyle ? oldStyle->maskBoxImage().image() : nullptr, m_style->maskBoxImage().image());
updateImage(oldStyle.get().borderImage().image(), m_style->borderImage().image());
updateImage(oldStyle.get().maskBoxImage().image(), m_style->maskBoxImage().image());

#if ENABLE(CSS_SHAPES)
updateShapeImage(oldStyle ? oldStyle->shapeInside() : nullptr, m_style->shapeInside());
updateShapeImage(oldStyle ? oldStyle->shapeOutside() : nullptr, m_style->shapeOutside());
updateShapeImage(oldStyle.get().shapeInside(), m_style->shapeInside());
updateShapeImage(oldStyle.get().shapeOutside(), m_style->shapeOutside());
#endif

// We need to ensure that view->maximalOutlineSize() is valid for any repaints that happen
Expand All @@ -432,12 +431,12 @@ void RenderElement::setStyle(PassRef<RenderStyle> style)

bool doesNotNeedLayout = !parent();

styleDidChange(diff, oldStyle.get());
styleDidChange(diff, &oldStyle.get());

// Text renderers use their parent style. Notify them about the change.
for (RenderObject* child = firstChild(); child; child = child->nextSibling()) {
if (child->isText())
toRenderText(child)->styleDidChange(diff, oldStyle.get());
toRenderText(child)->styleDidChange(diff, &oldStyle.get());
}

// FIXME: |this| might be destroyed here. This can currently happen for a RenderTextFragment when
Expand All @@ -455,9 +454,9 @@ void RenderElement::setStyle(PassRef<RenderStyle> style)
if (updatedDiff == StyleDifferenceLayout)
setNeedsLayoutAndPrefWidthsRecalc();
else if (updatedDiff == StyleDifferenceLayoutPositionedMovementOnly)
setNeedsPositionedMovementLayout(oldStyle.get());
setNeedsPositionedMovementLayout(&oldStyle.get());
else if (updatedDiff == StyleDifferenceSimplifiedLayoutAndPositionedMovement) {
setNeedsPositionedMovementLayout(oldStyle.get());
setNeedsPositionedMovementLayout(&oldStyle.get());
setNeedsSimplifiedNormalFlowLayout();
} else if (updatedDiff == StyleDifferenceSimplifiedLayout)
setNeedsSimplifiedNormalFlowLayout();
Expand Down Expand Up @@ -950,7 +949,7 @@ void RenderElement::styleDidChange(StyleDifference diff, const RenderStyle* oldS
return;

if (diff == StyleDifferenceLayout || diff == StyleDifferenceSimplifiedLayout) {
RenderCounter::rendererStyleChanged(this, oldStyle, m_style.get());
RenderCounter::rendererStyleChanged(this, oldStyle, &m_style.get());

// If the object already needs layout, then setNeedsLayout won't do
// any work. But if the containing block has changed, then we may need
Expand Down Expand Up @@ -1015,7 +1014,7 @@ void RenderElement::willBeRemovedFromTree()
}

bool repaintFixedBackgroundsOnScroll = shouldRepaintFixedBackgroundsOnScroll();
if (repaintFixedBackgroundsOnScroll && m_style && m_style->hasFixedBackgroundImage())
if (repaintFixedBackgroundsOnScroll && m_style->hasFixedBackgroundImage())
view().frameView().removeSlowRepaintObject(this);

if (isOutOfFlowPositioned() && parent()->childrenInline())
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/rendering/RenderElement.h
Expand Up @@ -35,7 +35,7 @@ class RenderElement : public RenderObject {

bool hasInitializedStyle() const { return m_hasInitializedStyle; }

RenderStyle* style() const { return m_style.get(); }
RenderStyle* style() const { return const_cast<RenderStyle*>(&m_style.get()); }
RenderStyle* firstLineStyle() const;

void initializeStyle();
Expand Down Expand Up @@ -170,7 +170,7 @@ class RenderElement : public RenderObject {
RenderObject* m_firstChild;
RenderObject* m_lastChild;

RefPtr<RenderStyle> m_style;
Ref<RenderStyle> m_style;

// FIXME: Get rid of this hack.
// Store state between styleWillChange and styleDidChange
Expand Down

0 comments on commit 06b651c

Please sign in to comment.