Bug 89868 – Web Inspector: [Extensions API] Resource manipulations should be based on UISourceCode thus extending Sources Panel.

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

[Vsevolod Vlasov]

Patch to follow.

[Vsevolod Vlasov]

Created attachment 149717 [details]
Patch

[Pavel Feldman]

Comment on attachment 149717 [details]
Patch

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

> Source/WebCore/inspector/front-end/ExtensionServer.js:486
> +        var contentProvider = WebInspector.panels.scripts.uiSourceCodeForURL(url) || WebInspector.resourceForURL(url);

This is not nice. Could we move composite provider into the WebInspector and pass it into this class?

> Source/WebCore/inspector/front-end/ExtensionServer.js:509
> +            else

We don't use else after return branches

[Andrey Kosyakov]

Comment on attachment 149717 [details]
Patch

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

> Source/WebCore/inspector/front-end/ExtensionServer.js:510
> +                return this._status.E_FAILED("Resource is not editable")

I think E_NOTSUPPORTED() is more appropriate here.

> Source/WebCore/inspector/front-end/ExtensionServer.js:625
> +        var contentProvider = resource.uiSourceCode() || resource;

Can we extract this to something like ExtensionServer.contentProviderForURL()?

> Source/WebCore/inspector/front-end/StylesPanel.js:116
> +    _callOrSetTimeout: function(func)

func => callback? Also, please annotate.

> LayoutTests/http/tests/inspector/extensions-test.js:37
> +    WebInspector.StyleSource.updateTimeout = oldStyleSheetUpdateTimeout;

Why would we need this for all extension tests?

> LayoutTests/http/tests/inspector/extensions-test.js:54
> +    // Extensions tests override WebInspector.StyleSource.updateTimeout because otherwise extensions don't have any control over applying changes to domain specific bindings.
> +    var oldStyleSheetUpdateTimeout = WebInspector.StyleSource.updateTimeout;
> +    WebInspector.StyleSource.updateTimeout = -1;

As above, this should be done in tests that actually update resource content.

> LayoutTests/inspector/extensions/extensions-resources.html:36
> +            const resourceURLsWhiteList = ["abe.png", "audits-style1.css", "extensions-resources.html", "extensions-test.js", "inspector-test.js", "test-script.js"];

This looks a bit fishy -- can you please check we don't hit the case you're working around in test in real life, e.g. on reload or when we do not change renderer during navigation.

[Vsevolod Vlasov]

Committed r121792: <http://trac.webkit.org/changeset/121792>