RESOLVED FIXED 70600
Web Inspector: [Extensions API] allow extensions to specify script to be injected on reload
https://bugs.webkit.org/show_bug.cgi?id=70600
Summary Web Inspector: [Extensions API] allow extensions to specify script to be inje...
Andrey Kosyakov
Reported 2011-10-21 04:43:18 PDT
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.
Attachments
patch (17.74 KB, patch)
2011-10-21 04:45 PDT, Andrey Kosyakov
no flags
patch (fixed style) (17.74 KB, patch)
2011-10-21 04:49 PDT, Andrey Kosyakov
pfeldman: review-
patch (17.19 KB, patch)
2011-10-24 09:32 PDT, Andrey Kosyakov
pfeldman: review-
patch (16.95 KB, patch)
2011-10-24 09:55 PDT, Andrey Kosyakov
pfeldman: review+
Andrey Kosyakov
Comment 1 2011-10-21 04:45:27 PDT
WebKit Review Bot
Comment 2 2011-10-21 04:47:21 PDT
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
Comment 3 2011-10-21 04:49:24 PDT
Created attachment 111944 [details] patch (fixed style)
Pavel Feldman
Comment 4 2011-10-21 05:46:54 PDT
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
Comment 5 2011-10-24 09:32:38 PDT
Andrey Kosyakov
Comment 6 2011-10-24 09:35:07 PDT
(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 7 2011-10-24 09:43:39 PDT
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
Comment 8 2011-10-24 09:55:42 PDT
Created attachment 112204 [details] patch Perform script injection only in case there's an active front-end.
Andrey Kosyakov
Comment 9 2011-10-24 09:59:25 PDT
(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 10 2011-10-24 10:12:25 PDT
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
Comment 11 2011-10-25 05:59:41 PDT
Note You need to log in before you can comment on or make changes to this bug.