Bug 67840

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 Flags
patch
pfeldman: review-, webkit.review.bot: commit-queue-
patch pfeldman: review+

Description Andrey Kosyakov 2011-09-09 04:13:36 PDT
This exposed a DOM-messaging based interface for a content script to request an extension to be added to the inspector front-end and receive extension API code.
Comment 1 Andrey Kosyakov 2011-09-09 04:16:11 PDT
Created attachment 106860 [details]
patch
Comment 2 WebKit Review Bot 2011-09-09 09:16:20 PDT
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 3 Pavel Feldman 2011-09-14 06:46:46 PDT
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.
Comment 4 Andrey Kosyakov 2011-09-16 02:29:35 PDT
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
Comment 5 WebKit Review Bot 2011-09-16 02:32:32 PDT
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 6 Pavel Feldman 2011-09-16 02:47:12 PDT
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
Comment 7 Andrey Kosyakov 2011-09-16 08:51:00 PDT
(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.
Comment 8 Andrey Kosyakov 2011-09-16 08:51:39 PDT
Manually committed r95289: http://trac.webkit.org/changeset/95289