Bug 51334 – Web Inspector: Protocol cleanup task. Introduce Network, DOMStorage and Database domains.

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

[Ilya Tikhonovsky]

%subj% and other cleanup activities like:

Domain was changed for FileSystem related methods.
ResourcesManager was renamed to NetworkManager.
ResourceTreeModel was extracted as separate file.
FileSystem, Database, DOMStorage and ApplicationCache methods were removed from NetworkManager and added to corresponding classes.

Patch to follow.

[Ilya Tikhonovsky]

Created attachment 77000 [details]
[patch] initial version

[Ilya Tikhonovsky]

Created attachment 77023 [details]
[patch] initial version. rebaselined.

[Yury Semikhatsky]

Comment on attachment 77000 [details]
[patch] initial version

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

> LayoutTests/platform/chromium/test_expectations.txt:-630
> -BUG_DRT WIN DEBUG SLOW : inspector = PASS

I don't undertand how protocol clean up could affect these expectations.

> WebCore/inspector/front-end/Database.js:128
> +    delete WebInspector.Database.successCallbacks[transactionId];

Please remove both success and error callbacks when result is received.

> WebCore/inspector/front-end/Database.js:137
> +    delete WebInspector.Database.errorCallbacks[transactionId];

Ditto.

[Ilya Tikhonovsky]

Created attachment 77106 [details]
[patch] second iteration

Problem with callbacks in WebInspector.Database was fixed.
Unnecessary changes were removed from test_expectations.txt

[Pavel Feldman]

Comment on attachment 77106 [details]
[patch] second iteration

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

I don't think that ResourceManager -> NetworkManager rename is thought through. There are two things that do not play well with it: 1) frames aspect and 2) direct calls from network to various panels (something resource manager was created with). I think we should leave it as is unless there is a good separation of network resources and resource panel resources. I am fine with moving view factories into ResourceView.

> WebCore/inspector/front-end/NetworkManager.js:254
> +    frameDetachedFromParent: function(frameId)

"Network" manager should not be involved with frames.

> WebCore/inspector/front-end/NetworkManager.js:312
> +    _addFramesRecursively: function(framePayload)

Also, does not look good as a part of 'network'.

> WebCore/inspector/front-end/NetworkManager.js:352
> +        var resource = this.resourceForURL(msg.url);

What does console message have to do with network?

[Ilya Tikhonovsky]

(In reply to comment #5)
> (From update of attachment 77106 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=77106&action=review
> 
> I don't think that ResourceManager -> NetworkManager rename is thought through. There are two things that do not play well with it: 1) frames aspect and 2) direct calls from network to various panels (something resource manager was created with). I think we should leave it as is unless there is a good separation of network resources and resource panel resources. I am fine with moving view factories into ResourceView.

I'm not sure that ResourceManager is a good name for Network domain handler.

[Ilya Tikhonovsky]

Created attachment 77132 [details]
[patch] third iteration

Looks like old ResourceManager has two parts. 
The first one is NetworkManager which is the handler for Network domain messages.
The second is ResourceTreeModel which keeps control over resources.

As result console messages related functions were moved from NetworkManager to ResourceTreeModel.
resourceForURL method and _resourcesByURL map were moved to ResourceTreeModel too.
cachedResources related methods also moved to ResourceTreeModel.

[Pavel Feldman]

Comment on attachment 77132 [details]
[patch] third iteration

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

r+ with comment. Please fix it before landing.

> WebCore/inspector/front-end/ResourceTreeModel.js:245
> +    frameDetachedFromParent: function(frameId)

This is a bug.

[Ilya Tikhonovsky]

Comment on attachment 77132 [details]
[patch] third iteration

Committed r74709
	M	WebCore/ChangeLog
	M	WebCore/WebCore.vcproj/WebCore.vcproj
	M	WebCore/WebCore.gypi
	M	WebCore/inspector/CodeGeneratorInspector.pm
	M	WebCore/inspector/Inspector.idl
	D	WebCore/inspector/front-end/ResourceManager.js
	M	WebCore/inspector/front-end/ConsoleView.js
	M	WebCore/inspector/front-end/ScriptsPanel.js
	M	WebCore/inspector/front-end/FileSystemView.js
	M	WebCore/inspector/front-end/DOMAgent.js
	M	WebCore/inspector/front-end/NetworkItemView.js
	M	WebCore/inspector/front-end/inspector.js
	M	WebCore/inspector/front-end/CSSStyleModel.js
	M	WebCore/inspector/front-end/Database.js
	M	WebCore/inspector/front-end/DOMStorage.js
	M	WebCore/inspector/front-end/NetworkPanel.js
	A	WebCore/inspector/front-end/NetworkManager.js
	M	WebCore/inspector/front-end/ResourceView.js
	M	WebCore/inspector/front-end/ResourcesPanel.js
	M	WebCore/inspector/front-end/Resource.js
	M	WebCore/inspector/front-end/inspector.html
	M	WebCore/inspector/front-end/AuditRules.js
	A	WebCore/inspector/front-end/ResourceTreeModel.js
	M	WebCore/inspector/front-end/WebKit.qrc
W: -empty_dir: trunk/WebCore/inspector/front-end/ResourceManager.js
r74709 = ccc4092102558078160dc04d059eaae108a49e51 (refs/remotes/trunk)

[WebKit Review Bot]

http://trac.webkit.org/changeset/74709 might have broken Qt Linux Release
The following tests are not passing:
http/tests/inspector-enabled/console-log-before-frame-navigation.html
http/tests/inspector/change-iframe-src.html
http/tests/inspector/console-resource-errors.html
http/tests/inspector/console-xhr-logging.html
http/tests/inspector/extensions-headers.html
http/tests/inspector/inspect-iframe-from-different-domain.html
http/tests/inspector/resource-parameters.html
inspector/console-assert.html
inspector/console-clear.html
inspector/console-command-clear.html
inspector/console-dir-global.html
inspector/console-dir.html
inspector/console-dirxml.html
inspector/console-eval-global.html
inspector/console-eval.html
inspector/console-format-collections.html
inspector/console-format.html
inspector/console-log-before-inspector-open.html
inspector/console-log-syntax-error.html
inspector/console-object-constructor-name.html
inspector/console-tests.html
inspector/console-trace-in-eval.html
inspector/console-trace.html
inspector/console-uncaught-exception-in-eval.html
inspector/console-uncaught-exception.html
inspector/cookie-parser.html
inspector/cookie-resource-match.html
inspector/debugger-autocontinue-on-syntax-error.html
inspector/debugger-suspend-active-dom-objects.html
inspector/elements-delete-inline-style.html
inspector/elements-img-tooltip.html
inspector/elements-panel-limited-children.html
inspector/elements-panel-rewrite-href.html
inspector/elements-panel-search.html
inspector/elements-panel-selection-on-refresh.html
inspector/elements-panel-styles.html
inspector/elements-panel-xhtml-structure.xhtml
inspector/evaluate-in-frontend.html
inspector/extensions-api.html
inspector/extensions-audits-api.html
inspector/extensions-audits.html
inspector/extensions-eval.html
inspector/extensions-events.html
inspector/extensions-resources.html
inspector/inspected-objects-not-overriden.html
inspector/report-API-errors.html
inspector/report-protocol-errors.html
inspector/storage-panel-dom-storage.html
inspector/styles-add-blank-property.html
inspector/styles-computed-trace.html
inspector/styles-disable-inherited.html
inspector/styles-disable-then-delete.html
inspector/styles-disable-then-enable.html
inspector/styles-iframe.html
inspector/styles-new-API.html
inspector/styles-source-lines-inline.html
inspector/styles-source-lines.html
inspector/styles-source-offsets.html
inspector/syntax-highlight-css.html
inspector/syntax-highlight-html.html
inspector/syntax-highlight-javascript.html
inspector/timeline-enum-stability.html
inspector/timeline-event-dispatch.html
inspector/timeline-mark-timeline.html
inspector/timeline-network-resource.html
inspector/timeline-script-tag-1.html
inspector/timeline-script-tag-2.html
inspector/timeline-trivial.html