devtools icon indicating copy to clipboard operation
devtools copied to clipboard

Tracking bug for re-establishing breakpoints in debuggers

Open elliette opened this issue 1 year ago • 1 comments

Spinning this off of https://github.com/dart-lang/webdev/issues/2257, although this issue involves work for other systems besides DevTools. But I would like everything to be tracked in one place.

This all began because on web we were experiencing an issue of "phantom breakpoints" where a breakpoint that a user had removed was still being hit in Chrome, see https://github.com/dart-lang/webdev/issues/2257. We would like to remove that behavior and have the debugging clients be the source of truth for breakpoints. However, while DAP clients re-establish breakpoints on hot-restart, DevTools does not.

On further investigation, we discovered that breakpoints set along the app's initial startup path in DevTools and in DAP clients don't always get hit on hot-restart or (for web) when users refresh the page. This is because breakpoints are re-set by the debugging client after the app's isolate has already started. Therefore, we want to use a combination of:

  • set_isolates_paused_on_start - make sure the app initially starts out paused
  • requirePermissionToResume(onPauseStart: true) - make sure that all connected debugging clients using set_isolates_paused_on_start send resume event before resuming
  • debugging clients send resume after they have set breakpoints

Therefore, the work required here:

Note: the following needs to be done before rolling unpinning DWDS and rolling it into google3. It was pinned here: https://github.com/dart-lang/sdk/commit/4642c73347c3d9e04b0d5a4078bd4ba40cfd56d3

In DWDS:

  • [x] Remove the logic to re-establish breakpoints after a hot-restart from DWDS (DONE: https://github.com/dart-lang/webdev/pull/2371)
  • [x] DWDS needs to implement the VM Service setFlag method for pause_isolates_on_start (DONE: https://github.com/dart-lang/webdev/pull/2373)
  • [x] During hot-restart, wait to run main if pause-isolates-on-start is true (DONE: https://github.com/dart-lang/webdev/pull/2378)
  • [x] After the debugger disconnects, ensure that we no longer pause for hot-restarts/page reloads (DONE: https://github.com/dart-lang/webdev/pull/2398)
  • [x] Follow-up: Make pauseIsolatesOnStart optional (DONE: https://github.com/dart-lang/webdev/pull/2397)

In DevTools:

  • [x] Add logic to re-establish breakpoints after a hot-restart to DevTools (DONE: https://github.com/flutter/devtools/pull/7205)
  • [ ] Call setFlag('pause_isolates_on_start') and requirePermissionToResume(onPauseStart: true), then only resume once breakpoints are set (WIP: https://github.com/flutter/devtools/pull/7234)

In DDS:

  • [x] Add requireUserPermissionToResume and readyToResume (DONE BY @bkonyi): https://github.com/dart-lang/sdk/commit/5536951738ba599d96e075b7140e52b28e2336a2)
  • [x] Determine initial requireUserPermissionToResume values from the VM service flags (DONE: https://github.com/dart-lang/sdk/commit/9382e58fc9ff8a2978037a25161b2bbf2d37c1ea)

Note: This can be done after rolling DWDS into google3

In Flutter Engine:

  • [ ] Make sure pause_isolates_on_start is respected (see https://github.com/dart-lang/sdk/issues/54900, currently flutter_engine isn't respecting the flag so setting it in the VM Service doesn't do anything)

In DAP:

  • [ ] Set requirePermissionToResume(onPauseStart: true) (WIP: https://dart-review.googlesource.com/c/sdk/+/350649)

Make sure pause_isolates_on_start is respected for hot-restarts and page refreshes for web (changes required in multiple locations):

  • [ ] page refresh when running from flutter_tools - code pointer here
  • [x] hot-restart when running from flutter_tools (main is triggered by the require_restarter (code pointer here) (DONE: https://github.com/dart-lang/webdev/pull/2378)
  • [ ] Hot-restart and page-refreshes in g3: b/335227246
  • [ ] Mirror of hot-restart in g3 (WIP: https://dart-review.googlesource.com/c/sdk/+/355880)

elliette avatar Feb 15 '24 18:02 elliette

Since this requires coordination across multiple debugging clients, it makes sense to pin DWDS in g3 until everything is ready to land.

elliette avatar Feb 27 '24 22:02 elliette