Update FeatureObserver on top level navigation
Created attachment 188448 [details] Patch
abarth@ could you take a look at this? With this update, we are flushing the histogram on top level navigation, in addition to page destruction, in order to get closer to "per page" tracking. The new enum is for tracking number of pages visited (i.e. total number of histogram flushes). It is in a separate enum, because this statistic is also relevant to CSS tracking, which will be added to FeatureObserver in a separate patch. There is a place holder in the new enum due do a restriction in histogramEnumeration, which requires there to be 3 or more buckets.
Comment on attachment 188448 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=188448&action=review > Source/WebCore/loader/FrameLoader.cpp:3272 > + m_frame->page()->setFeatureObserver(adoptPtr(reinterpret_cast<FeatureObserver*>(0))); You can write: m_frame->page()->setFeatureObserver(nullptr) which is a bit cleaner. > Source/WebCore/page/FeatureObserver.cpp:45 > + HistogramSupport::histogramEnumeration("WebCore.FeatureObserver.TrackingStats", PageVisits, NumberOfStatistics); I'd prefer to use the same histogram for the baseline stats as for the other stats. That makes sure they stay in sync. > Source/WebCore/page/FeatureObserver.cpp:66 > + if (!page->featureObserver()) > + page->setFeatureObserver(adoptPtr(new FeatureObserver())); Rather than calling "new" and "delete" all the time, we can just trigger measurement on navigation and reset the m_featureBits.
Created attachment 188961 [details] Patch
@abarth I've uploaded a new patch with all the improvements you suggested: -simplification in FrameLoader.cpp -keeping pagevisit stats in existing enum for consistency -no longer calling 'new' Thank you!
Comment on attachment 188961 [details] Patch Looks great. Thanks!
Comment on attachment 188961 [details] Patch Clearing flags on attachment: 188961 Committed r143363: <http://trac.webkit.org/changeset/143363>
All reviewed patches have been landed. Closing bug.