Add helper funtion for checking pointer equivalency and use it
Created attachment 262877 [details] Patch
Not sure if RefPtr.h is the best place for bool areEquivalent(const T* a, const T* b), and it would be nice to have a unique_ptr version. Where to put it?
Comment on attachment 262877 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=262877&action=review > Source/WTF/ChangeLog:3 > + Add helper funtion for checking pointer equivalency and use it Where does this use of the word "equivalency" originate from? I'm not familiar with it, and it looks quite misleading to me.
It was used in the style code quite a bit: - && shadowDataEquivalent(o) - && willChangeDataEquivalent(o) - && reflectionDataEquivalent(o) - && animationDataEquivalent(o) - && transitionDataEquivalent(o) + && areEquivalent(m_counterDirectives.get(), o.m_counterDirectives.get()) + && areEquivalent(m_boxShadow.get(), o.m_boxShadow.get()) + && areEquivalent(m_willChange, o.m_willChange) + && areEquivalent(m_boxReflect, o.m_boxReflect) + && areEquivalent(m_animations.get(), o.m_animations.get()) + && areEquivalent(m_transitions.get(), o.m_transitions.get())
Comment on attachment 262877 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=262877&action=review > Source/WTF/wtf/RefPtr.h:223 > +template<typename T> inline bool areEquivalent(const T* a, const T* b) > +{ > + return a == b || (a && b && *a == *b); > +} Should just implement this for all types instead of just pointers: template<typename T> inline bool areEquivalent(const T& a, const T& b) { return a == b || (a && b && *a == *b); } Then you don’t need a RefPtr overload. I suggest putting this somewhere in the WebCore tree instead of in WTF and naming it something more like isPointedToDataEqual.
I'm not sure I can handle that singular use of data :)
We like arePointingToEqualData().
Created attachment 262971 [details] Patch
I put it in WTF because there's place in WebCore for shared, non-platform-specific utilities, and we may want to use it in JSC or WebKit.
Created attachment 262973 [details] Patch
Comment on attachment 262973 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=262973&action=review > Source/WebCore/rendering/style/FillLayer.cpp:157 > + return arePointingToEqualData(m_image.get(), o.m_image.get()) && m_xPosition == o.m_xPosition && m_yPosition == o.m_yPosition I believe there is no need for the .get() here since arePointingToEqualData works on smart pointers too, not just raw pointers. > Source/WebCore/rendering/style/RenderStyle.cpp:559 > - if (rareNonInheritedData->m_grid.get() != other.rareNonInheritedData->m_grid.get() > - || rareNonInheritedData->m_gridItem.get() != other.rareNonInheritedData->m_gridItem.get()) > + if (rareNonInheritedData->m_grid != other.rareNonInheritedData->m_grid > + || rareNonInheritedData->m_gridItem != other.rareNonInheritedData->m_gridItem) Too bad we don’t have test cases for this bug fix. > Source/WebCore/rendering/style/RenderStyle.cpp:943 > + if (arePointingToEqualData(rareNonInheritedData->m_willChange.get(), willChangeData.get())) I believe there is no need for the .get() here since arePointingToEqualData works on smart pointers too, not just raw pointers.
(In reply to comment #11) > Comment on attachment 262973 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=262973&action=review > > > Source/WebCore/rendering/style/FillLayer.cpp:157 > > + return arePointingToEqualData(m_image.get(), o.m_image.get()) && m_xPosition == o.m_xPosition && m_yPosition == o.m_yPosition > > I believe there is no need for the .get() here since arePointingToEqualData > works on smart pointers too, not just raw pointers. I'll fix. > > Source/WebCore/rendering/style/RenderStyle.cpp:559 > > - if (rareNonInheritedData->m_grid.get() != other.rareNonInheritedData->m_grid.get() > > - || rareNonInheritedData->m_gridItem.get() != other.rareNonInheritedData->m_gridItem.get()) > > + if (rareNonInheritedData->m_grid != other.rareNonInheritedData->m_grid > > + || rareNonInheritedData->m_gridItem != other.rareNonInheritedData->m_gridItem) > > Too bad we don’t have test cases for this bug fix. Yes! > > Source/WebCore/rendering/style/RenderStyle.cpp:943 > > + if (arePointingToEqualData(rareNonInheritedData->m_willChange.get(), willChangeData.get())) > > I believe there is no need for the .get() here since arePointingToEqualData > works on smart pointers too, not just raw pointers. These are different types (PassRefPtr vs RefPtr), so the .get() are needed to turn both into raw pointers.
https://trac.webkit.org/r191017