Small refactoring of the states-machine we use in the grid tracks sizing algorithm.
Created attachment 284697 [details] Patch
Comment on attachment 284697 [details] Patch Change looks fine, although I would not exactly call this "refactoring".
Comment on attachment 284697 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=284697&action=review > Source/WebCore/rendering/RenderGrid.cpp:264 > + bool isValidTransition(GridTrackSizingDirection direction) const Instead of a const member function, this should be a static member function, since it’s not dependent on the state of the object at all.
Comment on attachment 284697 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=284697&action=review >> Source/WebCore/rendering/RenderGrid.cpp:264 >> + bool isValidTransition(GridTrackSizingDirection direction) const > > Instead of a const member function, this should be a static member function, since it’s not dependent on the state of the object at all. It depends on the current object's state, defined in its "sizingState" attribute. This change, precisely, removes the code which was altering the value of the sizingState attribute when the object it's at the ColumnSizingSecondIteration and we are going to perform a transition in the ForRows direction. By adding 'const' I'm trying to avoid the same mistake and state clearly that this function performs some read-only checks.
(In reply to comment #2) > Comment on attachment 284697 [details] > Patch > > Change looks fine, although I would not exactly call this "refactoring". Perhaps I should change the description, indeed.
Created attachment 284711 [details] Patch
Comment on attachment 284711 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=284711&action=review > Source/WebCore/rendering/RenderGrid.cpp:264 > + bool isValidTransition(GridTrackSizingDirection direction) const Hmm, I set commit-queue+ on this, but this is still using a const member function instead of a static member function.
Comment on attachment 284711 [details] Patch Clearing flags on attachment: 284711 Committed r203785: <http://trac.webkit.org/changeset/203785>
All reviewed patches have been landed. Closing bug.
Comment on attachment 284711 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=284711&action=review >> Source/WebCore/rendering/RenderGrid.cpp:264 >> + bool isValidTransition(GridTrackSizingDirection direction) const > > Hmm, I set commit-queue+ on this, but this is still using a const member function instead of a static member function. And it should because it's checking the value of sizingState which is a member of the GridSizingData struct (it does not have the m_ as it's a struct). That'll become less confusing after the refactoring and cleanup of that struct which is becoming a C++ class.