Issue metadata
Sign in to add a comment
|
Make sure execution context suspension affects the right set of tasks |
||||||||||||||||||||||
Issue descriptionWhen an execution context is suspended (e.g., because of a synchronous call to document.print()), we should ensure the right set of tasks get suspended. To do this, we will introduce an unsuspended task queue (to complement the unthrottled task queue) and route the appropriate tasks to it. The QoS hierarchy of task queues then (roughly) becomes: timer < loading < unthrottled < unsuspended
,
Mar 16 2017
Should this be an M-58 RB-Stable given the known regression in 57 that we're trying to address here?
,
Mar 16 2017
Yes, sounds right.
,
Mar 27 2017
Gentle ping to get an update on this issue , as it is marked as ReleaseBlock-Stable.
,
Mar 27 2017
Yes, I'm working on this and I expect to resolve it by the end of this week.
,
Apr 4 2017
Gentle ping to get an update on this issue .
,
Apr 4 2017
https://codereview.chromium.org/2754673002/ was landed, but it was reverted due to perf waterfall failures. Investigating root cause.
,
Apr 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cb434223a4b462abf2b9f9123646137250b3c2d9 commit cb434223a4b462abf2b9f9123646137250b3c2d9 Author: altimin <altimin@chromium.org> Date: Wed Apr 05 20:45:32 2017 [scheduler] Add suspendable task queue. Add suspendable task queue to avoid running (and cancelling) callbacks when execution context is suspended. R=skyostil@chromium.org,alexclarke@chromium.org,haraken@chromium.org CC=nhiroki@chromium.org,tzik@chromium.org BUG= 704939 , 702160 Review-Url: https://codereview.chromium.org/2754673002 Cr-Original-Commit-Position: refs/heads/master@{#459787} Committed: https://chromium.googlesource.com/chromium/src/+/12e16766adec5bb1e5cdd73d3cd8f5016b59ce18 Review-Url: https://codereview.chromium.org/2754673002 Cr-Commit-Position: refs/heads/master@{#462208} [modify] https://crrev.com/cb434223a4b462abf2b9f9123646137250b3c2d9/third_party/WebKit/Source/core/dom/TaskRunnerHelper.cpp [modify] https://crrev.com/cb434223a4b462abf2b9f9123646137250b3c2d9/third_party/WebKit/Source/core/loader/EmptyClients.cpp [modify] https://crrev.com/cb434223a4b462abf2b9f9123646137250b3c2d9/third_party/WebKit/Source/platform/WebFrameScheduler.h [modify] https://crrev.com/cb434223a4b462abf2b9f9123646137250b3c2d9/third_party/WebKit/Source/platform/scheduler/renderer/web_frame_scheduler_impl.cc [modify] https://crrev.com/cb434223a4b462abf2b9f9123646137250b3c2d9/third_party/WebKit/Source/platform/scheduler/renderer/web_frame_scheduler_impl.h [modify] https://crrev.com/cb434223a4b462abf2b9f9123646137250b3c2d9/third_party/WebKit/Source/platform/scheduler/renderer/web_frame_scheduler_impl_unittest.cc
,
Apr 5 2017
A friendly reminder that M58 Stable launch is coming soon! Your bug is labelled as Stable ReleaseBlock, please make sure to land the fix, verified in trunk and get it merged into the release branch ASAP.
,
Apr 6 2017
A friendly reminder that M58 Stable is launch is coming soon (less than 2 weeks)! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch ASAP so it gets enough baking time in Beta (before Stable promotion). Thank you!
,
Apr 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7e5e14e80e181ad914e09d9f607281f248ec49e6 commit 7e5e14e80e181ad914e09d9f607281f248ec49e6 Author: altimin <altimin@chromium.org> Date: Wed Apr 12 01:08:10 2017 DCHECK for execution context being unsuspended during v8 bindings callback. To prevent loss of data, change silent failure when trying to run a v8 callback on a suspended execution context to a DCHECK. R=haraken@chromium.org CC=skyostil@chromium.org BUG= 702218 , 702160 Review-Url: https://codereview.chromium.org/2800093002 Cr-Commit-Position: refs/heads/master@{#463869} [modify] https://crrev.com/7e5e14e80e181ad914e09d9f607281f248ec49e6/third_party/WebKit/Source/bindings/templates/callback_interface.cpp.tmpl [modify] https://crrev.com/7e5e14e80e181ad914e09d9f607281f248ec49e6/third_party/WebKit/Source/bindings/tests/results/core/V8TestCallbackInterface.cpp
,
Apr 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/78a049660074003b8e1e602b0e7783ca7f1f4ee0 commit 78a049660074003b8e1e602b0e7783ca7f1f4ee0 Author: altimin <altimin@chromium.org> Date: Wed Apr 12 15:08:03 2017 [scheduler] Move some task types to suspendable task runner. Move several safe-looking task types to suspendable task runner. More task types will be moved to suspendable task runner, but we'll start with this set and will monitor any problems (there were some perfbot failures when all task queues were moved to suspendable task runner). R=haraken@chromium.org CC=skyostil@chromium.org BUG= 702160 Review-Url: https://codereview.chromium.org/2808273003 Cr-Commit-Position: refs/heads/master@{#464025} [modify] https://crrev.com/78a049660074003b8e1e602b0e7783ca7f1f4ee0/third_party/WebKit/Source/core/dom/TaskRunnerHelper.cpp
,
Apr 13 2017
Thanks for thefix, please ensure to verify in canary and request a merge to M58. FYI: Final Beta and Pre-Stable RC cut @ 4.00 PM Monday, 04/17.
,
Apr 17 2017
Just a heads up since this is marked as RB-Stable - we are aiming to launch M58 early stable this Wednesday, RC cut today @ 5:00 PM PT or latest by tomorrow noon if all goes well. Can you please take a look at this urgently and confirm if it's still a blocker for M58?
,
Apr 17 2017
As per offline chat with altimin@ punting this to M59 since there is a V8 regression which needs investigation. Lopping to release folks as well.
,
Apr 17 2017
,
Apr 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/990315f377a2df9604edcad7b6f9c2d1c53dd059 commit 990315f377a2df9604edcad7b6f9c2d1c53dd059 Author: hablich <hablich@chromium.org> Date: Wed Apr 19 15:41:18 2017 Revert of [scheduler] Move some task types to suspendable task runner. (patchset #1 id:1 of https://codereview.chromium.org/2808273003/ ) Reason for revert: Reverting as suggested in BUG=711196 Original issue's description: > [scheduler] Move some task types to suspendable task runner. > > Move several safe-looking task types to suspendable task runner. > More task types will be moved to suspendable task runner, but > we'll start with this set and will monitor any problems > (there were some perfbot failures when all task queues were > moved to suspendable task runner). > > R=haraken@chromium.org > CC=skyostil@chromium.org > BUG= 702160 > > Review-Url: https://codereview.chromium.org/2808273003 > Cr-Commit-Position: refs/heads/master@{#464025} > Committed: https://chromium.googlesource.com/chromium/src/+/78a049660074003b8e1e602b0e7783ca7f1f4ee0 TBR=haraken@chromium.org,skyostil@chromium.org,altimin@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 702160 Review-Url: https://codereview.chromium.org/2829643002 Cr-Commit-Position: refs/heads/master@{#465619} [modify] https://crrev.com/990315f377a2df9604edcad7b6f9c2d1c53dd059/third_party/WebKit/Source/core/dom/TaskRunnerHelper.cpp
,
Apr 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a8835855fb816d7f96b3b009d52b814129a02bde commit a8835855fb816d7f96b3b009d52b814129a02bde Author: hablich <hablich@chromium.org> Date: Wed Apr 19 15:47:35 2017 Revert of DCHECK for execution context being unsuspended during v8 bindings callback. (patchset #2 id:20001 of https://codereview.chromium.org/2800093002/ ) Reason for revert: Speculative revert because of BUG=711196 Original issue's description: > DCHECK for execution context being unsuspended during v8 bindings callback. > > To prevent loss of data, change silent failure when trying to run a v8 callback on a suspended execution context to a DCHECK. > > R=haraken@chromium.org > CC=skyostil@chromium.org > > BUG= 702218 , 702160 > > Review-Url: https://codereview.chromium.org/2800093002 > Cr-Commit-Position: refs/heads/master@{#463869} > Committed: https://chromium.googlesource.com/chromium/src/+/7e5e14e80e181ad914e09d9f607281f248ec49e6 TBR=haraken@chromium.org,altimin@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 702218 , 702160 Review-Url: https://codereview.chromium.org/2825363002 Cr-Commit-Position: refs/heads/master@{#465622} [modify] https://crrev.com/a8835855fb816d7f96b3b009d52b814129a02bde/third_party/WebKit/Source/bindings/templates/callback_interface.cpp.tmpl [modify] https://crrev.com/a8835855fb816d7f96b3b009d52b814129a02bde/third_party/WebKit/Source/bindings/tests/results/core/V8TestCallbackInterface.cpp
,
Apr 20 2017
,
Apr 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/89bf4588a7238e2da23c119ad363edab624e73e4 commit 89bf4588a7238e2da23c119ad363edab624e73e4 Author: altimin <altimin@chromium.org> Date: Wed Apr 26 21:20:35 2017 DCHECK for execution context being unsuspended during v8 bindings callback. To prevent loss of data, change silent failure when trying to run a v8 callback on a suspended execution context to a DCHECK. R=haraken@chromium.org CC=skyostil@chromium.org BUG= 702218 , 702160 Review-Url: https://codereview.chromium.org/2800093002 Cr-Original-Commit-Position: refs/heads/master@{#463869} Committed: https://chromium.googlesource.com/chromium/src/+/7e5e14e80e181ad914e09d9f607281f248ec49e6 Review-Url: https://codereview.chromium.org/2800093002 Cr-Commit-Position: refs/heads/master@{#467459} [modify] https://crrev.com/89bf4588a7238e2da23c119ad363edab624e73e4/third_party/WebKit/Source/bindings/templates/callback_interface.cpp.tmpl [modify] https://crrev.com/89bf4588a7238e2da23c119ad363edab624e73e4/third_party/WebKit/Source/bindings/tests/results/core/V8TestCallbackInterface.cpp
,
Apr 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c18764a33a595baab6ba35a1447e51c7c74b6e30 commit c18764a33a595baab6ba35a1447e51c7c74b6e30 Author: altimin <altimin@chromium.org> Date: Thu Apr 27 10:39:19 2017 [scheduler] Move some task types to suspendable task runner. Move several safe-looking task types to suspendable task runner. More task types will be moved to suspendable task runner, but we'll start with this set and will monitor any problems (there were some perfbot failures when all task queues were moved to suspendable task runner). R=haraken@chromium.org CC=skyostil@chromium.org BUG= 702160 Review-Url: https://codereview.chromium.org/2808273003 Cr-Original-Commit-Position: refs/heads/master@{#464025} Committed: https://chromium.googlesource.com/chromium/src/+/78a049660074003b8e1e602b0e7783ca7f1f4ee0 Review-Url: https://codereview.chromium.org/2808273003 Cr-Commit-Position: refs/heads/master@{#467633} [modify] https://crrev.com/c18764a33a595baab6ba35a1447e51c7c74b6e30/third_party/WebKit/Source/core/dom/TaskRunnerHelper.cpp
,
May 10 2017
altimin@ want to check if we need any further CL's to get merged ti M59, if so can we get the CL's merged sooner and if not please mark the bug as fixed.
,
May 18 2017
Reminder that M59 Stable is launch is coming soon (less than 2 weeks)! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch ASAP so it gets enough baking time in Beta (before Stable promotion). Thank you!
,
May 22 2017
This turned out to be more difficult that we expected. Removing stable-block label.
,
Dec 12 2017
I believe that with all refactorings around per-frame task queues and task runners we finally got it right and all appropriate tasks do not run when the execution context is suspended.
,
Dec 12 2017
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by altimin@chromium.org
, Mar 16 2017Labels: Postmortem-Followup