Bug 109874 – Update FeatureObserver on top level navigation

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Kassy Coan]

Update FeatureObserver on top level navigation

[Kassy Coan]

Created attachment 188448 [details]
Patch

[Kassy Coan]

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.

[Adam Barth]

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.

[Kassy Coan]

Created attachment 188961 [details]
Patch

[Kassy Coan]

@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!

[Adam Barth]

Comment on attachment 188961 [details]
Patch

Looks great.  Thanks!

[WebKit Review Bot]

Comment on attachment 188961 [details]
Patch

Clearing flags on attachment: 188961

Committed r143363: <http://trac.webkit.org/changeset/143363>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.