* SUMMARY Using the power of code inspection, I discovered that StyleRareNonInheritedData::operator==() was comparing ContentData objects by pointer instead of using RenderStyle::contentDataEquivalent(). The RenderStyle::contentDataEquivalent() method needs to move to StyleRareNonInheritedData:: contentDataEquivalent() and StyleRareNonInheritedData::operator==() should start using it.
Created attachment 27339 [details] Patch v1 Proposed fix. NOTE: There may be too much const-ness in "bool contentDataEquivalent(const RenderStyle* otherStyle) const" now for the const_cast<>() constructs that must be used.
(In reply to comment #1) > Created an attachment (id=27339) [review] > Patch v1 > > Proposed fix. > > NOTE: There may be too much const-ness in "bool contentDataEquivalent(const > RenderStyle* otherStyle) const" now for the const_cast<>() constructs that must > be used. I should also note the only reason to keep around RenderStyle:: contentDataEquivalent() is that it's used by Node::diff().
Comment on attachment 27339 [details] Patch v1 > - bool contentDataEquivalent(const RenderStyle* otherStyle) const; > + bool contentDataEquivalent(const RenderStyle* otherStyle) const { return const_cast<RenderStyle*>(this)->rareNonInheritedData.access()->contentDataEquivalent(*(const_cast<RenderStyle*>(otherStyle)->rareNonInheritedData.access())); } You don't want to call access() here because it's going to force the non-inherited data to be copied. Instead you can just use the -> operator and * operators directly without the call to access(). review- because this needs to be changed to avoid causing memory bloat due to lost style sharing
Created attachment 27411 [details] Patch v2 Addresses issue in Comment #3 by removing use of the DataRef::access() method.
Comment on attachment 27411 [details] Patch v2 > - bool contentDataEquivalent(const RenderStyle* otherStyle) const; > + bool contentDataEquivalent(const RenderStyle* otherStyle) const { return const_cast<RenderStyle*>(this)->rareNonInheritedData->contentDataEquivalent(*(const_cast<RenderStyle*>(otherStyle)->rareNonInheritedData)); } I think this would be slightly more readable for me without the extra set of parentheses after the "*". r=me
$ git svn dcommit Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/rendering/style/RenderStyle.cpp M WebCore/rendering/style/RenderStyle.h M WebCore/rendering/style/StyleRareNonInheritedData.cpp M WebCore/rendering/style/StyleRareNonInheritedData.h Committed r40734 http://trac.webkit.org/changeset/40734 Removed extra parens as requested in Comment #5.
And a layout test: $ git svn dcommit Committing to http://svn.webkit.org/repository/webkit/trunk ... M LayoutTests/ChangeLog A LayoutTests/fast/css/compare-content-style.html A LayoutTests/platform/mac/fast/css/compare-content-style-expected.txt Committed r43474 http://trac.webkit.org/changeset/43474