Summary: | TimelineAgent should not add events to the timeline if there are no event listeners. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kelly Norton <knorton> | ||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, pfeldman, timothy | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | OS X 10.5 | ||||||||||
Attachments: |
|
Description
Kelly Norton
2009-11-01 09:37:14 PST
Created attachment 42278 [details]
A proposed solution for this.
Comment on attachment 42278 [details] A proposed solution for this. > +// static We normally don't add comments like this. > +inline InspectorTimelineAgent* InspectorTimelineAgent::retrieveIfEventHasListeners(InspectorTimelineAgent* timelineAgent, > + const AtomicString& eventType, Node* node, const Vector< RefPtr< ContainerNode > >& ancestors) Will this be used by other callers later? If not it might be best just to put this in Node.cpp so there are less headers included by InspectorTimelineAgent.h. I can see a variation of this being used by XHRs (we don't want to show readystate or load records if there are no listeners.) > + for (size_t i = ancestors.size(); i; --i) { > + ContainerNode* ancestor = ancestors[i - 1].get(); > + if (ancestor->hasEventListeners(eventType)) > + return timelineAgent; > + } This is better written as: for (ssize_t i = (ancestors.size() - 1); i >= 0; --i) { ContainerNode* ancestor = ancestors[i].get(); if (ancestor->hasEventListeners(eventType)) return timelineAgent; } Created attachment 42287 [details]
Updated w/ review feedback and went ahead and added the same thing to XHRs.
Created attachment 42288 [details]
Fixes ChangeLog
Comment on attachment 42288 [details] Fixes ChangeLog Clearing flags on attachment: 42288 Committed r50400: <http://trac.webkit.org/changeset/50400> All reviewed patches have been landed. Closing bug. |