Otherwise, I can't jump to the rule definition and fix it.
(In reply to comment #0) > Otherwise, I can't jump to the rule definition and fix it. Good idea. As per the ChangeLog comment, multiple rules with the same selectorText are merged into one entry, so there's no good universal way to bind them to a source location. Suggestions?
(In reply to comment #1) > (In reply to comment #0) > > Otherwise, I can't jump to the rule definition and fix it. > > Good idea. As per the ChangeLog comment, multiple rules with the same selectorText are merged into one entry, so there's no good universal way to bind them to a source location. Suggestions? I would stack them and make the row a bit taller: selector source ------------------ .foo, foo.css:15 .bar bar.css:25 ------------------ .baz baz.css:11 ------------------ ...
Scratch that... > selector source > ------------------ > .foo foo.css:15 > bar.css:25 > ------------------ > .baz baz.css:11 > ------------------ > ...
Created attachment 120795 [details] Patch
(In reply to comment #4) > Created an attachment (id=120795) [details] > Patch EWS Mac builder currently not available
Created attachment 120797 [details] [PATCH] Enable ellipsis for long source links, fix leftmost statusBarItems position (heapshot button removed recently)
Comment on attachment 120797 [details] [PATCH] Enable ellipsis for long source links, fix leftmost statusBarItems position (heapshot button removed recently) View in context: https://bugs.webkit.org/attachment.cgi?id=120797&action=review > Source/WebCore/inspector/InspectorCSSAgent.cpp:103 > + typedef std::pair<String, unsigned> SelectorLocation; Is it time for SelectorProfile to move into its own file. > Source/WebCore/inspector/InspectorCSSAgent.cpp:152 > +inline bool SelectorProfile::isRegularRule(const CSSStyleRule* rule) You should not need to inline heavy methods. > Source/WebCore/inspector/InspectorCSSAgent.cpp:154 > + CSSStyleSheet* parentStyleSheet = rule->parentStyleSheet(); This logic looks familiar. InspectorCSSAgent::detectOrigin ? > Source/WebCore/inspector/InspectorCSSAgent.cpp:165 > +inline String SelectorProfile::sourceURL(const CSSStyleRule* rule) ditto: no need to inline, looks like the logic could be reused. > Source/WebCore/inspector/InspectorCSSAgent.cpp:189 > + RuleMatchingStatsMap::iterator statsByLocationIt = m_ruleMatchingStats.find(m_currentMatchData.selector); Why did this change? It looks like a lot of boilerplate code. Could it be that you've chosen a wrong model for storing the data here?
Comment on attachment 120797 [details] [PATCH] Enable ellipsis for long source links, fix leftmost statusBarItems position (heapshot button removed recently) View in context: https://bugs.webkit.org/attachment.cgi?id=120797&action=review > Source/WebCore/inspector/front-end/profilesPanel.css:191 > + color: white; color: white should apply to all columns, why only > a?
(In reply to comment #8) > (From update of attachment 120797 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=120797&action=review > > > Source/WebCore/inspector/front-end/profilesPanel.css:191 > > + color: white; > > color: white should apply to all columns, why only > a? Selected TD color is set up in dataGrid.css but A does not inherit this as it has the a:-webkit-any-link rule in the user agent stylesheet (blue), so it is roughly the same color as the selected row background.
(In reply to comment #7) > (From update of attachment 120797 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=120797&action=review > > > Source/WebCore/inspector/InspectorCSSAgent.cpp:103 > > + typedef std::pair<String, unsigned> SelectorLocation; > > Is it time for SelectorProfile to move into its own file. This is not going to grow due to the following comments. > > Source/WebCore/inspector/InspectorCSSAgent.cpp:152 > > +inline bool SelectorProfile::isRegularRule(const CSSStyleRule* rule) > > You should not need to inline heavy methods. It is no longer used, thanks to the fact that I made use of InspectorDOMAgent::documentURLString(). sourceURL() can be inlined (it only has one call site.) > > Source/WebCore/inspector/InspectorCSSAgent.cpp:154 > > + CSSStyleSheet* parentStyleSheet = rule->parentStyleSheet(); > > This logic looks familiar. InspectorCSSAgent::detectOrigin ? Yeah, it's that the algo requires the document which is retrieved from the agent instance, which does not help in this case. > > Source/WebCore/inspector/InspectorCSSAgent.cpp:165 > > +inline String SelectorProfile::sourceURL(const CSSStyleRule* rule) > > ditto: no need to inline, looks like the logic could be reused. Yes, will reuse. > > Source/WebCore/inspector/InspectorCSSAgent.cpp:189 > > + RuleMatchingStatsMap::iterator statsByLocationIt = m_ruleMatchingStats.find(m_currentMatchData.selector); > > Why did this change? It looks like a lot of boilerplate code. Could it be that you've chosen a wrong model for storing the data here? We used to coalesce rule stats by selector in the backend (yet we've always had enough data to collect all the stats rule-wise.) Now we split the selector-wise statistics by location (sourceURL + sourceLine) to avoid linear location lookup among the entries for a particular selector. Since we don't need this two-layer mapping in the protocol (and in the frontend to represent the data,) we flatten the second-level map down. Is this wrong?
Created attachment 121829 [details] Patch
Comment on attachment 121829 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=121829&action=review > Source/WebCore/inspector/Inspector.json:1632 > + { "name": "sourceURL", "type": "string", "description": "URL of the resource containing the corresponding rule." }, url > Source/WebCore/inspector/Inspector.json:1633 > + { "name": "sourceLine", "type": "integer", "description": "Selector line number in the resource for the corresponding rule." }, lineNumber
Committed r104710: <http://trac.webkit.org/changeset/104710>