Bug 45887 – Web Inspector: move resourceTracking flag under control of BackendSettings

• Status: RESOLVED
• Resolution: WONTFIX
• Component: Web Inspector (Deprecated)

[Ilya Tikhonovsky]

Four Inspector API methods about resourceTracking flag were removed.
Actual state of the flag will be transfered as response for setResourceTracking.
Initial state of the flag on the frontend side will be obtained from settings.backend.

[Ilya Tikhonovsky]

Created attachment 67794 [details]
[patch] initial version.

[WebKit Review Bot]

Attachment 67794 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/inspector/InspectorController.cpp:1115:  Missing space before ( in if(  [whitespace/parens] [5]
WebCore/inspector/InspectorController.cpp:1119:  An else should appear on the same line as the preceding }  [whitespace/newline] [4]
Total errors found: 2 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Yury Semikhatsky]

Comment on attachment 67794 [details]
[patch] initial version.

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

> LayoutTests/http/tests/inspector/inspector-test2.js:103
> +    WebInspector.panels.resources._toggleResourceTracking();

Please check that current state is "enabled"

> LayoutTests/http/tests/inspector/resource-parameters.html:-36
> -        InspectorBackend.enableResourceTracking(false);

This test should be refactored to use inspector-test2.js It can be done in a separate change though.

> WebCore/inspector/InspectorController.cpp:1128
> +    if (m_resourceTrackingEnabled == enable)

I think we may safely remove this if.

> WebKit/chromium/src/WebDevToolsAgentImpl.cpp:261
> +        ic->setResourceTracking(value == "true" ? true : false);

May be just value == "true"

[Ilya Tikhonovsky]

Comments addressed.


Committed r67647
	M	WebKit/chromium/ChangeLog
	M	WebKit/chromium/src/WebDevToolsAgentImpl.cpp
	M	WebCore/ChangeLog
	M	WebCore/inspector/InspectorController.cpp
	M	WebCore/inspector/Inspector.idl
	M	WebCore/inspector/front-end/ResourcesPanel.js
	M	WebCore/inspector/front-end/Settings.js
	M	WebCore/inspector/front-end/inspector.js
	M	WebCore/inspector/InspectorController.h
	M	LayoutTests/http/tests/inspector/inspector-test2.js
	M	LayoutTests/http/tests/inspector/resource-parameters.html
	M	LayoutTests/inspector/report-API-errors-expected.txt
	M	LayoutTests/inspector/styles-source-offsets.html
	M	LayoutTests/inspector/report-API-errors.html
	M	LayoutTests/inspector/audits-panel-functional.html
	M	LayoutTests/ChangeLog
r67647 = d230d0b077360e0c3991f9db1d7bd97df6f12bae (refs/remotes/trunk)

[Pavel Feldman]

Comment on attachment 67794 [details]
[patch] initial version.

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

> WebCore/inspector/InspectorController.cpp:1102
> +void InspectorController::setResourceTracking(bool enable, bool always, bool* newState)

setResourceTrackingEnabled

> WebCore/inspector/InspectorController.h:173
> +    void setResourceTracking(bool enable);

setResourceTrackingEnabled

> WebCore/inspector/front-end/ResourcesPanel.js:758
> +                WebInspector.panels.resources.resourceTrackingWasEnabled();

No need to access this via WebInspector.panels.resources. Just bing the function.

> WebCore/inspector/front-end/Settings.js:90
> +        if (settings.resourceTrackingEnabled)

Settings should not be resources aware. I'd r- for this.

[Pavel Feldman]

There is no resource tracking any more.