Adding a histogram to track when m_needsCompositedScrolling is turned on and off
Created attachment 191986 [details]
Comment on attachment 191986 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=191986&action=review
> + if (acceleratedCompositingForOverflowScrollEnabled())
> + HistogramSupport::histogramEnumeration("Renderer.NeedsCompositedScrolling", m_needsCompositedScrolling, 2);
It would have been nice to explain *what* you are trying to measure as well as *why*. Without the context, it's difficult to know if this is correct or why we can't other some other mechanism like FeatureObserver.
After talking with Glenn over IM, they want to measure the frequency of the feature at the layer level as a rough estimate of what could be done. They understand that this number will not be actionable but that giving the reasons for not having composited scrolling are expensive to compute and we want the overhead minimal.
All in all, the code change make sense but please add the relevant bits to the ChangeLog or as a comment in the code (remember that WebKit prefers 'why' comments over 'what' comments though).
Out of context, I've no idea what it means to "add a Histogram". How do I get to see the histogram?
I think what you really mean is "add some instrumentation so that Google can collect data, which might be displayed in a histogram at some point".
Created attachment 192219 [details]
Comment on attachment 192219 [details]
Clearing flags on attachment: 192219
Committed r145229: <http://trac.webkit.org/changeset/145229>
All reviewed patches have been landed. Closing bug.