Web Inspector: [Extensions API] allow extensions to specify script to be injected on reload

[Andrey Kosyakov]

This changes webInspector.inspectedWindow.reload() to accept an options object that lets caller to specify a script to inject upon reload in addition to user agent string. The old interface is supported, but passing string instead of object causes deprecation warning to be printed.

[Andrey Kosyakov]

Created attachment 111941 [details]
patch

[WebKit Review Bot]

Attachment 111941 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1

Source/WebCore/inspector/InspectorPageAgent.h:67:  This { should be at the end of the previous line  [whitespace/braces] [4]
Source/WebCore/inspector/InspectorPageAgent.h:85:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 2 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Andrey Kosyakov]

Created attachment 111944 [details]
patch (fixed style)

[Pavel Feldman]

Comment on attachment 111944 [details]
patch (fixed style)

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

> Source/WebCore/inspector/Inspector.json:164
> +                    { "name": "removeOnReload", "type": "boolean", "optional": true }

"persistent" + docs?

> Source/WebCore/inspector/front-end/ExtensionAPI.js:497
> +        else if (typeof optionsOrUserAgent === "string") {

This will never compile.

> Source/WebCore/inspector/front-end/WorkersSidebarPane.js:81
> +        if (!enabled === !this._fakeWorkersScriptIdentifier)

Could you disable checkbox instead?

[Andrey Kosyakov]

Created attachment 112200 [details]
patch

[Andrey Kosyakov]

(In reply to comment #4)

> > Source/WebCore/inspector/Inspector.json:164
> > +                    { "name": "removeOnReload", "type": "boolean", "optional": true }
> 
> "persistent" + docs?

Dropped this altogether -- instead, we expose scriptToEvaluateOnLoad as a parameter in Page.reload().

> > Source/WebCore/inspector/front-end/ExtensionAPI.js:497
> > +        else if (typeof optionsOrUserAgent === "string") {
> 
> This will never compile.

There are quite a few things in ExtensionAPI.js that won't compile. We need this as a compatibility hack, this will be gone in the future.

> > Source/WebCore/inspector/front-end/WorkersSidebarPane.js:81
> > +        if (!enabled === !this._fakeWorkersScriptIdentifier)
> 
> Could you disable checkbox instead?

Done.

[Pavel Feldman]

Comment on attachment 112200 [details]
patch

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

> Source/WebCore/inspector/InspectorPageAgent.cpp:342
> +    m_pendingScriptToEvaluateOnLoadOnce = optionalScriptToEvaluateOnLoad ? *optionalScriptToEvaluateOnLoad : "";

You don't need this pending script.

> Source/WebCore/inspector/InspectorPageAgent.cpp:584
> +    ScriptState* scriptState = 0; // Initialize lazily, as mainWorldScriptState() will create one if missing.

Early return in case of no front-end?

[Andrey Kosyakov]

Created attachment 112204 [details]
patch

Perform script injection only in case there's an active front-end.

[Andrey Kosyakov]

(In reply to comment #7)
> (From update of attachment 112200 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=112200&action=review
> 
> > Source/WebCore/inspector/InspectorPageAgent.cpp:342
> > +    m_pendingScriptToEvaluateOnLoadOnce = optionalScriptToEvaluateOnLoad ? *optionalScriptToEvaluateOnLoad : "";
> 
> You don't need this pending script.

I do need it: the script is only supposed to live during one reload cycle and we're clearing it upion main frame navigation, which happens between reload() and didClearWindowObject(), where the script is used.

> > Source/WebCore/inspector/InspectorPageAgent.cpp:584
> > +    ScriptState* scriptState = 0; // Initialize lazily, as mainWorldScriptState() will create one if missing.
> 
> Early return in case of no front-end?

Fixed.

[Pavel Feldman]

Comment on attachment 112204 [details]
patch

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

> Source/WebCore/inspector/InspectorPageAgent.h:58
> +class ScriptToEvaluateOnLoad;

Please remove.

> Source/WebCore/inspector/InspectorPageAgent.h:66
> +public:

Why this change?

[Andrey Kosyakov]

Manually committed r98328: as http://trac.webkit.org/changeset/98328