Bug 89868

Created attachment 149717 [details] Patch

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

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.

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