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
,
Nov 8 2016
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
,
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
,
Nov 8 2016
From that changeset, the added regex in here: https://codereview.chromium.org/2372303003/diff/80001/third_party/WebKit/Source/devtools/front_end/components/Linkifier.js Might be the culprit.
,
Nov 8 2016
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.
,
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
,
Nov 9 2016
,
Nov 9 2016
,
Nov 9 2016
Blaise, could you please take a look at regex there?
,
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
,
Nov 14 2016
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.
,
Nov 14 2016
,
Dec 16 2016
,
Dec 31 2016
Why wasn't it back-merged to M-55?
,
Dec 31 2016
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.
,
Jan 3 2017
Issue 677660 has been merged into this issue. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by michiel1...@gmail.com
, Nov 8 2016