New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 893364 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 16
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Breakpoints in pretty-printed source file of an OOPIF don't work

Project Member Reported by creis@chromium.org, Oct 8

Issue description

Chrome Version: 70.0.3538.45
OS: Windows 10

What steps will reproduce the problem?
(0) Start Chrome with --site-per-process
(1) Visit http://csreis.github.io/tests/minified/index.html
(2) Open DevTools and go to Sources tab.
(3) Select script.js (under iframe.html -> csreis.github.io -> tests/minified)
(4) Click the Pretty-Print button: "{ }"
(5) Set a breakpoint on console.log("line 2");
(6) Reload the page.

What is the expected result?
Should hit the breakpoint set in step 5.

What happens instead?
Chrome does not stop at the breakpoint.

It appears that this only happens if the iframe is an out-of-process iframe.  The bug does not reproduce if you visit it at https://csreis.github.io/tests/minified/index.html, in which case both the main frame and iframe are HTTPS and in the same process.

This appears to work properly in 69.0.3497.100.  Running a bisect with --site-per-process points to r583934 from kozyatinskiy@.  Can you take a look?

https://chromium-review.googlesource.com/c/chromium/src/+/1179262
 
kozyatinskiy@ or lushnikov@: Friendly ping.  This was a regression in M70.  Would it be possible to get a fix into at least M71?  I imagine it's going to be problematic for web develoeprs who don't understand why their breakpoints don't work.  Thanks!
Status: Assigned (was: Untriaged)
The proper solution here is to teach DevTools to never create magic UISourceCode with ":formatted" suffix, we already have this experiment and most likely we will enable it by default in M72.
Meanwhile I will revert my CL and add test in followup and then later we should reland my CL again.

Thanks for you great report.
Labels: Merge-Request-71
Status: Fixed (was: Assigned)
revert landed: https://chromium-review.googlesource.com/c/chromium/src/+/1281688
Owner: kozy@chromium.org
Cc: gov...@chromium.org
Labels: Target-71
Thanks!  I can confirm the fix in Mac Canary 72.0.3582.0.  Merging r599774 sounds good to me.
Labels: -Merge-Request-71 Merge-Approved-71
Approving merge to M71 branch 3578 based on comment #6. Pls merge ASAP so we can pick it up for tomorrow's release. Thank you.
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 16

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4d4de858fe989f47a8ef9073f7ca6ecbd3a55837

commit 4d4de858fe989f47a8ef9073f7ca6ecbd3a55837
Author: Alexey Kozyatinskiy <kozyatinskiy@chromium.org>
Date: Tue Oct 16 18:35:54 2018

Revert "[DevTools] removed Breakpoint._currentState"

This reverts commit 9c39f918d0601422526a5b2f9d7798dfa553b67d.

Reason for revert: break breakpoint in minified sources.

Original change's description:
> [DevTools] removed Breakpoint._currentState
>
> Current state on breakpoint shared between different models is hack
> that should never work.
>
> R=​lushnikov@chromium.org
>
> Bug: none
> Change-Id: I60a0e5774c47c38a0f5f251f9e9d5f2437ceb9b4
> Reviewed-on: https://chromium-review.googlesource.com/1179262
> Reviewed-by: Andrey Lushnikov <lushnikov@chromium.org>
> Commit-Queue: Andrey Lushnikov <lushnikov@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#583934}

TBR=kozyatinskiy@chromium.org, lushnikov@chromium.org


(cherry picked from commit 0d7737c74fc25f1fdd71138cb1765913d5a7adcb)

Bug:  chromium:893364 
Change-Id: Id6a170405c0d07658ff5741142dc2b35b7f89c6b
Reviewed-on: https://chromium-review.googlesource.com/c/1281688
Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599774}
Reviewed-on: https://chromium-review.googlesource.com/c/1283936
Cr-Commit-Position: refs/branch-heads/3578@{#48}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/4d4de858fe989f47a8ef9073f7ca6ecbd3a55837/third_party/blink/renderer/devtools/front_end/bindings/BreakpointManager.js

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/4d4de858fe989f47a8ef9073f7ca6ecbd3a55837

Commit: 4d4de858fe989f47a8ef9073f7ca6ecbd3a55837
Author: kozyatinskiy@chromium.org
Commiter: kozyatinskiy@chromium.org
Date: 2018-10-16 18:35:54 +0000 UTC

Revert "[DevTools] removed Breakpoint._currentState"

This reverts commit 9c39f918d0601422526a5b2f9d7798dfa553b67d.

Reason for revert: break breakpoint in minified sources.

Original change's description:
> [DevTools] removed Breakpoint._currentState
>
> Current state on breakpoint shared between different models is hack
> that should never work.
>
> R=​lushnikov@chromium.org
>
> Bug: none
> Change-Id: I60a0e5774c47c38a0f5f251f9e9d5f2437ceb9b4
> Reviewed-on: https://chromium-review.googlesource.com/1179262
> Reviewed-by: Andrey Lushnikov <lushnikov@chromium.org>
> Commit-Queue: Andrey Lushnikov <lushnikov@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#583934}

TBR=kozyatinskiy@chromium.org, lushnikov@chromium.org


(cherry picked from commit 0d7737c74fc25f1fdd71138cb1765913d5a7adcb)

Bug:  chromium:893364 
Change-Id: Id6a170405c0d07658ff5741142dc2b35b7f89c6b
Reviewed-on: https://chromium-review.googlesource.com/c/1281688
Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599774}
Reviewed-on: https://chromium-review.googlesource.com/c/1283936
Cr-Commit-Position: refs/branch-heads/3578@{#48}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

Sign in to add a comment