Issue metadata
Sign in to add a comment
|
setinterval with extension issue: when tabs take up 100% CPU each when not the active tab
Reported by
gmsz...@gmail.com,
Jun 15 2017
|
||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.86 Safari/537.36 Steps to reproduce the problem: 1. install a extensions with setInterval, like React Develop tools: https://chrome.google.com/webstore/detail/react-developer-tools/fmkadmapgofadopljbjfkapdkoienihi 2. open in Tab 1 with Develop tool 3. switch to Tab 2 will cause this issue Code example in https://github.com/phodal/chrome-59-show-setinterval-issue-code What is the expected behavior? What went wrong? CPU run to near 100%; Did this work before? N/A Chrome version: 59.0.3071.86 Channel: stable OS Version: OS X 10.12.5 Flash Version: It seems the new feature in Chrome 59 about setinterval will trigger this.
,
Jun 16 2017
Oops. This patch has been reverted on master: https://codereview.chromium.org/2850143003/, but managed to sneak into the release branch. We should revert this upon release owners approval and I will write a postmortem. Notwithstanding the above, thanks for the bug report. I'll look into it to make sure that everything will be alright when we reland this.
,
Jun 16 2017
To clarify: the patch in question is known to be bad and was reverted in master due to test breakage. Most likely this problem affects not only extensions with setInterval, but many other use cases (e.g. telemetry failures causing the patch to be reverted in the first place). Hereby I request permission to revert the patch in question in a release branch.
,
Jun 16 2017
+amineer@, gkihumba@, cmasso@
,
Jun 16 2017
Can you please confirm if this revert will be a safe merge? And we still need to determine if this is absolutely necessary - how many users are currently affected by this and is there any workaround solution or experiment that we can turn off?
,
Jun 16 2017
Yes, this is going to be a safe merge. The main issue is that we don't know the full scope of the problem. Revert is the easiest and safest way.
,
Jun 16 2017
Based on comments and stars, it's an urgent fix and safe merge as well. Approving merge for M59. Please go ahead and merge to branch 3071. We'll plan for a desktop respin middle of next week.
,
Jun 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6ccf3815953a4d8210a8809fe06e21b633cf8f15 commit 6ccf3815953a4d8210a8809fe06e21b633cf8f15 Author: Alexander Timin <altimin@chromium.org> Date: Fri Jun 16 22:26:43 2017 Revert of [scheduler] Move some task types to suspendable task runner. This reverts commit 78a049660074003b8e1e602b0e7783ca7f1f4ee0. BUG= 733469 TBR=altimin@chromium.org Change-Id: I13ccff94000f70146d780da49268c2a30be74d70 Reviewed-on: https://chromium-review.googlesource.com/538696 Reviewed-by: Alexander Timin <altimin@chromium.org> Cr-Commit-Position: refs/branch-heads/3071@{#797} Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641} [modify] https://crrev.com/6ccf3815953a4d8210a8809fe06e21b633cf8f15/third_party/WebKit/Source/core/dom/TaskRunnerHelper.cpp
,
Jun 19 2017
,
Jun 19 2017
,
Jun 20 2017
Verified this issue on Windows-10, Ubuntu 14.04 and Mac OS 10.12.5 using chrome latest stable #59.0.3071.109 and observed the cpu usage of dev tools didn't exceed more that 2% in chrome task manager. Confirming the fix is working as expected and adding TE-Verified label. Thanks! |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by brajkumar@chromium.org
, Jun 16 2017Labels: -Type-Bug -Pri-2 hasbisect-per-revision ReleaseBlock-Stable M-59 Pri-1 Type-Bug-Regression
Owner: altimin@chromium.org
Status: Assigned (was: Unconfirmed)