Bug 157097

Summary: [css-grid] Store auto-repeat information in style
Product: WebKit Reporter: Sergio Villar Senin <svillar>
Component: CSSAssignee: Sergio Villar Senin <svillar>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, koivisto, rniwa, simon.fraser, tonikitoo
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 60731    
Attachments:
Description Flags
Patch
none
Patch for landing none

Sergio Villar Senin
Reported 2016-04-27 12:18:34 PDT
[css-grid] Store auto-repeat information in style
Attachments
Patch (26.66 KB, patch)
2016-04-27 12:27 PDT, Sergio Villar Senin
no flags
Patch for landing (26.81 KB, patch)
2016-04-28 02:17 PDT, Sergio Villar Senin
no flags
Sergio Villar Senin
Comment 1 2016-04-27 12:27:23 PDT
Darin Adler
Comment 2 2016-04-27 15:48:11 PDT
Comment on attachment 277526 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=277526&action=review > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1055 > + const Vector<GridTrackSize>& trackSizes = isRowAxis ? style.gridColumns() : style.gridRows(); > + const Vector<GridTrackSize>& autoRepeatTrackSizes = isRowAxis ? style.gridAutoRepeatColumns() : style.gridAutoRepeatRows(); > + const OrderedNamedGridLinesMap& orderedNamedGridLines = isRowAxis ? style.orderedNamedGridColumnLines() : style.orderedNamedGridRowLines(); I think auto& really works well to make code like this less wordy > Source/WebCore/css/StyleBuilderConverter.h:162 > + static bool createGridTrackList(CSSValue&, Vector<GridTrackSize>& trackSizes, NamedGridLinesMap&, OrderedNamedGridLinesMap&, > + Vector<GridTrackSize>& autoRepeatTrackSizes, NamedGridLinesMap& autoRepeatNamedLines, OrderedNamedGridLinesMap& autoRepeatOrderedNamedLines, > + unsigned& autoRepeatInsertionPoint, AutoRepeatType&, StyleResolver&); There are so many arguments here now that I think we may need to make some kind of class instead of just a function. With 10 different arguments that all are references, so all seem to be in/out, I think we are really looking at too many. Maybe this could still be a function, but return a struct with all the return valeus instead of using out arguments. > Source/WebCore/css/StyleBuilderConverter.h:864 > + NamedGridLinesMap::AddResult result = namedGridLines.add(namedGridLine, Vector<unsigned>()); Should either use auto here, or combine this with the next line. > Source/WebCore/css/StyleBuilderConverter.h:866 > + OrderedNamedGridLinesMap::AddResult orderedResult = orderedNamedGridLines.add(currentNamedGridLine, Vector<String>()); Should either use auto here, or combine this with the next line. > Source/WebCore/rendering/style/StyleGridData.h:108 > + AutoRepeatType m_autoRepeatRowsType; > private: WebKit style calls for a blank line here before private
Sergio Villar Senin
Comment 3 2016-04-28 02:17:21 PDT
Created attachment 277608 [details] Patch for landing Added a new struct which stores all the tracks data. Also created a new macro to store all the track info in style suitable for both rows and columns
Sergio Villar Senin
Comment 4 2016-04-28 02:28:40 PDT
Landed on r200182
Note You need to log in before you can comment on or make changes to this bug.