| Summary: | Web Inspector: [Extensions API] support extensions for remote inspector front-end | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Andrey Kosyakov <caseq> | ||||||
| Component: | Web Inspector (Deprecated) | Assignee: | Andrey Kosyakov <caseq> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | apavlov, bweinstein, dglazkov, joepeck, keishi, loislo, pfeldman, pmuellr, rik, webkit.review.bot, yurys | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Attachments: |
|
||||||||
|
Description
Andrey Kosyakov
2011-09-09 04:13:36 PDT
Created attachment 106860 [details]
patch
Comment on attachment 106860 [details] patch Attachment 106860 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9625657 New failing tests: http/tests/inspector/console-resource-errors.html http/tests/inspector/console-xhr-logging.html http/tests/inspector/network/network-cachedresources-with-same-urls.html http/tests/inspector/console-cd.html http/tests/inspector/network/network-disable-cache-memory.html http/tests/inspector/resource-har-headers.html http/tests/inspector/network/network-disable-cache-xhrs.html http/tests/inspector/network/download.html http/tests/inspector-enabled/console-clear-arguments-on-frame-remove.html http/tests/inspector/inspect-iframe-from-different-domain.html http/tests/inspector-enabled/console-log-before-frame-navigation.html http/tests/inspector/network-preflight-options.html http/tests/inspector/network/network-content-replacement-xhr.html http/tests/inspector/console-cd-completions.html http/tests/inspector-enabled/dom-storage-open.html http/tests/inspector/resource-har-conversion.html http/tests/inspector/network/network-initiator-from-console.html http/tests/inspector/resource-parameters.html http/tests/inspector-enabled/database-open.html http/tests/inspector/change-iframe-src.html Comment on attachment 106860 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=106860&action=review > Source/WebCore/inspector/front-end/ExtensionServer.js:224 > + iframe.addEventListener("load", this._postExtensionAPI.bind(this), false); This effectively declares that remote and non-remote extensions should be implemented differently. As is, it will result in more confusion than benefit. > Source/WebCore/inspector/front-end/ExtensionServer.js:536 > + _addExtension: function(startPage, onload) I'd expose this method so that it could be called from the bookmarklet. Created attachment 107628 [details]
patch
- do not expose messaging interface, rather expose an interface to scripts being injected to our context
- expose devtools-extension-api.js as a separate file for chromium builds, so it is accessible for remote extensions via HTTP
Attachment 107628 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebKit/chromium/scripts/generate_devtools_extension_api.py:34: expected 2 blank lines, found 1 [pep8/E302] [5]
Source/WebKit/chromium/scripts/generate_devtools_extension_api.py:37: multiple statements on one line (semicolon) [pep8/E702] [5]
Source/WebKit/chromium/scripts/generate_devtools_extension_api.py:45: trailing whitespace [pep8/W291] [5]
Source/WebKit/chromium/scripts/generate_devtools_extension_api.py:55: multiple statements on one line (semicolon) [pep8/E702] [5]
Source/WebKit/chromium/scripts/generate_devtools_extension_api.py:57: expected 2 blank lines, found 1 [pep8/E302] [5]
Source/WebKit/chromium/scripts/generate_devtools_extension_api.py:66: multiple statements on one line (semicolon) [pep8/E702] [5]
Total errors found: 6 in 7 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 107628 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=107628&action=review > Source/WebCore/WebCore.gypi:6464 > + 'inspector/front-end/ExtensionAPI.js', Can we glue these together? They seem to be related. > Source/WebKit/chromium/scripts/generate_devtools_extension_api.py:34 > +def write_devtools_extension_api(output, input_names): I don't think you should diverge static API generation from dynamic API generation. > Source/WebKit/chromium/scripts/generate_devtools_extension_api.py:60 > + print('usage: %s output_js inpit_files_list ...' % argv[0]) input (In reply to comment #6) > (From update of attachment 107628 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=107628&action=review > > > Source/WebCore/WebCore.gypi:6464 > > + 'inspector/front-end/ExtensionAPI.js', > > Can we glue these together? They seem to be related. done! > > > Source/WebKit/chromium/scripts/generate_devtools_extension_api.py:34 > > +def write_devtools_extension_api(output, input_names): > > I don't think you should diverge static API generation from dynamic API generation. there are some inherent differences in the way we need the API for local and remote cases: in local case, we need script text to submit to back-end, in remote case we need the script that produces the API upon inclusion into the extension page; the identifier for the client is also passed/computed differently. So we won't be able to make this entirely uniform, though I've done a bit to minimize the difference. There's still some overlap between DevTools.js and generate_devtools_extension_api.py, but given this is a few lines, I'd rather keep it so, to avoid creating another file to reuse it. > > Source/WebKit/chromium/scripts/generate_devtools_extension_api.py:60 > > + print('usage: %s output_js inpit_files_list ...' % argv[0]) > > input fixed. Manually committed r95289: http://trac.webkit.org/changeset/95289 |