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

Issue 663342 link

Starred by 9 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

Regression: Dev console hanging when logging a long string of slashes ("////....")

Reported by michiel1...@gmail.com, Nov 8 2016

Issue description

Chrome Version       : 56.0.2913.0
OS Version: 10.0
URLs (if applicable) :
Other browsers tested:
  Add OK or FAIL after other browsers where you have tested this issue:
     Safari 5:
  Firefox 4.x:
     IE 7/8/9:

What steps will reproduce the problem?
1. Open the developer console.
2. Enter: "/".repeat(40);
3. Run the script.

What is the expected result?

"////////////////////////////////////////" is logged immediately.

What happens instead of that?

It takes several seconds for the console to display the string of slashes. The longer the string, the longer it takes.
This issue only appears to surface for the "/" character, as far as I've tried.


UserAgentString: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2913.0 Safari/537.36



 
I assumed I could edit this after submitting.

I can't reproduce this issue on any of the other browsers under "Other browsers tested" (logging "///..." where .repeat is unavailable)

This issue also occurs when you simply try to log "////////////////////////////////////////"

Comment 2 Deleted

Note that it freezes regardless of there being other characters in addition to the /s. However, the presence of other character results in an increase in freeze time. 

To reproduce: (Both evaluate to strings with 27 characters; The former takes longer to display)

"path/to/path/to/path/to/path/to/".repeat(3)

and

"/".repeat(27)

Also, this issue might be related to this:https://bugs.chromium.org/p/chromium/issues/detail?id=658525

Comment 4 by woxxom@gmail.com, Nov 8 2016

This is a regression.
Bisect: 421698 (good) - 421705 (bad), 55.0.2876.0
Changelog: https://chromium.googlesource.com/chromium/src/+log/64af6cd2..1c98a2e1?pretty=fuller
Suspecting https://crrev.com/2372303003 as the only DevTools-related change
That code is absolutely the culprit.

Running:

var pathLineRegex = /(?:\/[\/\w\.-]+)+\:[\d]+/;
console.log(pathLineRegex.exec("/".repeat(40)));

Takes a few seconds. Any other sting is instant.

Comment 7 by woxxom@gmail.com, Nov 8 2016

#6, yes, and another evidence is that devtools-over-devtools Profiler shows 99% CPU is spent in this callstack:

	exec    
	WebInspector.linkifyStringAsFragmentWithCustomLinkifier inspector.js:7330
	WebInspector.linkifyStringAsFragment                    inspector.js:7335
	_formatParameterAsString                                console_module.js:119
	..........

The pathLineRegex.exec part was added by the suspected CL in Linkifier.js:
https://codereview.chromium.org/2372303003/diff/80001/third_party/WebKit/Source/devtools/front_end/components/Linkifier.js
Components: Platform>DevTools
Cc: dgozman@chromium.org
Labels: -Pri-3 ReleaseBlock-Stable M-56 Pri-1
Owner: allada@chromium.org
Status: Assigned (was: Unconfirmed)
Summary: Regression: Dev console hanging when logging a long string of slashes ("////....") (was: Dev console hanging when logging a long string of slashes ("////...."))
Blaise, could you please take a look at regex there?
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 10 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c45e5d718830df671d29c09104f0868b696d98a6

commit c45e5d718830df671d29c09104f0868b696d98a6
Author: allada <allada@chromium.org>
Date: Thu Nov 10 03:09:25 2016

[Devtools] Fixed regression with linkifier regex

This patch fixes an offending patch:
https://codereview.chromium.org/2372303003 that caused devtools to
be extremely slow when many slashes as a string were entered into
console.

BUG= 663342 
R=dgozman,pfeldman

Review-Url: https://codereview.chromium.org/2485963005
Cr-Commit-Position: refs/heads/master@{#431158}

[modify] https://crrev.com/c45e5d718830df671d29c09104f0868b696d98a6/third_party/WebKit/LayoutTests/inspector/components/linkifier.html
[modify] https://crrev.com/c45e5d718830df671d29c09104f0868b696d98a6/third_party/WebKit/Source/devtools/front_end/components/Linkifier.js

Labels: TE-Verified-M56 TE-Verified-56.0.2918.0
Tested the issue on windows 7 using chrome version 56.0.2918.0.Observed "////////////////////////////////////////" is logged immediately.Please find the attached screen cast for the same.

adding Te-Verified labels.

663342.mp4
380 KB View Download
Status: Fixed (was: Assigned)
Cc: l...@chromium.org
 Issue 674515  has been merged into this issue.

Comment 15 by woxxom@gmail.com, Dec 31 2016

Why wasn't it back-merged to M-55?
TL;DR:
It simply fell through the cracks.

Details:
We usually revert changes then fix them then re-upload them, which resets the date of when it will launch.

However, this patch fell between a strange time because we did two huge refactors of the code that changed almost every line of our code base. This means that we would have had to revert almost every change since the refactor to revert it, so we applied it to ToT and planned to wait the (standard) week before we merge it to beta, but I simply forgot.

We have a safety to keep this from happening, which is flag the bug with "M-{VersionNumber}" and "Release-Block-Stable" which you can see this bug had. However we flagged it with the wrong version number; this kept the teams in charge of releasing from letting us know we need to merge it.

In short our normal procedure is:
1) Use bisect to find offending patch and revert it.
2) Flag bug with branch it's offending and if necessary add "Release-Block-Stable".
3) Submit fixed patch.
4) If needed request merge to beta and merge it after ~one week of ToT.

If one does not revert the patch and forgets to upload patch to Beta, the version number + release-block-stable will cause the teams in charge of releasing to contact (usually) the bug owner to figure out what to do.

In our case:
* We could not revert it.
* We accidentally flagged it with the wrong version number.
* I forgot to merge it to beta after ~one week of it on ToT.
 Issue 677660  has been merged into this issue.

Sign in to add a comment