Skip to content

Commit

Permalink
Revert "[DevTools] Terminate execution after reload on breakpoint."
Browse files Browse the repository at this point in the history
This reverts commit 335c416.

Reason for revert: Suspect of causing test failures on https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20Tests%20(dbg)(1)/104074/overview

Original change's description:
> [DevTools] Terminate execution after reload on breakpoint.
>
> This relands https://crrev.com/c/2041623 (by sigurds@chromium.org),
> which fixes the problem that JavaScript execution would continue after
> reloading on a breakpoint. Continue executing JavaScript is causing a
> lot of trouble for developers and might leave the renderer in a broken
> state (due to some React parts getting into an endless loop for example
> [1]). This is clearly a pain point for Web developers. This change also
> aligns Chromium's behavior with that of Firefox.
>
> The original CL https://crrev.com/c/2041623 was reverted due to being
> "suspect for rare crashes", which aligns with recent (unrelated)
> findings by szuend@chromium.org: The termination exception that is used
> to stop execution here and elsewhere is not always handled properly
> all over V8 and Blink. Landing this CL will make it more likely that
> Chromium crashes because of a place where termination is not handled
> correctly. On the other hand, not fixing the issue with continuing JS
> execution after reload will just paper over the problem.
>
> So going forward we will land this CL and thereby address the developer
> pain point, and look into any crashes related to this in detail.
>
> [1]: https://stackoverflow.com/q/64183472
>
> Doc: http://doc/1aO9v0YhoKNqKleqfACGUpwrBUayLFGqktz9ltdgKHMk
> Bug: chromium:1004038, chromium:1014415
> Change-Id: I0b89e3bf1f139f3af022988358f986be3387b7ce
> Fixed: chromium:1112863, chromium:1134899
> Also-By: sigurds@chromium.org
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3571746
> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#989463}

Bug: chromium:1004038, chromium:1014415
Change-Id: I9cb091d23523b4e4e9d2adfbbccef01a2e431918
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3574990
Auto-Submit: Jian Li <jianli@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#989798}
  • Loading branch information
jianli-chromium authored and Chromium LUCI CQ committed Apr 7, 2022
1 parent 7f733a0 commit 21698f8
Show file tree
Hide file tree
Showing 6 changed files with 1 addition and 93 deletions.
Expand Up @@ -238,7 +238,7 @@ void DevToolsSession::DispatchProtocolCommandImpl(
void DevToolsSession::DidStartProvisionalLoad(LocalFrame* frame) {
if (v8_session_ && agent_->inspected_frames_->Root() == frame) {
v8_session_->setSkipAllPauses(true);
v8_session_->resume(true /* terminate on resume */);
v8_session_->resume();
}
}

Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

0 comments on commit 21698f8

Please sign in to comment.