Bug 52588 - Web Inspector: [refactoring] use events to notify NetworkManager clients
Summary: Web Inspector: [refactoring] use events to notify NetworkManager clients
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Andrey Kosyakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-17 10:30 PST by Andrey Kosyakov
Modified: 2011-01-19 23:44 PST (History)
10 users (show)

See Also:

Attachments
patch (18.78 KB, patch)
2011-01-17 12:35 PST, Andrey Kosyakov 		yurys: review+
yurys: commit-queue-
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Kosyakov 2011-01-17 10:30:31 PST 
Currently, NetworkManager directly calls each client to notify it on resource start/update/finish. This should be done via generic event dispatch to decouple higher-level code from NetworkManager.
Comment 1 Andrey Kosyakov 2011-01-17 12:35:07 PST 
Created attachment 79204 [details]
patch
Comment 2 Yury Semikhatsky 2011-01-18 04:05:09 PST 
Comment on attachment 79204 [details]
patch

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

> Source/WebCore/ChangeLog:9
> +        (WebInspector.AuditLauncherView): Do not expose public interface to resource count. Do not c reset resource count upon reset().

typo: Do not __c__ reset
Comment 3 Pavel Feldman 2011-01-18 04:23:02 PST 
Comment on attachment 79204 [details]
patch

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

> Source/WebCore/inspector/front-end/NetworkManager.js:132
> +            this.dispatchEventToListeners(WebInspector.NetworkManager.EventTypes.ResourceUpdated, resource);

nit: _fireResourceUpdated(resource);

> Source/WebCore/inspector/front-end/NetworkManager.js:218
> +        this.dispatchEventToListeners(WebInspector.NetworkManager.EventTypes.ResourceContentUpdated, resource);

We only need this to remove XHRs, so ResourceUpdated should be sufficient.

> Source/WebCore/inspector/front-end/NetworkManager.js:229
> +                this.dispatchEventToListeners(WebInspector.NetworkManager.EventTypes.MainResourceChanged, mainResource);

MainResourceChanged sounds weird. How about MainResourceCommitLoad?

> Source/WebCore/inspector/front-end/NetworkPanel.js:84
> +    WebInspector.networkManager.addEventListener(WebInspector.NetworkManager.EventTypes.ResourceContentUpdated, this._onResourceUpdated, this);

No need to listen to content update.

> Source/WebCore/inspector/front-end/NetworkPanel.js:1438
> +            this._resource.populateImageSource(previewImage);

This will get merged hopefully.
Comment 4 Andrey Kosyakov 2011-01-19 23:44:35 PST 
Manually committed r76022: http://trac.webkit.org/changeset/76022