Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(11)

Issue 2812293002: Flush TaskScheduler in ~ScopedTaskEnvironment and ~ScopedAsyncTaskScheduler. (Closed)

Created:
2 years, 1 month ago by fdoray
Modified:
2 years, 1 month ago
Reviewers:
gab
CC:
chromium-reviews, danakj+watch_chromium.org, vmpstr+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Flush TaskScheduler in ~ScopedTaskEnvironment and ~ScopedAsyncTaskScheduler. Skipping tasks when destroying ScopedTaskEnvironment or ScopedAsyncTaskScheduler means that DeleteSoon() tasks can be skipped, resulting in memory leaks. BUG=708584, 709095 Review-Url: https://codereview.chromium.org/2812293002 Cr-Commit-Position: refs/heads/master@{#464380} Committed: https://chromium.googlesource.com/chromium/src/+/6044a51d3e0eff211412224d21f62777db7641a4

Patch Set 1 #

Total comments: 2

Patch Set 2 : CR-gab-8-add-comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -0 lines) Patch
M base/test/scoped_async_task_scheduler.cc View 1 1 chunk +3 lines, -0 lines 0 comments Download
M base/test/scoped_task_environment.cc View 1 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 22 (16 generated)
fdoray
PTAL
2 years, 1 month ago (2017-04-12 15:45:12 UTC) #4
gab
lgtm w/ comment https://codereview.chromium.org/2812293002/diff/1/base/test/scoped_task_environment.cc File base/test/scoped_task_environment.cc (right): https://codereview.chromium.org/2812293002/diff/1/base/test/scoped_task_environment.cc#newcode35 base/test/scoped_task_environment.cc:35: TaskScheduler::GetInstance()->FlushForTesting(); Add a comment explaining why ...
2 years, 1 month ago (2017-04-12 19:18:34 UTC) #8
fdoray
https://codereview.chromium.org/2812293002/diff/1/base/test/scoped_task_environment.cc File base/test/scoped_task_environment.cc (right): https://codereview.chromium.org/2812293002/diff/1/base/test/scoped_task_environment.cc#newcode35 base/test/scoped_task_environment.cc:35: TaskScheduler::GetInstance()->FlushForTesting(); On 2017/04/12 19:18:34, gab wrote: > Add a ...
2 years, 1 month ago (2017-04-12 19:38:06 UTC) #14
commit-bot: I haz the power
Prior attempt to commit was detected, but we were not able to check whether the ...
2 years, 1 month ago (2017-04-13 01:30:58 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2812293002/20001
2 years, 1 month ago (2017-04-13 12:00:58 UTC) #19
commit-bot: I haz the power
2 years, 1 month ago (2017-04-13 12:05:14 UTC) #22
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/6044a51d3e0eff211412224d21f6...

Powered by Google App Engine
This is Rietveld 408576698