1. 20 Apr, 2022 1 commit
    • Simon Zünd's avatar
      [inspector] Add 'canBeRestarted' flag to CallFrames when debugger pauses · ec41a70e
      Simon Zünd authored
      Doc: https://bit.ly/revive-restart-frame
      Context: https://crrev.com/c/3582395 (whole feature)
      
      This CL adds a new optional flag `canBeRestarted` to every call frame
      in Debugger.paused events. As the name suggests, the flag indicates
      whether we can restart a particular frame through Debugger.restartFrame
      once implemented.
      
      We are not able to safely restart all frames:
        * We don't support WASM frames
        * We don't support frames where resumable functions (async fns,
          generators) and embedder C++ frames are between the top-most
          frame and the to-be-restarted frame.
      
      Note that from a CDP perspective the flag doesn't actually guarantee
      a successful restart. CDP clients can issue
      CDP commands between the Debugger.paused event and before a user
      decides to restart a frame, which can potentially mess
      with the stack.
      
      The `canBeRestarted` flag tests are folded into the
      Debugger.restartFrame tests. As the feature is not yet fully
      implemented we short-circuit most of the tests for now and only
      run them up until the first Debugger.restartFrame call fails
      (except "fails-for-resumables.js").
      This means the tests exercise the `canBeRestarted` flag, but not
      the restarting functionality itself.
      
      R=bmeurer@chromium.org, kimanh@chromium.org
      
      Bug: chromium:1303521
      Change-Id: I01ab46dc3557ab8383960969fbe03e00604cc5e2
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3596160Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
      Reviewed-by: 's avatarKim-Anh Tran <kimanh@chromium.org>
      Commit-Queue: Simon Zünd <szuend@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#80046}
      ec41a70e
  2. 11 Apr, 2022 1 commit
    • Maksim Sadym's avatar
      Add `WebDriverBiDi` serialization to CDP · a913a75b
      Maksim Sadym authored
      1. Added `generateWebDriverValue` flag to `Runtime.evaluate` and `Runtime.callFunctionOn`.
      2. Added `webDriverValue` field to `RemoteObject`, and set it in case of the `generateWebDriverValue` flag was set.
      3. Added virtual method `bidiSerialize` to allow embedder-implemented serialization (like in https://crrev.com/c/3472491).
      4. Implemented V8 serialization in a separate class `V8WebDriverSerializer`.
      5. Hardcode `max_depth=1`.
      6. Added tests.
      
      Not implemented yet:
      1. `objectId`.
      2. Test of embedder-implemented serialization.
      
      Tested automatically by:
      ```
      python3 tools/run-tests.py --outdir out/foo inspector/runtime/add-web-driver-value
      ```
      
      Naming to be discussed. Suggestions are very welcome.
      
      Design doc: http://go/bidi-serialization
      
      Change-Id: Ib35ed8ff58e40b3304423cc2139050136d844e2c
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3472077Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
      Commit-Queue: Maksim Sadym <sadym@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#79922}
      a913a75b
  3. 08 Apr, 2022 1 commit
  4. 06 Apr, 2022 1 commit
  5. 30 Mar, 2022 1 commit
    • Benedikt Meurer's avatar
      [inspector] Add custom error dispatch machinery for debug evaluate. · 56cfdd68
      Benedikt Meurer authored
      This introduces a `V8InspectorClient::dispatchError()` callback that
      embedders can use to dispatch errors from scripts injected by DevTools
      (via debug evaluate). The idea here being that while these errors are
      technically caught by the inspector logic, the DevTools UX presents them
      just like other uncaught errors, with the exception that they don't
      trigger error handlers installed by the page. The latter can be quite
      confusing to developers, and surprising when for example testing these
      error handlers from DevTools. So this adds the foundations on the V8
      side to enable triggering error handlers for these technically caught,
      but morally uncaught, exceptions.
      
      On the Chromium side https://crrev.com/c/3560458 will implement and
      use the hook. And that CL also adds a web tests to check the behavior.
      
      Bug: chromium:1295750
      Change-Id: I945c8a9e9b4ec5705fc7f1891dcda185b04c8310
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3557234
      Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
      Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Commit-Queue: Yang Guo <yangguo@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#79673}
      56cfdd68
  6. 04 Mar, 2022 2 commits
  7. 01 Mar, 2022 1 commit
    • Benedikt Meurer's avatar
      [inspector] Simplify script end position logic. · 7eb22c89
      Benedikt Meurer authored
      Don't expose the line end table logic to V8DebuggerScript, but instead
      use the existing Script::GetPositionInfo() logic to resolve end line and
      column numbers for scripts. This also avoids having to copy (the
      potentially huge) line ends tables to std::vector's twice per script.
      
      Bug: chromium:1162229
      Change-Id: I03365d42c320d462360bacc444f7fa97904a9748
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3494240
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
      Reviewed-by: 's avatarSimon Zünd <szuend@chromium.org>
      Commit-Queue: Simon Zünd <szuend@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#79311}
      7eb22c89
  8. 28 Feb, 2022 1 commit
  9. 23 Feb, 2022 1 commit
  10. 22 Feb, 2022 3 commits
  11. 18 Feb, 2022 1 commit
  12. 15 Feb, 2022 1 commit
  13. 11 Feb, 2022 1 commit
  14. 08 Feb, 2022 1 commit
    • Benedikt Meurer's avatar
      [debug] Implement stepping out of async functions in the debugger. · 536e96cc
      Benedikt Meurer authored
      Previously the inspector was trying to handle step-out for async
      functions by annotating the async stacks, but this was merely a
      hack and didn't work reliably
      
      (a) when the async caller that is `await`ing the result of the
          callee was still in the synchronous part (because then there
          was no async task yet in the inspector), or
      (b) not at all when the async stack tracking wasn't enabled or the
          maximum async stack depth was too small.
      
      This CL replaces that hack with a pragmatic solution inside the
      V8 debugger, where upon `await` we memorize the async function
      object of the caller on the outer promise of the callee, and when
      stepping out of the callee we check whether the returned promise
      has a memorized async function object and if so, we schedule that
      to resume.
      
      This CL thereby effectively reverts https://crrev.com/c/1054618
      and replaces it with a V8 debug solution, and thereby further
      reduces the (memory) overhead of an AsyncStackTrace.
      
      Fixed: chromium:1246867
      Bug: v8:6161, v8:7753, chromium:1277451, chromium:1280519
      Change-Id: I6aa79e90f49d204f66bfd37e7a328c7fb8d635b1
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3439865Reviewed-by: 's avatarJaroslav Sevcik <jarin@chromium.org>
      Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#78990}
      536e96cc
  15. 07 Feb, 2022 1 commit
  16. 04 Feb, 2022 2 commits
  17. 01 Feb, 2022 2 commits
  18. 27 Jan, 2022 1 commit
  19. 24 Jan, 2022 1 commit
  20. 19 Jan, 2022 1 commit
    • Simon Zünd's avatar
      [inspector] Add Runtime#getExceptionDetails CDP method · 1f53cbf1
      Simon Zünd authored
      CDP has a "ExceptionDetails" structure that is attached to various
      CDP commands, e.g. "Runtime#exceptionThrown" or "Runtime#evaluate".
      The stack trace in the "ExceptionDetails" structure is used in
      various places in DevTools. The information in the "ExceptionDetails"
      structure is extracted from a v8::Message object. Message objects
      are normally created at the exception throw site and may augment
      the error with manually inspecting the stack (both to capture a fresh
      stack trace in some cases, as well as to calculate location info).
      
      The problem is that in some cases we want to get an "ExceptionDetails"
      structure after the fact, e.g. when logging a JS "Error" object in
      a catch block. To help in this case, this CL introduces a new
      CDP method "Runtime#getExceptionDetails" that behaves exactly as
      advertised: It provides a populated "ExceptionDetails" structure
      from a JS Error object.
      
      R=bmeurer@chromium.org
      
      Doc: https://bit.ly/runtime-get-exception-details
      Bug: chromium:1278650
      Change-Id: I084be10c1d852d3b7cac8d88e7f820e867be4722
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3337258
      Commit-Queue: Simon Zünd <szuend@chromium.org>
      Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#78676}
      1f53cbf1
  21. 17 Jan, 2022 1 commit
  22. 13 Jan, 2022 2 commits
    • Lei Zhang's avatar
      Remove many superfluous STL includes in headers. · 87cf0bdd
      Lei Zhang authored
      Use grep to check for obviously unneeded includes. e.g. headers that
      include <vector> but does not contain "std::vector".
      
      Change-Id: I43a9e9f01e072fd495918d28ca4cdad5cfa0294c
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3354400Reviewed-by: 's avatarMarja Hölttä <marja@chromium.org>
      Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
      Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
      Commit-Queue: Lei Zhang <thestig@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#78613}
      87cf0bdd
    • Benedikt Meurer's avatar
      [debug] Simplify async function instrumentation. · 41f0c0ba
      Benedikt Meurer authored
      This unifies and simplifies the way we instrument async functions for
      the purpose of async stack traces and async stepping. It does so while
      retaining the observable behavior on the inspector level (for now).
      
      Previously we'd mark the implicit promise of the async function object
      with the async task ID, and whenever we awaited, we'd copy the async
      task ID to the throwaway promise that is created by the `await`. This
      however made things unnecessarily interesting in the following regards:
      
      1. We'd see `DebugDidHandle` and `DebugWillHandle` events after the
      `AsyncFunctionFinished` events, coming from the throwaway promises,
      while the implicit promise is "done". This is especially confusing
      with rejection propagation and requires very complex stepping logic
      for async functions (after this CL it'll be possible to unify and
      simplify the stepping logic).
      2. We have to thread through the "can suspend" information from the
      Parser all the way through AsyncFunctionReject/AsyncFunctionResolve
      to the async function instrumentation to decide whether to cancel the
      pending task when the async function finishes.
      
      This CL changes the instrumentation to only happen (non recurringly) for
      the throwaway promises allocated upon `await`. This solves both problems
      mentioned above, and works because upon the first `await` the stack
      captured for the throwaway promise will include the synchronous part as
      expected, while upon later `await`s the synchronous part will be empty
      and the asynchronous part will be the stack captured for the previous
      throwaway promise (and the V8Debugger automatically short circuits
      stacks with empty synchronous part).
      
      Bug: chromium:1280519, chromium:1277451, chromium:1246867
      Change-Id: Id604dabc19ea133ea2e9dd63181b1fc33ccb5eda
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3383775Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
      Reviewed-by: 's avatarMaya Lekova <mslekova@chromium.org>
      Reviewed-by: 's avatarSimon Zünd <szuend@chromium.org>
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#78599}
      41f0c0ba
  23. 10 Jan, 2022 1 commit
  24. 05 Jan, 2022 2 commits
  25. 04 Jan, 2022 2 commits
  26. 03 Jan, 2022 1 commit
    • Benedikt Meurer's avatar
      Revert "[inspector] Fix `Runtime.setMaxCallStackSizeToCapture`." · c51b582d
      Benedikt Meurer authored
      This reverts commit 34f73cc7.
      
      Reason for revert: Performance regressions throughout a lot of
      system health and browsing benchmarks.
      
      Original change's description:
      > [inspector] Fix `Runtime.setMaxCallStackSizeToCapture`.
      >
      > This change fixes the implementation of the previously introduced API
      > `Runtime.setMaxCallStackSizeToCapture` to work correctly and also apply
      > (consistently) to stack traces captured by V8 when exceptions are
      > thrown. It does so in a fully backwards compatible manner.
      >
      > This change thus makes the previous fix for catapult (which landed in
      > http://crrev.com/c/3347789) effective, and therefore ensures that real
      > world performance benchmarks aren't affected by the use of the `Runtime`
      > domain in the catapult test framework.
      >
      > Bug: chromium:1283162, chromium:1278650, chromium:1258599
      > Bug: chromium:1280803, chromium:1280832, chromium:1280818
      > Fixed: chromium:1280831
      > Doc: https://bit.ly/v8-cheaper-inspector-stack-traces
      > Change-Id: I4ec951a858317fa49096cd4023deb0104d92c9c9
      > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3361839
      > Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      > Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
      > Reviewed-by: Yang Guo <yangguo@chromium.org>
      > Cr-Commit-Position: refs/heads/main@{#78458}
      
      Bug: chromium:1283162, chromium:1278650, chromium:1258599
      Bug: chromium:1280803, chromium:1280832, chromium:1280818
      Bug: chromium:1280831
      Change-Id: Id1efaffa2f7f08c47f833f68b8a297494edee21e
      Fixed: chromium:1283751, chromium:1283749, chromium:1283746
      Fixed: chromium:1283729, chromium:1283700, chromium:1283700
      Fixed: chromium:1283691, chromium:1283687, chromium:1283678
      Fixed: chromium:1283677, chromium:1283676, chromium:1283675
      Fixed: chromium:1283674, chromium:1283618, chromium:1283536
      Fixed: chromium:1283523, chromium:1283516
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3364078
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
      Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
      Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
      Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#78462}
      c51b582d
  27. 31 Dec, 2021 1 commit
  28. 30 Dec, 2021 2 commits
    • Benedikt Meurer's avatar
      [inspector] Decouple Console domain from stack trace capturing. · 451a101b
      Benedikt Meurer authored
      The `Console` domain has been deprecated (in favor of `Log` and
      `Runtime`) since over four years now, and its use is strongly
      discouraged.
      
      However, making `Runtime.setMaxCallStackSizeToCapture` useful (in
      light of the refactorings for crbug.com/1283162) and more correct
      (wrt. to the anticipated behavior), would be complicated seriously
      if we also need to worry about `Console` domain interference.
      
      So this CL simply removes the feature that `Console.enable` turns
      on stack trace capturing for error and message objects, and won't
      send `line`, `column`, and `url` with `Console.Message` events
      if they aren't present on the `v8_inspector::V8ConsoleMessage`
      instance (these fields have always been optional anyways).
      
      Bug: chromium:1283162
      Change-Id: I78bd1e040fe15a2372639c403bfc2f4579fd4d0c
      Doc: https://bit.ly/v8-cheaper-inspector-stack-traces
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3361837
      Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
      Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Commit-Queue: Yang Guo <yangguo@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#78454}
      451a101b
    • Benedikt Meurer's avatar
      [inspector] Introduce v8::StackFrame::GetLocation() API. · ed7b6640
      Benedikt Meurer authored
      This introduces a new `GetLocation()` method for `v8::StackFrame`s,
      which returns both line and column number at the same time (using the
      existing `v8::Location` class). Since `v8::StackFrame` instances store
      only the source position (per https://bit.ly/v8-stack-frame), we
      currently need to look up the source position in the Script's line table
      twice, once when we request the line number, and another time when we
      request the column number.
      
      With `GetLocation()` we perform only a single lookup in the Script's
      line table and return both line and column number at the same time. This
      cuts roughly 8% of the average execution time from the `standalone.js`
      benchmark mentioned in crbug.com/1280519.
      
      Bug: chromium:1280519, chromium:1278650, chromium:1069425
      Bug: chromium:1077657, chromium:1283162
      Doc: https://bit.ly/v8-cheaper-inspector-stack-traces
      Change-Id: Ia3a0502990b6230363112a358b59875283399404
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3359628Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#78452}
      ed7b6640
  29. 14 Dec, 2021 1 commit
  30. 08 Dec, 2021 1 commit
    • Benedikt Meurer's avatar
      [inspector] Consistent frame function name in V8 Inspector and API. · 54584461
      Benedikt Meurer authored
      On the way to a cheaper and more scalable stack frame representation
      for the inspector (crbug/1258599), this removes the need to expose
      both what was called "function name" and what was called "function
      debug name" on a v8::StackFrame instance.
      
      The reason to having a distinction between that the V8 API exposes
      and what the inspector exposes as frame function name is that after
      the initial refactoring around v8::internal::StackFrameInfo, some
      wasm cctests would still dig into the implementation details and
      insist on seeing the "function name" rather than the "function
      debug name". This CL now addresses that detail in the wasm cctests
      and going forward unifies the function names used by the inspector
      and the V8 API (which is not only needed for internal consistency
      and reduced storage requirements in the future, but also because
      Blink for example uses v8 API and v8_inspector API interchangeably
      and assumes that they agree, even though at this point Blink
      luckily wasn't paying attention to the function name):
      
      - The so-called "detailed stack trace", which is produced for the
        inspector and exposed by the v8 API, always yields the "function
        debug name" (which for example in case of wasm will be a WAT
        compatible name),
      - while the so-called "simple stack trace", which is what is used
        to implement the CallSite API and underlies Error.stack continues
        to stick to the "function name" which in case of wasm is not
        WAT compatible).
      
      Bug: chromium:1258599
      Change-Id: Ib15d038f3ec893703d0f7b03f6e7573a38e82b39
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3312274Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
      Reviewed-by: 's avatarSimon Zünd <szuend@chromium.org>
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#78283}
      54584461
  31. 07 Dec, 2021 1 commit