New issue
Advanced search Search tips

Issue 770736 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Continue running per-frame tasks after frame was detached

Project Member Reported by altimin@chromium.org, Oct 2 2017

Issue description

Do not silently delete posted tasks to per-frame task queues after frame was detached.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 4 2017

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

commit d5389f4dd66f040f4e585732c200e6572e840c24
Author: Alexander Timin <altimin@chromium.org>
Date: Wed Oct 04 17:27:27 2017

[scheduler] Do not reference TaskQueue from TaskQueueManager.

Change task queue ownership semantics in the scheduler:
- TaskQueueManager does not keep a scoped_refptr to TaskQueue.
- TaskQueueImpl may be detached from TaskQueue and passed back to
  TaskQueueManager.

New task queue lifetime:
- TaskQueueImpl should be unregistered before deletion.
- TaskQueueManager unregisters all task queues upon deletion.
- TaskQueue holds a unique pointer to TaskQueueImpl while the queue
  is active.
- TaskQueue passes TaskQueueImpl to TaskQueueManager when
  TaskQueue::UnregisterTaskQueue is called.
- (future) TaskQueue may opt into graceful shutdown. In this case
  a async task runner is provided and upon deleting unregistered
  TaskQueue TaskQueueImpl will be asynchronously passed
  to TaskQueueManager. No new tasks may be posted to this task queue
  due to TaskQueue being deleted and TaskQueueManager will continue
  running tasks until TaskQueueImpl is empty.

This patch is a first step towards supporting graceful shutdown.

R=alexclarke@chromium.org, skyostil@chromium.org
BUG= 770736 

Change-Id: I872a3b9e85dd376289e9838fc3dd83b1901ad4ff
Reviewed-on: https://chromium-review.googlesource.com/695546
Reviewed-by: Alex Clarke <alexclarke@chromium.org>
Commit-Queue: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506428}
[modify] https://crrev.com/d5389f4dd66f040f4e585732c200e6572e840c24/third_party/WebKit/Source/platform/scheduler/base/task_queue.cc
[modify] https://crrev.com/d5389f4dd66f040f4e585732c200e6572e840c24/third_party/WebKit/Source/platform/scheduler/base/task_queue.h
[modify] https://crrev.com/d5389f4dd66f040f4e585732c200e6572e840c24/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc
[modify] https://crrev.com/d5389f4dd66f040f4e585732c200e6572e840c24/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h
[modify] https://crrev.com/d5389f4dd66f040f4e585732c200e6572e840c24/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc
[modify] https://crrev.com/d5389f4dd66f040f4e585732c200e6572e840c24/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.h
[modify] https://crrev.com/d5389f4dd66f040f4e585732c200e6572e840c24/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc
[modify] https://crrev.com/d5389f4dd66f040f4e585732c200e6572e840c24/third_party/WebKit/Source/platform/scheduler/base/task_queue_selector_unittest.cc
[modify] https://crrev.com/d5389f4dd66f040f4e585732c200e6572e840c24/third_party/WebKit/Source/platform/scheduler/base/time_domain_unittest.cc
[modify] https://crrev.com/d5389f4dd66f040f4e585732c200e6572e840c24/third_party/WebKit/Source/platform/scheduler/child/idle_helper.cc
[modify] https://crrev.com/d5389f4dd66f040f4e585732c200e6572e840c24/third_party/WebKit/Source/platform/scheduler/child/scheduler_helper_unittest.cc
[modify] https://crrev.com/d5389f4dd66f040f4e585732c200e6572e840c24/third_party/WebKit/Source/platform/scheduler/child/worker_scheduler_helper.cc
[modify] https://crrev.com/d5389f4dd66f040f4e585732c200e6572e840c24/third_party/WebKit/Source/platform/scheduler/renderer/main_thread_scheduler_helper.cc
[modify] https://crrev.com/d5389f4dd66f040f4e585732c200e6572e840c24/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
[modify] https://crrev.com/d5389f4dd66f040f4e585732c200e6572e840c24/third_party/WebKit/Source/platform/scheduler/test/test_task_queue.cc

Cc: panicker@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 2 2017

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

commit ada0b50c9759a09fc38eb7331c5aa5d35266d9dc
Author: Alexander Timin <altimin@chromium.org>
Date: Thu Nov 02 10:16:46 2017

[scheduler] Graceful shutdown for task queues

Introduce a concept of "graceful shutdown" - TaskQueue may opt into this
by providing an async task runner to delete on. This way TaskQueue won't
have to call UnregisterTaskQueue manually and ownership of TaskQueueImpl
will be transferred to TaskQueueManager via async task posted to the
provided task runner. TaskQueueManager will delete TaskQueueImpl when
all tasks have been executed.

R=alexclarke@chromium.org,skyostil@chromium.org,hajimehoshi@chromium.org

Bug:  770736 
Change-Id: Ida963924c30b6e8ff97944877c05a8f59bea7f60
Reviewed-on: https://chromium-review.googlesource.com/712936
Commit-Queue: Alexander Timin <altimin@chromium.org>
Reviewed-by: Alex Clarke <alexclarke@chromium.org>
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513449}
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/base/task_queue.cc
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/base/task_queue.h
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.h
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/child/idle_helper.cc
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/child/scheduler_helper_unittest.cc
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/child/worker_global_scope_scheduler.cc
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/child/worker_scheduler_helper.cc
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/child/worker_task_queue.cc
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/child/worker_task_queue.h
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/renderer/auto_advancing_virtual_time_domain_unittest.cc
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/renderer/main_thread_scheduler_helper.cc
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/renderer/main_thread_task_queue.cc
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/renderer/main_thread_task_queue.h
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/renderer/renderer_metrics_helper_unittest.cc
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.h
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.cc
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.h
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/renderer/web_frame_scheduler_impl.cc
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/test/test_task_queue.cc
[modify] https://crrev.com/ada0b50c9759a09fc38eb7331c5aa5d35266d9dc/third_party/WebKit/Source/platform/scheduler/test/test_task_queue.h

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 30 2017

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

commit 54b1525817895d9d76759ae19d21a69fe50a8a5a
Author: Alexander Timin <altimin@chromium.org>
Date: Thu Nov 30 13:42:41 2017

[scheduler] Fix problems in frame shutdown sequence

This changes are necessary for running tasks after frame detach.

- Changing time domain to a correct one in
  TaskQueueThrottler::ShutdownTaskQueue.
- Delete tasks after clearing all fields in a task queue to guarantee
  a correct shutdown for tasks which hold a reference to a task queue
  they were posted.
- Clear references to WebFrameScheduler in MainThreadTaskQueue when
  needed.
- Reset OnTaskCompleted/Started handlers when taking TaskQueueImpl from
  owning TaskQueue.

R=skyostil@chromium.org,alexclarke@chromium.org
CC=hajimehoshi@chromium.org
BUG= 770736 

Change-Id: I6028e1485db8ca77d5dda539baaed8423043aa14
Reviewed-on: https://chromium-review.googlesource.com/793816
Reviewed-by: Alex Clarke <alexclarke@chromium.org>
Commit-Queue: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520529}
[modify] https://crrev.com/54b1525817895d9d76759ae19d21a69fe50a8a5a/third_party/WebKit/Source/platform/scheduler/base/task_queue.cc
[modify] https://crrev.com/54b1525817895d9d76759ae19d21a69fe50a8a5a/third_party/WebKit/Source/platform/scheduler/base/task_queue.h
[modify] https://crrev.com/54b1525817895d9d76759ae19d21a69fe50a8a5a/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc
[modify] https://crrev.com/54b1525817895d9d76759ae19d21a69fe50a8a5a/third_party/WebKit/Source/platform/scheduler/renderer/main_thread_task_queue.cc
[modify] https://crrev.com/54b1525817895d9d76759ae19d21a69fe50a8a5a/third_party/WebKit/Source/platform/scheduler/renderer/main_thread_task_queue.h
[modify] https://crrev.com/54b1525817895d9d76759ae19d21a69fe50a8a5a/third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 1 2017

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

commit 956d2e18df7c6309303b469aebd7a53b77930880
Author: Alexander Timin <altimin@chromium.org>
Date: Fri Dec 01 10:48:16 2017

[scheduler] Run tasks after frame detach

Stop deleting tasks in task queues upon frame detach.

R=dcheng@chromium.org,hajimehoshi@chromium.org,alexclarke@chromium.org
BUG= 770736 

Change-Id: I8c0a7f6da2bd383bddf32789a1c14d1a64d2b959
Reviewed-on: https://chromium-review.googlesource.com/800943
Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
Commit-Queue: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520908}
[modify] https://crrev.com/956d2e18df7c6309303b469aebd7a53b77930880/third_party/WebKit/Source/platform/scheduler/renderer/web_frame_scheduler_impl.cc
[modify] https://crrev.com/956d2e18df7c6309303b469aebd7a53b77930880/third_party/WebKit/Source/platform/scheduler/renderer/web_view_scheduler_impl_unittest.cc

Is this finished?
Status: Fixed (was: Started)
Yes!

Sign in to add a comment