Summary: | [MutationObservers] Add histogram collection for usage of DOM Mutation Events | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Rafael Weinstein <rafaelw> | ||||||||||
Component: | DOM | Assignee: | Rafael Weinstein <rafaelw> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | abarth, adamk, darin, ojan, rniwa, sam, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 68729, 73026 | ||||||||||||
Attachments: |
|
Description
Rafael Weinstein
2011-11-14 15:15:51 PST
Created attachment 115041 [details]
Patch
Comment on attachment 115041 [details] Patch Attachment 115041 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/10485078 Comment on attachment 115041 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=115041&action=review > Source/WebCore/dom/Document.cpp:499 > +static void histogramMutationEventUsage(const unsigned short& listenerTypes) > +{ > + PlatformSupport::histogramEnumeration("DOMAPI.PerDocumentMutationEventUsage.DOMSubtreeModified", listenerTypes & Document::DOMSUBTREEMODIFIED_LISTENER, 2); I don't these functions are available on non-Chromium ports. You need to wrap it in #if PLATFORM(CHROMIUM) Created attachment 115048 [details]
Patch
Comment on attachment 115041 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=115041&action=review >> Source/WebCore/dom/Document.cpp:499 >> + PlatformSupport::histogramEnumeration("DOMAPI.PerDocumentMutationEventUsage.DOMSubtreeModified", listenerTypes & Document::DOMSUBTREEMODIFIED_LISTENER, 2); > > I don't these functions are available on non-Chromium ports. You need to wrap it in #if PLATFORM(CHROMIUM) ifdef'd Comment on attachment 115048 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=115048&action=review > Source/WebCore/dom/Document.cpp:118 > +#if PLATFORM(CHROMIUM) > +#include "PlatformSupport.h" > +#endif This should be listed separately see below for examples. Created attachment 115055 [details]
Patch
Comment on attachment 115048 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=115048&action=review >> Source/WebCore/dom/Document.cpp:118 >> +#endif > > This should be listed separately see below for examples. done Created attachment 115061 [details]
Patch
Sorry. I realized that I don't want to collect the bit in each of these cases, I actually want a 1 or a 0. Comment on attachment 115061 [details] Patch Clearing flags on attachment: 115061 Committed r100222: <http://trac.webkit.org/changeset/100222> All reviewed patches have been landed. Closing bug. It seems unfortunate adding platform specific code to Document.cpp. Is there another cross-platform way to do this? (In reply to comment #13) > It seems unfortunate adding platform specific code to Document.cpp. Is there another cross-platform way to do this? We could have a stub implementation of PlatformSupport.h for other platforms? This would be similar to the other *Client interfaces we have. I'm happy to do whatever you guys think is most appropriate. IMHO, we should have a header that exposes only the histogram functions which has stub implementations on non-Chromium platforms. The Chromium implementation would just trampoline through PlatformSupport. (In reply to comment #16) > IMHO, we should have a header that exposes only the histogram functions which has stub implementations on non-Chromium platforms. The Chromium implementation would just trampoline through PlatformSupport. I think this would be the right approach. opened 73026 for cleanup |