Bug 73026 – [MutationObservers] Remove platform-dependent code in Document.cpp resulting from Mutation Event histogram collection

• Component: DOM
• Status: RESOLVED FIXED

[Rafael Weinstein]

This is cleanup from bug 72316

[Rafael Weinstein]

Created attachment 119665 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 119665 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=119665&action=review

> Source/WebCore/ChangeLog:11
> +        No tests need. This patch is just a refactor. (OOPS!)

You need to remove (OOPS!).

> Source/WebCore/platform/chromium/HistogramSupportChromium.cpp:41
> +void HistogramSupport::histogramEnumeration(const char* name, int sample, int boundaryValue)
> +{
> +    PlatformSupport::histogramEnumeration(name, sample, boundaryValue);
> +}

Is it really okay to add an extra function call? Can't we make this function inline?

[Rafael Weinstein]

This only gets called in the document dtor.

[Rafael Weinstein]

Comment on attachment 119665 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=119665&action=review

>> Source/WebCore/ChangeLog:11
>> +        No tests need. This patch is just a refactor. (OOPS!)
> 
> You need to remove (OOPS!).

oops.

[Adam Barth]

Comment on attachment 119665 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=119665&action=review

> Source/WebCore/platform/HistogramSupport.h:42
> +#if PLATFORM(CHROMIUM)
> +    static void histogramEnumeration(const char* name, int sample, int boundaryValue);
> +#else
> +    static void histogramEnumeration(const char*, int, int) { }
> +#endif

This header shouldn't have an ifdef.  We should have a HistogramSupportNone.cpp with the stub in it.

[Ryosuke Niwa]

Comment on attachment 119665 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=119665&action=review

> Source/WebCore/platform/HistogramSupport.h:47
> +#endif // FileSystem_h

Wrong comment.

[Rafael Weinstein]

Created attachment 119682 [details]
Patch

[WebKit Review Bot]

Comment on attachment 119682 [details]
Patch

Clearing flags on attachment: 119682

Committed r103131: <http://trac.webkit.org/changeset/103131>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.