Bug 30995 - TimelineAgent should not add events to the timeline if there are no event listeners.
Summary: TimelineAgent should not add events to the timeline if there are no event lis...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-01 09:37 PST by Kelly Norton
Modified: 2009-11-01 19:23 PST (History)
3 users (show)

See Also:


Attachments
A proposed solution for this. (3.97 KB, patch)
2009-11-01 09:39 PST, Kelly Norton
timothy: review-
Details | Formatted Diff | Diff
Updated w/ review feedback and went ahead and added the same thing to XHRs. (4.38 KB, patch)
2009-11-01 17:56 PST, Kelly Norton
no flags Details | Formatted Diff | Diff
Fixes ChangeLog (4.40 KB, patch)
2009-11-01 18:03 PST, Kelly Norton
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kelly Norton 2009-11-01 09:37:14 PST
TimelineAgent currently sends all DOM event dispatches even if there are no event listeners wired to the Node or its ancestors.
(attaching patch)
Comment 1 Kelly Norton 2009-11-01 09:39:21 PST
Created attachment 42278 [details]
A proposed solution for this.
Comment 2 Timothy Hatcher 2009-11-01 11:02:02 PST
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;
    }
Comment 3 Kelly Norton 2009-11-01 17:56:32 PST
Created attachment 42287 [details]
Updated w/ review feedback and went ahead and added the same thing to XHRs.
Comment 4 Kelly Norton 2009-11-01 18:03:52 PST
Created attachment 42288 [details]
Fixes ChangeLog
Comment 5 WebKit Commit Bot 2009-11-01 19:23:26 PST
Comment on attachment 42288 [details]
Fixes ChangeLog

Clearing flags on attachment: 42288

Committed r50400: <http://trac.webkit.org/changeset/50400>
Comment 6 WebKit Commit Bot 2009-11-01 19:23:30 PST
All reviewed patches have been landed.  Closing bug.