[webkit-reviews] review denied: [Bug 74221] Web Inspector: consider disabling network tracking while running the CPU profile : [Attachment 119153] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Ilya Tikhonovsky
<loislo at chromium.org>'s request for review:
Bug 74221: Web Inspector: consider disabling network tracking while running the
CPU profile
https://bugs.webkit.org/show_bug.cgi?id=74221

Attachment 119153: Patch
https://bugs.webkit.org/attachment.cgi?id=119153&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=119153&action=review


> Source/WebCore/ChangeLog:7
> +	   As the result the CPU profiler's stats data are far away from the
reality.

Not that far, just skewed.

> Source/WebCore/inspector/front-end/NetworkManager.js:86
> +		   return;

Can this happen?

> Source/WebCore/inspector/front-end/NetworkManager.js:102
> +		   return;

ditto

> Source/WebCore/inspector/front-end/ProfileView.js:591
>	       ProfilerAgent.start();

You should proceed with the profiling from within the callback.

> Source/WebCore/inspector/front-end/ProfileView.js:594
> +	       WebInspector.networkManager.enableResourceTracking();

ditto

> Source/WebCore/inspector/front-end/ResourceTreeModel.js:50
> +    networkManager.enableResourceTracking();

You already enable resource tracking from the network manager constructor.

> Source/WebCore/inspector/front-end/ResourceTreeModel.js:51
> +    this._resourceTrackingWasDisabled = false;

There is probably no need to mirror the network manager state here.