Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2445763002:
Disallow posting tasks to SequencedWorkerPools by default. (Closed)
DescriptionDisallow posting tasks to SequencedWorkerPools by default.
We have observed crash reports in which SequencedWorkerPool workers
were active while redirection to TaskScheduler was enabled. This can
happen if a task is posted to a SequencedWorkerPool before redirection
to TaskScheduler is enabled. With this CL, DumpWithoutCrashing() is
called if a task is posted to a SequencedWorkerPool too early. This
will help us find offending call sites.
A commented DCHECK is also added to find offending call sites. It
will be uncommented in a separate CL to avoid a revert of this CL
in case it fails on the waterfall.
BUG=622400
Committed: https://crrev.com/4606ada249a240e115a08a7c66409e8fdd457ea1
Committed: https://crrev.com/8fd22217395d50b6704109c7f1d4deff59f9a109
Committed: https://crrev.com/50a3834c24c1e4a4a61130db156405240b04b346
Cr-Original-Original-Commit-Position: refs/heads/master@{#432913}
Cr-Original-Commit-Position: refs/heads/master@{#433237}
Cr-Commit-Position: refs/heads/master@{#433639}
Patch Set 1 #Patch Set 2 : move #Patch Set 3 : allow test launcher #Patch Set 4 : move #Patch Set 5 : ios #Patch Set 6 : refactor #Patch Set 7 : self-review #Patch Set 8 : self-review #Patch Set 9 : enable in service process #Patch Set 10 : rebase #Patch Set 11 : fix build error #Patch Set 12 : fix build error #Patch Set 13 : fix test errors #Patch Set 14 : refactor #Patch Set 15 : refactor #Patch Set 16 : fix test errors #Patch Set 17 : self-review #Patch Set 18 : self-review #Patch Set 19 : self-review #
Total comments: 6
Patch Set 20 : CR gab #57 #
Total comments: 6
Patch Set 21 : CR gab #61 #Patch Set 22 : self-review #
Total comments: 2
Patch Set 23 : rebase #Patch Set 24 : rebase #Patch Set 25 : rebase #Patch Set 26 : prevent DCHECK failures when SWP is not enabled #Patch Set 27 : rebase #Patch Set 28 : self-review #Patch Set 29 : self-review #Messages
Total messages: 126 (101 generated)
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromeos_x86-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...) android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...) android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Disallow posting tasks to SequencedWorkerPools by default. BUG= ========== to ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. BUG=622400 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
gab@chromium.org changed reviewers: + gab@chromium.org
https://codereview.chromium.org/2445763002/diff/360001/base/threading/sequenc... File base/threading/sequenced_worker_pool.cc (right): https://codereview.chromium.org/2445763002/diff/360001/base/threading/sequenc... base/threading/sequenced_worker_pool.cc:1409: void SequencedWorkerPool::EnableForProcess() { DCHECK_EQ(POST_TASK_DISABLED, ...)? https://codereview.chromium.org/2445763002/diff/360001/base/threading/sequenc... base/threading/sequenced_worker_pool.cc:1416: DCHECK_EQ(AllPoolsState::POST_TASK_DISABLED, g_all_pools_state); Also DCHECK(TaskScheduler::GetInstance()); as per API definition? https://codereview.chromium.org/2445763002/diff/360001/chrome/browser/chrome_... File chrome/browser/chrome_browser_main.cc (right): https://codereview.chromium.org/2445763002/diff/360001/chrome/browser/chrome_... chrome/browser/chrome_browser_main.cc:1248: base::SequencedWorkerPool::EnableForProcess(); Add a bool to MaybeInitializeTaskScheduler() and make this conditional (then you can DCHECK in its impl -- I don't like that it no-ops when already enabled some other way)
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
PTAnL https://codereview.chromium.org/2445763002/diff/360001/base/threading/sequenc... File base/threading/sequenced_worker_pool.cc (right): https://codereview.chromium.org/2445763002/diff/360001/base/threading/sequenc... base/threading/sequenced_worker_pool.cc:1409: void SequencedWorkerPool::EnableForProcess() { On 2016/11/01 16:09:47, gab wrote: > DCHECK_EQ(POST_TASK_DISABLED, ...)? Done. https://codereview.chromium.org/2445763002/diff/360001/base/threading/sequenc... base/threading/sequenced_worker_pool.cc:1416: DCHECK_EQ(AllPoolsState::POST_TASK_DISABLED, g_all_pools_state); On 2016/11/01 16:09:47, gab wrote: > Also > > DCHECK(TaskScheduler::GetInstance()); > > as per API definition? Done. https://codereview.chromium.org/2445763002/diff/360001/chrome/browser/chrome_... File chrome/browser/chrome_browser_main.cc (right): https://codereview.chromium.org/2445763002/diff/360001/chrome/browser/chrome_... chrome/browser/chrome_browser_main.cc:1248: base::SequencedWorkerPool::EnableForProcess(); On 2016/11/01 16:09:48, gab wrote: > Add a bool to MaybeInitializeTaskScheduler() and make this conditional (then you > can DCHECK in its impl -- I don't like that it no-ops when already enabled some > other way) Done.
Also, add CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_tsan_rel_ng to your CL's description to catch anything now instead of on waterfall :) https://codereview.chromium.org/2445763002/diff/380001/base/threading/sequenc... File base/threading/sequenced_worker_pool.h (right): https://codereview.chromium.org/2445763002/diff/380001/base/threading/sequenc... base/threading/sequenced_worker_pool.h:205: // Returns true is posting tasks to this process' SequencedWorkerPool is s/is/if/ https://codereview.chromium.org/2445763002/diff/380001/chrome/browser/chrome_... File chrome/browser/chrome_browser_main.cc (right): https://codereview.chromium.org/2445763002/diff/380001/chrome/browser/chrome_... chrome/browser/chrome_browser_main.cc:320: void MaybeInitializeTaskScheduler(bool* sequenced_worker_pool_redirected) { Prefer return value to out-param. https://codereview.chromium.org/2445763002/diff/380001/content/browser/browse... File content/browser/browser_main_loop.cc (right): https://codereview.chromium.org/2445763002/diff/380001/content/browser/browse... content/browser/browser_main_loop.cc:738: DCHECK(!base::SequencedWorkerPool::IsEnabled()); DCHECK are supposed to document invariants, so a DCHECK followed by an if doesn't make sense.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
Description was changed from ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. BUG=622400 ========== to ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. BUG=622400 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_tsan_rel_ng ==========
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
PTAnL https://codereview.chromium.org/2445763002/diff/380001/base/threading/sequenc... File base/threading/sequenced_worker_pool.h (right): https://codereview.chromium.org/2445763002/diff/380001/base/threading/sequenc... base/threading/sequenced_worker_pool.h:205: // Returns true is posting tasks to this process' SequencedWorkerPool is On 2016/11/01 21:48:02, gab wrote: > s/is/if/ Done. https://codereview.chromium.org/2445763002/diff/380001/chrome/browser/chrome_... File chrome/browser/chrome_browser_main.cc (right): https://codereview.chromium.org/2445763002/diff/380001/chrome/browser/chrome_... chrome/browser/chrome_browser_main.cc:320: void MaybeInitializeTaskScheduler(bool* sequenced_worker_pool_redirected) { On 2016/11/01 21:48:02, gab wrote: > Prefer return value to out-param. Done. https://codereview.chromium.org/2445763002/diff/380001/content/browser/browse... File content/browser/browser_main_loop.cc (right): https://codereview.chromium.org/2445763002/diff/380001/content/browser/browse... content/browser/browser_main_loop.cc:738: DCHECK(!base::SequencedWorkerPool::IsEnabled()); On 2016/11/01 21:48:02, gab wrote: > DCHECK are supposed to document invariants, so a DCHECK followed by an if > doesn't make sense. Added comment to clarify. SequencedWorkerPool is not enabled when BrowserMainLoop::PreCreateThreads enters. In Chrome, it is enabled by parts_->PreCreateThreads() below. In other products (android webview, cast...) it is enabled by base::SequencedWorkerPool::EnableForProcess() below (surrounded by an if).
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm https://codereview.chromium.org/2445763002/diff/420001/chrome/browser/chrome_... File chrome/browser/chrome_browser_main.cc (right): https://codereview.chromium.org/2445763002/diff/420001/chrome/browser/chrome_... chrome/browser/chrome_browser_main.cc:332: return sequenced_worker_pool_redirected; No var, just return false on each failure and return true if reaching the end of the method.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
fdoray@chromium.org changed reviewers: + brettw@chromium.org
brettw@: PTAL https://codereview.chromium.org/2445763002/diff/420001/chrome/browser/chrome_... File chrome/browser/chrome_browser_main.cc (right): https://codereview.chromium.org/2445763002/diff/420001/chrome/browser/chrome_... chrome/browser/chrome_browser_main.cc:332: return sequenced_worker_pool_redirected; On 2016/11/02 13:29:55, gab wrote: > No var, just return false on each failure and return true if reaching the end of > the method. We may reach the end of the method without having enabled redirection of SequencedWorkerPool to TaskScheduler. I don't want to "return true;" right after line 344 either because this early termination would prevent us from adding new code at the end of this function (e.g. I have a pending CL which enables redirection of HistoryService to TaskScheduler at the end of this function). Returning false instead of |sequenced_worker_pool_redirected| above would work but IMO would be less clear.
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_clang on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
lgtm
Description was changed from ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. BUG=622400 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_tsan_rel_ng ========== to ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. BUG=622400 ==========
The CQ bit was checked by fdoray@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from gab@chromium.org Link to the patchset: https://codereview.chromium.org/2445763002/#ps460001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by fdoray@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by fdoray@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. BUG=622400 ========== to ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. BUG=622400 ==========
Message was sent while issue was closed.
Committed patchset #24 (id:460001)
Message was sent while issue was closed.
Description was changed from ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. BUG=622400 ========== to ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. BUG=622400 Committed: https://crrev.com/4606ada249a240e115a08a7c66409e8fdd457ea1 Cr-Commit-Position: refs/heads/master@{#432913} ==========
Message was sent while issue was closed.
Patchset 24 (id:??) landed as https://crrev.com/4606ada249a240e115a08a7c66409e8fdd457ea1 Cr-Commit-Position: refs/heads/master@{#432913}
Message was sent while issue was closed.
A revert of this CL (patchset #24 id:460001) has been created in https://codereview.chromium.org/2517443002/ by estade@chromium.org. The reason for reverting is: Probable cause of test failures: "cronet_test_instrumentation_apk" on "Android Cronet Builder (dbg)" https://build.chromium.org/p/chromium.android/builders/Android%20Cronet%20Bui....
Message was sent while issue was closed.
Description was changed from ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. BUG=622400 Committed: https://crrev.com/4606ada249a240e115a08a7c66409e8fdd457ea1 Cr-Commit-Position: refs/heads/master@{#432913} ========== to ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. BUG=622400 Committed: https://crrev.com/4606ada249a240e115a08a7c66409e8fdd457ea1 Cr-Commit-Position: refs/heads/master@{#432913} ==========
Description was changed from ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. BUG=622400 Committed: https://crrev.com/4606ada249a240e115a08a7c66409e8fdd457ea1 Cr-Commit-Position: refs/heads/master@{#432913} ========== to ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing() is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. A commented DCHECK is also added to find offending call sites. It will be uncommented in a separate CL to avoid a revert of this CL in case it fails on the waterfall. BUG=622400 Committed: https://crrev.com/4606ada249a240e115a08a7c66409e8fdd457ea1 Cr-Commit-Position: refs/heads/master@{#432913} ==========
Re-landing with commented DCHECK...
The CQ bit was checked by fdoray@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from gab@chromium.org, brettw@chromium.org Link to the patchset: https://codereview.chromium.org/2445763002/#ps480001 (title: "comment DCHECK rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing() is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. A commented DCHECK is also added to find offending call sites. It will be uncommented in a separate CL to avoid a revert of this CL in case it fails on the waterfall. BUG=622400 Committed: https://crrev.com/4606ada249a240e115a08a7c66409e8fdd457ea1 Cr-Commit-Position: refs/heads/master@{#432913} ========== to ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing() is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. A commented DCHECK is also added to find offending call sites. It will be uncommented in a separate CL to avoid a revert of this CL in case it fails on the waterfall. BUG=622400 Committed: https://crrev.com/4606ada249a240e115a08a7c66409e8fdd457ea1 Cr-Commit-Position: refs/heads/master@{#432913} ==========
Message was sent while issue was closed.
Committed patchset #25 (id:480001)
Message was sent while issue was closed.
Description was changed from ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing() is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. A commented DCHECK is also added to find offending call sites. It will be uncommented in a separate CL to avoid a revert of this CL in case it fails on the waterfall. BUG=622400 Committed: https://crrev.com/4606ada249a240e115a08a7c66409e8fdd457ea1 Cr-Commit-Position: refs/heads/master@{#432913} ========== to ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing() is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. A commented DCHECK is also added to find offending call sites. It will be uncommented in a separate CL to avoid a revert of this CL in case it fails on the waterfall. BUG=622400 Committed: https://crrev.com/4606ada249a240e115a08a7c66409e8fdd457ea1 Committed: https://crrev.com/8fd22217395d50b6704109c7f1d4deff59f9a109 Cr-Original-Commit-Position: refs/heads/master@{#432913} Cr-Commit-Position: refs/heads/master@{#433237} ==========
Message was sent while issue was closed.
Patchset 25 (id:??) landed as https://crrev.com/8fd22217395d50b6704109c7f1d4deff59f9a109 Cr-Commit-Position: refs/heads/master@{#433237}
Message was sent while issue was closed.
A revert of this CL (patchset #25 id:480001) has been created in https://codereview.chromium.org/2519543002/ by fsamuel@chromium.org. The reason for reverting is: chrome --mash crashing..
Message was sent while issue was closed.
Description was changed from ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing() is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. A commented DCHECK is also added to find offending call sites. It will be uncommented in a separate CL to avoid a revert of this CL in case it fails on the waterfall. BUG=622400 Committed: https://crrev.com/4606ada249a240e115a08a7c66409e8fdd457ea1 Committed: https://crrev.com/8fd22217395d50b6704109c7f1d4deff59f9a109 Cr-Original-Commit-Position: refs/heads/master@{#432913} Cr-Commit-Position: refs/heads/master@{#433237} ========== to ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing() is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. A commented DCHECK is also added to find offending call sites. It will be uncommented in a separate CL to avoid a revert of this CL in case it fails on the waterfall. BUG=622400 Committed: https://crrev.com/4606ada249a240e115a08a7c66409e8fdd457ea1 Committed: https://crrev.com/8fd22217395d50b6704109c7f1d4deff59f9a109 Cr-Original-Commit-Position: refs/heads/master@{#432913} Cr-Commit-Position: refs/heads/master@{#433237} ==========
On 2016/11/18 19:34:13, Fady Samuel wrote: > A revert of this CL (patchset #25 id:480001) has been created in > https://codereview.chromium.org/2519543002/ by mailto:fsamuel@chromium.org. > > The reason for reverting is: chrome --mash crashing.. Re-landing without DCHECK failure in processes where SequencedWorkerPool is not enabled. DCHECKs will be added in a separate CL.
The CQ bit was checked by fdoray@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from gab@chromium.org, brettw@chromium.org Link to the patchset: https://codereview.chromium.org/2445763002/#ps560001 (title: "self-review")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by fdoray@chromium.org
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by fdoray@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 560001, "attempt_start_ts": 1479760757093160,
"parent_rev": "ac913641de8e23d631e73f108ad1598b1db0bc6f", "commit_rev":
"13a06f8825b7a20c3ca662cbe9cd9b36e3acb3ef"}
Message was sent while issue was closed.
Description was changed from ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing() is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. A commented DCHECK is also added to find offending call sites. It will be uncommented in a separate CL to avoid a revert of this CL in case it fails on the waterfall. BUG=622400 Committed: https://crrev.com/4606ada249a240e115a08a7c66409e8fdd457ea1 Committed: https://crrev.com/8fd22217395d50b6704109c7f1d4deff59f9a109 Cr-Original-Commit-Position: refs/heads/master@{#432913} Cr-Commit-Position: refs/heads/master@{#433237} ========== to ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing() is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. A commented DCHECK is also added to find offending call sites. It will be uncommented in a separate CL to avoid a revert of this CL in case it fails on the waterfall. BUG=622400 Committed: https://crrev.com/4606ada249a240e115a08a7c66409e8fdd457ea1 Committed: https://crrev.com/8fd22217395d50b6704109c7f1d4deff59f9a109 Cr-Original-Commit-Position: refs/heads/master@{#432913} Cr-Commit-Position: refs/heads/master@{#433237} ==========
Message was sent while issue was closed.
Committed patchset #29 (id:560001)
Message was sent while issue was closed.
Description was changed from ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing() is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. A commented DCHECK is also added to find offending call sites. It will be uncommented in a separate CL to avoid a revert of this CL in case it fails on the waterfall. BUG=622400 Committed: https://crrev.com/4606ada249a240e115a08a7c66409e8fdd457ea1 Committed: https://crrev.com/8fd22217395d50b6704109c7f1d4deff59f9a109 Cr-Original-Commit-Position: refs/heads/master@{#432913} Cr-Commit-Position: refs/heads/master@{#433237} ========== to ========== Disallow posting tasks to SequencedWorkerPools by default. We have observed crash reports in which SequencedWorkerPool workers were active while redirection to TaskScheduler was enabled. This can happen if a task is posted to a SequencedWorkerPool before redirection to TaskScheduler is enabled. With this CL, DumpWithoutCrashing() is called if a task is posted to a SequencedWorkerPool too early. This will help us find offending call sites. A commented DCHECK is also added to find offending call sites. It will be uncommented in a separate CL to avoid a revert of this CL in case it fails on the waterfall. BUG=622400 Committed: https://crrev.com/4606ada249a240e115a08a7c66409e8fdd457ea1 Committed: https://crrev.com/8fd22217395d50b6704109c7f1d4deff59f9a109 Committed: https://crrev.com/50a3834c24c1e4a4a61130db156405240b04b346 Cr-Original-Original-Commit-Position: refs/heads/master@{#432913} Cr-Original-Commit-Position: refs/heads/master@{#433237} Cr-Commit-Position: refs/heads/master@{#433639} ==========
Message was sent while issue was closed.
Patchset 29 (id:??) landed as https://crrev.com/50a3834c24c1e4a4a61130db156405240b04b346 Cr-Commit-Position: refs/heads/master@{#433639} |
