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

Issue 733469 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Regression



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 description

UserAgent: 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.
 
Cc: ranjitkan@chromium.org pbomm...@chromium.org abdulsyed@chromium.org brajkumar@chromium.org
Labels: -Type-Bug -Pri-2 hasbisect-per-revision ReleaseBlock-Stable M-59 Pri-1 Type-Bug-Regression
Owner: altimin@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce on Mac OS 10.12.5 using chrome latest stable #59.0.3071.86. This issue is not observed on Windows and Linux platform.

Bisect Information:
--------------------
Good build: 59.0.3068.1
Bad Build : 59.0.3070.0 

You are probably looking for a change made after 464024 (known good), but no later than 464025 (first known bad).

Change Log URL: 
https://chromium.googlesource.com/chromium/src/+log/5bc8d44f3ab226674d0dd2c52ef631d6d52dc464..78a049660074003b8e1e602b0e7783ca7f1f4ee0

From the above change log suspecting below change
Review-Url: https://codereview.chromium.org/2808273003

altimin@ - Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner. Since this is a latest regression observed on M-59 adding RB-Stable, Please feel free to remove if this is not the case.

Thanks!
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.
Labels: -OS-Mac Merge-Request-59 OS-All
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.
Cc: gkihumba@chromium.org cma...@chromium.org amineer@chromium.org
+amineer@, gkihumba@, cmasso@
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?
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.
Labels: -Merge-Request-59 Merge-Approved-59
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. 
Project Member

Comment 8 by bugdroid1@chromium.org, Jun 16 2017

Labels: -merge-approved-59 merge-merged-3071
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

Components: -Platform>DevTools Blink>Scheduling
Status: Fixed (was: Assigned)
Labels: TE-Verified-M59 TE-Verified-59.0.3071.109
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!
Screen Shot 2017-06-20 at 2.12.58 PM.png
171 KB View Download

Sign in to add a comment