Bug 200652 – Web Inspector: Debugger: add a global breakpoint for pausing in the next microtask

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

[Devin Rousso]

This would be similar to the "All Animation Frames"/"All Timeouts"/"All Intervals"/"All Events" breakpoints, except that it would also work for JSContext inspection.

[Devin Rousso]

Created attachment 376667 [details]
Patch

[EWS Watchlist]

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

[Devin Rousso]

Created attachment 376671 [details]
[Image] After Patch is applied

[Joseph Pecoraro]

Comment on attachment 376667 [details]
Patch

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

Nice! Good tests. r=me

Seems like perhaps the only remaining script entry points (that I know of) are:

  (1) API Entry points
    - Ex. JSEvaluteScript / -[JSContext evaluteScript:]
  (2) Normal Script entry points for a Program / Module
    - Ex. <script src="..." async>
    - Ex. eval('...')

  (3) Observer Script handlers
    - Ex. PerformanceObserver, IntersectionObserver, handlers
  (4) APIs with callbacks that aren't events or Observers
    - Ex. geolocation callbacks


(1) and (2) seem useful for users to break on. Maybe a Breakpoint on script entry "All Script Entries" / "All Script Evaluations" would be good.

(3) and (4) cases seem like Events / specific functions akin to setTimeout. Seems overkill to call them out specifically though.

Anything I missed worth considering?

> Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js:152
> +        // COMPATIBILITY (iOS 12): DebuggerAgent.setPauseOnMicrotasks did not exist yet.

Nit: I think we should use (iOS 13) in all of these compatibility comments, as it did not exist in iOS 13.0.

> Source/WebInspectorUI/UserInterface/Images/Microtask.svg:3
> +<svg xmlns="http://www.w3.org/2000/svg" id="root" version="1.1" viewBox="0 0 16 16">

Sometimes we have `id="root"` and other times we don't. Not sure why... I normally remove it if I don't see a need for it.

[Devin Rousso]

Comment on attachment 376667 [details]
Patch

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

> Seems like perhaps the only remaining script entry points (that I know of) are:
> 
>   (1) API Entry points
>     - Ex. JSEvaluteScript / -[JSContext evaluteScript:]
>   (2) Normal Script entry points for a Program / Module
>     - Ex. <script src="..." async>
>     - Ex. eval('...')
> 
>   (3) Observer Script handlers
>     - Ex. PerformanceObserver, IntersectionObserver, handlers
>   (4) APIs with callbacks that aren't events or Observers
>     - Ex. geolocation callbacks
> 
> 
> (1) and (2) seem useful for users to break on. Maybe a Breakpoint on script entry "All Script Entries" / "All Script Evaluations" would be good.
I like it!  I see this being useful along the lines (somewhat) of the Inspector Bootstrap Script concept <https://webkit.org/b/195847>.

> (3) and (4) cases seem like Events / specific functions akin to setTimeout. Seems overkill to call them out specifically though.
Perhaps we should consider these as microtasks.  It looks like `MutationObserver` actually uses a form of microtask, whereas `IntersectionObserver` uses a timer and `PerformanceObserver` uses a task queue (run loop).  I'm not against the idea of creating a "Observer Callback..." breakpoint that can be customized by observer type (like "Event Breakpoint..."), but if we create a "All Script Entries" breakpoint then these would likely be covered by that as well (microtasks and events too).

> Anything I missed worth considering?
I feel like with the addition of the "All Microtasks" breakpoint, all of the various ways to enter script (other than (1) and (2) above) are covered as part of one of the "categories" we already have breakpoints for (e.g. events, timers, etc).  I think the next useful thing to do is to provide breakpoint actions for all of the non-JavaScript breakpoints, as well as getting full stack trace information (where applicable).

I also have been pondering the idea of extending URL breakpoints to work for _any_ situation where network activity is triggered by script (e.g. `img.src = "..."`).

>> Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js:152
>> +        // COMPATIBILITY (iOS 12): DebuggerAgent.setPauseOnMicrotasks did not exist yet.
> 
> Nit: I think we should use (iOS 13) in all of these compatibility comments, as it did not exist in iOS 13.0.

LOL 😂 bad copypasta.  Nice catch!

>> Source/WebInspectorUI/UserInterface/Images/Microtask.svg:3
>> +<svg xmlns="http://www.w3.org/2000/svg" id="root" version="1.1" viewBox="0 0 16 16">
> 
> Sometimes we have `id="root"` and other times we don't. Not sure why... I normally remove it if I don't see a need for it.

This is needed by `WI.ImageUtilities.useSVGSymbol`.
```
    // URL must contain a fragment reference to a graphical element, like a symbol. If none is given
    // append #root which all of our SVGs have on the top level <svg> element.
    if (!url.includes("#"))
        url += "#root";
```

[Devin Rousso]

Created attachment 376748 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 376748 [details]
Patch

Rejecting attachment 376748 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'apply-attachment', '--no-update', '--non-interactive', 376748, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Logging in as [e-mail]...
Fetching: https://bugs.webkit.org/attachment.cgi?id=376748&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=200652&ctype=xml&excludefield=attachmentdata
Processing 1 patch from 1 bug.
Processing patch 376748 from bug 200652.
Fetching: https://bugs.webkit.org/attachment.cgi?id=376748
Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Parsed 25 diffs from patch file(s).
patching file Source/JavaScriptCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebInspectorUI/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/JavaScriptCore/debugger/Debugger.h
patching file Source/JavaScriptCore/inspector/ScriptDebugListener.h
patching file Source/JavaScriptCore/inspector/ScriptDebugServer.cpp
patching file Source/JavaScriptCore/inspector/ScriptDebugServer.h
patching file Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp
patching file Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.h
patching file Source/JavaScriptCore/inspector/agents/JSGlobalObjectDebuggerAgent.h
patching file Source/JavaScriptCore/inspector/protocol/Debugger.json
patching file Source/JavaScriptCore/runtime/JSMicrotask.cpp
patching file Source/WebCore/inspector/agents/InspectorTimelineAgent.h
patching file Source/WebCore/inspector/agents/page/PageDebuggerAgent.h
patching file Source/WebCore/inspector/agents/worker/WorkerDebuggerAgent.h
patching file Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js
Hunk #2 succeeded at 692 (offset 1 line).
patching file Source/WebInspectorUI/UserInterface/Base/Setting.js
Hunk #1 FAILED at 162.
1 out of 1 hunk FAILED -- saving rejects to file Source/WebInspectorUI/UserInterface/Base/Setting.js.rej
patching file Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js
patching file Source/WebInspectorUI/UserInterface/Images/Microtask.svg
patching file Source/WebInspectorUI/UserInterface/Views/BreakpointTreeElement.css
patching file Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js
Hunk #5 succeeded at 1131 (offset 14 lines).
Hunk #6 succeeded at 1387 (offset 14 lines).
Hunk #7 succeeded at 1569 (offset 14 lines).
Hunk #8 succeeded at 1613 (offset 14 lines).
Hunk #9 succeeded at 1641 (offset 14 lines).
Hunk #10 succeeded at 1654 (offset 14 lines).
Hunk #11 succeeded at 1685 (offset 14 lines).
patching file Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js
Hunk #1 succeeded at 328 (offset 11 lines).
Hunk #2 succeeded at 918 (offset 11 lines).
Hunk #3 succeeded at 993 (offset 11 lines).
Hunk #4 succeeded at 1413 (offset 11 lines).
Hunk #5 succeeded at 1603 (offset 11 lines).
Hunk #6 succeeded at 1648 (offset 11 lines).
Hunk #7 succeeded at 1676 (offset 11 lines).
Hunk #8 succeeded at 1689 (offset 11 lines).
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/inspector/debugger/setPauseOnMicrotasks-expected.txt
patching file LayoutTests/inspector/debugger/setPauseOnMicrotasks.html

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: https://webkit-queues.webkit.org/results/12944958

[Devin Rousso]

Created attachment 376749 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 376749 [details]
Patch

Clearing flags on attachment: 376749

Committed r248894: <https://trac.webkit.org/changeset/248894>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/54500837>