Bug 215793 – Web Inspector: allow url breakpoints to be configured

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Devin Rousso]

This would allow developers to do things like:
 - ignore the first N requests
 - evaluate JavaScript whenever a network request is dispatched without pausing

[Devin Rousso]

Created attachment 407544 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/68118986>

[Blaze Burg]

Comment on attachment 407544 [details]
Patch

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

r=me, great cleanup!

Please rebase and run through EWS prior to landing.

> LayoutTests/inspector/debugger/resources/breakpoint-options-utilities.js:56
> +                await resumePromise;

Oops.

> LayoutTests/inspector/dom-debugger/url-breakpoints-all-requests-expected.txt:14
> +Adding "text" URL Breakpoint...

Uh, why "text"?

> LayoutTests/inspector/dom-debugger/url-breakpoints-all-requests-expected.txt:143
> +-- Running test case: URLBreakpoint.BreakOnAllXHR..Options.Condition

Nit: Extra ..

> LayoutTests/inspector/dom-debugger/url-breakpoints-all-requests-expected.txt:187
> +-- Running test case: URLBreakpoint.BreakOnAllXHR..Options.Action.Log

Ditto

> LayoutTests/inspector/dom-debugger/url-breakpoints-all-requests-expected.txt:219
> +-- Running test case: URLBreakpoint.BreakOnAllXHR..Options.Actions.Evaluate

Ditto.

> Source/JavaScriptCore/ChangeLog:9
> +        Add an `options` paramater to `DOMDebugger.setURLBreakpoint` to allow configuration.

Nit: parameter

> Source/WebInspectorUI/UserInterface/Controllers/DOMDebuggerManager.js:725
> +        if (target.hasCommand("DOMDebugger.setXHRBreakpoint")) {

Nit: comment and the command name don't match. Please add separate COMPATIBILITY for setXHRBreakpoint and setURLBreakpoint.

> Source/WebInspectorUI/UserInterface/Controllers/DOMDebuggerManager.js:734
> +        if (!target.hasCommand("DOMDebugger.setURLBreakpoint"))

.. down here.

> Source/WebInspectorUI/UserInterface/Controllers/DOMDebuggerManager.js:750
> +        if (target.hasCommand("DOMDebugger.removeXHRBreakpoint")) {

Ditto to the above COMPATIBILITY note.

[Devin Rousso]

Comment on attachment 407544 [details]
Patch

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

>> LayoutTests/inspector/dom-debugger/url-breakpoints-all-requests-expected.txt:14
>> +Adding "text" URL Breakpoint...
> 
> Uh, why "text"?

The All Requests breakpoint is a text URL breakpoint (i.e. not regular expression) that has no URL.  I'll special case that in the logging :)

>> Source/WebInspectorUI/UserInterface/Controllers/DOMDebuggerManager.js:725
>> +        if (target.hasCommand("DOMDebugger.setXHRBreakpoint")) {
> 
> Nit: comment and the command name don't match. Please add separate COMPATIBILITY for setXHRBreakpoint and setURLBreakpoint.

The compatibility comment here is that `setXHRBreakpoint` is what existed before `setURLBreakpoint` replaced it (when `fetch` support was added).

I agree that this is confusing tho, so I'll add separate comments for each :)

[Devin Rousso]

Created attachment 407849 [details]
Patch

Also ensure that modifying the options of a `WI.EventBreakpoint` doesn't enable breakpoints globally.  This matches what is done with `WI.JavaScriptBreakpoint`.

[EWS Watchlist]

This patch modifies the inspector protocol. Please ensure that any frontend changes appropriately use feature checks for new protocol features.

[EWS]

Committed r266538: <https://trac.webkit.org/changeset/266538>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 407849 [details].