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.
Created attachment 376667 [details] Patch
This patch modifies the inspector protocol. Please ensure that any frontend changes appropriately use feature checks for new protocol features.
Created attachment 376671 [details] [Image] After Patch is applied
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.
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"; ```
Created attachment 376748 [details] Patch
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 commit-queue@webkit.org... 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
Created attachment 376749 [details] Patch
Comment on attachment 376749 [details] Patch Clearing flags on attachment: 376749 Committed r248894: <https://trac.webkit.org/changeset/248894>
All reviewed patches have been landed. Closing bug.
<rdar://problem/54500837>