New issue
Advanced search Search tips

Issue 844016 link

Starred by 1 user
Status: Fixed
Owner:
w...@chromium.org
Closed: Jul 22
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac , Fuchsia
Pri: 2
Type: Bug



Sign in to add a comment

Quit the BrowserMainLoop via RunLoop::QuitClosure() rather than RunLoop::QuitCurrentWhenIdleDeprecated

Project Member Reported by w...@chromium.org, May 17 2018 
Since the correct lifetime management of the Browser and Profile objects depends upon BrowserProcess::Unpin() being the sole controller of when to exit the browser RunLoop, there is a risk that other code intended to run in the context of a nested RunLoop accidentally causes the main RunLoop to quit (and thereby exit or crash the browser process) by calling QuitCurrentWhenIdleDeprecated.

Mediaum/long-term, we should migrate callers off that API and remove it.

Short-term we can address the special-case of the BrowserMainLoop's RunLoop by migrating BrowserProcess to using the RunLoop's QuitClosure to quit it, and then disallowing QuitCurrentWhenIdleDeprecated from being called on it, so that broken call-sites can be easily identified, and fixed.

Comment 1 by w...@chromium.org, May 17 2018 

This bug was filed as part of proposals to address issue 720078.
Project Member

Comment 2 by bugdroid1@chromium.org, May 17 2018 

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

commit a78944d35a94696ac1aeb0ee0d94ca7a57cde2d5
Author: Wez <wez@chromium.org>
Date: Thu May 17 16:56:29 2018

Add missing expectation to RepeatedNotificationObserver test helper.

This test helper is intended to verify that the expected number of
notifications are received, but relies on a RunLoop which can be quit
too soon by QuitCurrentWhenIdleDeprecated() calls by code-under-test.

Bug:  844016 , 844019
Change-Id: I416d24e2accd4ef3834197dd50a445db8fe801d0
Reviewed-on: https://chromium-review.googlesource.com/1064000
Commit-Queue: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559572}
[modify] https://crrev.com/a78944d35a94696ac1aeb0ee0d94ca7a57cde2d5/chrome/browser/lifetime/browser_close_manager_browsertest.cc
Project Member

Comment 3 by bugdroid1@chromium.org, May 25 2018 

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

commit 25fd35f2fb3113198c239b3b99ea732b4d3a43c6
Author: Wez <wez@chromium.org>
Date: Fri May 25 22:57:41 2018

Quit browser RunLoop via injected Closure rather than via QuitCurrent*.

Use of QuitCurrent*Deprecated() is ambiguous, since RunLoops can be
nested, and so is unsafe to use in general.

TBR: bartfab
Bug:  845966 ,  844016 , 749312
Change-Id: I537862ea02c4ed67887db32993f40e53f614b3cd
Reviewed-on: https://chromium-review.googlesource.com/1019967
Reviewed-by: Wez <wez@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562046}
[modify] https://crrev.com/25fd35f2fb3113198c239b3b99ea732b4d3a43c6/chrome/browser/apps/app_shim/app_shim_quit_interactive_uitest_mac.mm
[modify] https://crrev.com/25fd35f2fb3113198c239b3b99ea732b4d3a43c6/chrome/browser/apps/app_window_interactive_uitest.cc
[modify] https://crrev.com/25fd35f2fb3113198c239b3b99ea732b4d3a43c6/chrome/browser/browser_process_impl.cc
[modify] https://crrev.com/25fd35f2fb3113198c239b3b99ea732b4d3a43c6/chrome/browser/browser_process_impl.h
[modify] https://crrev.com/25fd35f2fb3113198c239b3b99ea732b4d3a43c6/chrome/browser/chrome_browser_main.cc
[modify] https://crrev.com/25fd35f2fb3113198c239b3b99ea732b4d3a43c6/chrome/browser/chrome_browser_main.h
[modify] https://crrev.com/25fd35f2fb3113198c239b3b99ea732b4d3a43c6/chrome/browser/chromeos/login/auto_launched_kiosk_browsertest.cc
[modify] https://crrev.com/25fd35f2fb3113198c239b3b99ea732b4d3a43c6/chrome/browser/chromeos/login/kiosk_browsertest.cc
[modify] https://crrev.com/25fd35f2fb3113198c239b3b99ea732b4d3a43c6/chrome/browser/chromeos/login/oobe_browsertest.cc
[modify] https://crrev.com/25fd35f2fb3113198c239b3b99ea732b4d3a43c6/chrome/browser/chromeos/policy/device_system_use_24hour_clock_browsertest.cc
[modify] https://crrev.com/25fd35f2fb3113198c239b3b99ea732b4d3a43c6/chrome/browser/chromeos/policy/display_rotation_default_handler_browsertest.cc
[modify] https://crrev.com/25fd35f2fb3113198c239b3b99ea732b4d3a43c6/chrome/browser/chromeos/policy/unaffiliated_arc_allowed_browsertest.cc
[modify] https://crrev.com/25fd35f2fb3113198c239b3b99ea732b4d3a43c6/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos_browsertest.cc
[modify] https://crrev.com/25fd35f2fb3113198c239b3b99ea732b4d3a43c6/chrome/browser/chromeos/shutdown_policy_browsertest.cc
[modify] https://crrev.com/25fd35f2fb3113198c239b3b99ea732b4d3a43c6/chrome/browser/ui/cocoa/first_run_dialog.mm
[modify] https://crrev.com/25fd35f2fb3113198c239b3b99ea732b4d3a43c6/chrome/browser/ui/tab_modal_confirm_dialog_browsertest.cc
[modify] https://crrev.com/25fd35f2fb3113198c239b3b99ea732b4d3a43c6/chrome/test/base/in_process_browser_test.cc
[modify] https://crrev.com/25fd35f2fb3113198c239b3b99ea732b4d3a43c6/chrome/test/base/in_process_browser_test.h

Comment 5 by w...@chromium.org, Jun 6 2018

Blockedon: 850145
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 7 2018 

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

commit 927db59630b51307f33eaf3d2b01ec90b8f36fdc
Author: Wez <wez@chromium.org>
Date: Thu Jun 07 03:20:52 2018

Add new RunUntilComplete() and set_on_<event>() APIs to TestDelegate.

- Simple tests can use RunUntilComplete() to wait specifically for
  completion of a request.
- More complex tests can use set_on_<event>() to provide closures to
  invoke on completion, redirect, or auth-required. These will usually
  be passed a RunLoop's QuitClosure() to invoke.
- set_on_<event>() can be called with a null RepeatingClosure to
  cause that event to be silently ignored.

TestDelegate's default behaviour is unchanged, and the "legacy" APIs
to set_quit_on_<event>() flags are still available, so that tests can
be migrated incrementally.

Bug:  844016 , 850145
Change-Id: I5d6aea0a6a25a67f388ddb91c685b29d8b9eb6a8
Reviewed-on: https://chromium-review.googlesource.com/1088153
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Eric Roman <eroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565163}
[modify] https://crrev.com/927db59630b51307f33eaf3d2b01ec90b8f36fdc/net/url_request/url_request_test_util.cc
[modify] https://crrev.com/927db59630b51307f33eaf3d2b01ec90b8f36fdc/net/url_request/url_request_test_util.h
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 7 2018 

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

commit 6a139c4aeea722bec03c7b2d48be379188a3612e
Author: Wez <wez@chromium.org>
Date: Thu Jun 07 16:01:24 2018

Migrate some History tests off RunLoop::QuitCurrent*Deprecated.

These tests mix use of QuitCurrent*Deprecated() with use of the current
RunLoop::Quit*Closure() model. Migrate them off the deprecated API so
that we can enforce quit-by-closure-only wherever QuitClosure is used.

Bug:  844016 
Change-Id: I5aca4ada0678f0cb1d84ced9fef9495fba751a7b
Reviewed-on: https://chromium-review.googlesource.com/1090213
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565282}
[modify] https://crrev.com/6a139c4aeea722bec03c7b2d48be379188a3612e/components/history/core/browser/history_querying_unittest.cc
[modify] https://crrev.com/6a139c4aeea722bec03c7b2d48be379188a3612e/components/history/core/browser/history_service_unittest.cc
Project Member

Comment 8 by bugdroid1@chromium.org, Jun 7 2018 

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

commit 1aa68b3cf1b71508ba2fdbee2be3d0376e3627ea
Author: Wez <wez@chromium.org>
Date: Thu Jun 07 16:07:15 2018

Migrate some Autofill tests off RunLoop::QuitCurrent*Deprecated.

These tests mix use of QuitCurrent*Deprecated() with use of the current
RunLoop::Quit*Closure() model. Migrate them off the deprecated API so
that we can enforce quit-by-closure-only wherever QuitClosure is used.

Bug:  844016 
Change-Id: Ib93c6419a81642a3d374ec08a3c71ddaacc3cb88
Reviewed-on: https://chromium-review.googlesource.com/1090211
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565285}
[modify] https://crrev.com/1aa68b3cf1b71508ba2fdbee2be3d0376e3627ea/components/autofill/core/browser/autofill_download_manager_unittest.cc
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 7 2018 

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

commit 9a58a15ae3fca2ac65ae394982bd1734ee04171c
Author: Wez <wez@chromium.org>
Date: Thu Jun 07 18:59:33 2018

Migrate content::Shell off RunLoop::QuitCurrent*Deprecated.

content:Shell used QuitCurrent*Deprecated(), while many of the //content
browser-tests use the modern RunLoop::Quit*Closure() mechanism.

Migrate content::Shell off the deprecated API, so that we can enforce
quit-by-closure-only wherever QuitClosure is used.

TBR: kinuko
Bug:  844016 
Change-Id: I661c3ed1c799680dea9760d3fb43d78aa8039897
Reviewed-on: https://chromium-review.googlesource.com/1090219
Reviewed-by: Wez <wez@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565369}
[modify] https://crrev.com/9a58a15ae3fca2ac65ae394982bd1734ee04171c/content/browser/service_worker/service_worker_browsertest.cc
[modify] https://crrev.com/9a58a15ae3fca2ac65ae394982bd1734ee04171c/content/shell/browser/shell.cc
[modify] https://crrev.com/9a58a15ae3fca2ac65ae394982bd1734ee04171c/content/shell/browser/shell.h
Project Member

Comment 10 by bugdroid1@chromium.org, Jun 7 2018 

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

commit d528644cfebdf7c8d81c419a1205de5b2d6b5dc4
Author: Wez <wez@chromium.org>
Date: Thu Jun 07 19:07:45 2018

Migrate some Sync tests off RunLoop::QuitCurrent*Deprecated.

These tests mix use of QuitCurrent*Deprecated() with use of the current
RunLoop::Quit*Closure() model. Migrate them off the deprecated API so
that we can enforce quit-by-closure-only wherever QuitClosure is used.

Bug:  844016 
Change-Id: I5c8b2557d361ea9ade9f5da0440bac7f7878440e
Reviewed-on: https://chromium-review.googlesource.com/1090214
Reviewed-by: Nicolas Zea <zea@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565371}
[modify] https://crrev.com/d528644cfebdf7c8d81c419a1205de5b2d6b5dc4/components/sync/driver/glue/sync_backend_host_impl_unittest.cc
Project Member

Comment 11 by bugdroid1@chromium.org, Jun 7 2018 

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

commit fda077b4480f46c94c830e803651762c3b9f574d
Author: Wez <wez@chromium.org>
Date: Thu Jun 07 21:18:11 2018

Migrate some //base tests off RunLoop::QuitCurrent*Deprecated.

These tests mix use of QuitCurrent*Deprecated() with use of the current
RunLoop::Quit*Closure() model. Migrate them off the deprecated API so
that we can enforce quit-by-closure-only wherever QuitClosure is used.

Bug:  844016 
Change-Id: Ie832373ecba2060b860b9f49e056f437c30b9b7e
Reviewed-on: https://chromium-review.googlesource.com/1089829
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565412}
[modify] https://crrev.com/fda077b4480f46c94c830e803651762c3b9f574d/base/files/file_path_watcher_unittest.cc
[modify] https://crrev.com/fda077b4480f46c94c830e803651762c3b9f574d/base/message_loop/message_pump_glib_unittest.cc
[modify] https://crrev.com/fda077b4480f46c94c830e803651762c3b9f574d/base/observer_list_unittest.cc
Project Member

Comment 12 by bugdroid1@chromium.org, Jun 18 2018 

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

commit 1f9dbee1cfe5c4d27fc478b276189dacd54067a6
Author: Wez <wez@chromium.org>
Date: Mon Jun 18 19:25:35 2018

Exit HeadlessBrowserImpl main message loop by QuitClosure.

Replace use of QuitCurrent*Deprecated() with use of the QuitClosure of
the //content default main message loop's RunLoop.

Bug:  844016 
Change-Id: I14eb18304de9c9ddaabc2860998d12733572e7f7
Reviewed-on: https://chromium-review.googlesource.com/1103787
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568103}
[modify] https://crrev.com/1f9dbee1cfe5c4d27fc478b276189dacd54067a6/headless/lib/browser/headless_browser_impl.cc
[modify] https://crrev.com/1f9dbee1cfe5c4d27fc478b276189dacd54067a6/headless/lib/browser/headless_browser_main_parts.cc
[modify] https://crrev.com/1f9dbee1cfe5c4d27fc478b276189dacd54067a6/headless/lib/browser/headless_browser_main_parts.h

Comment 13 by w...@chromium.org, Jun 19 2018

Blockedon: -850145
Project Member

Comment 14 by bugdroid1@chromium.org, Jun 20 2018 

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

commit 7d3eb016cc02f4e66c6f9bf6b05793f46d907b4a
Author: Wez <wez@chromium.org>
Date: Wed Jun 20 22:51:28 2018

Update LayoutTestBrowserMain and BlinkTestController loop quit logic.

Migrate the LayoutTestBrowserMain and BlinkTestController classes to use
the quit closure registered with the content::Shell by the default main
message loop, to exit it, rather than QuitCurrent*Deprecated().

Bug:  844016 
Change-Id: I0b7e8249df6ed62757e5bb27c51dd16b35e0e4a6
Reviewed-on: https://chromium-review.googlesource.com/1106814
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569063}
[modify] https://crrev.com/7d3eb016cc02f4e66c6f9bf6b05793f46d907b4a/content/shell/browser/layout_test/blink_test_controller.cc
[modify] https://crrev.com/7d3eb016cc02f4e66c6f9bf6b05793f46d907b4a/content/shell/browser/layout_test/layout_test_browser_main.cc
[modify] https://crrev.com/7d3eb016cc02f4e66c6f9bf6b05793f46d907b4a/content/shell/browser/shell.cc
[modify] https://crrev.com/7d3eb016cc02f4e66c6f9bf6b05793f46d907b4a/content/shell/browser/shell.h
Project Member

Comment 15 by bugdroid1@chromium.org, Jun 22 2018 

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

commit a647c750dfb95d621807607c75939b8286c7acae
Author: Wez <wez@chromium.org>
Date: Fri Jun 22 09:13:27 2018

Update ChromeOS tests to use QuitClosure to terminate RunLoops.

These tests previously relied upon QuitCurrent*Deprecated() to terminate
RunLoops when the expected conditions arose.

Bug:  844016 
Change-Id: I290599b94936b2ef253925f6f58b67d017b4603b
Reviewed-on: https://chromium-review.googlesource.com/1107242
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569563}
[modify] https://crrev.com/a647c750dfb95d621807607c75939b8286c7acae/chrome/browser/chromeos/input_method/input_method_engine_browsertests.cc
[modify] https://crrev.com/a647c750dfb95d621807607c75939b8286c7acae/chrome/browser/chromeos/login/auth/cryptohome_authenticator_unittest.cc
[modify] https://crrev.com/a647c750dfb95d621807607c75939b8286c7acae/chrome/browser/chromeos/login/lock/screen_locker_browsertest.cc
[modify] https://crrev.com/a647c750dfb95d621807607c75939b8286c7acae/chrome/browser/chromeos/policy/device_local_account_browsertest.cc
[modify] https://crrev.com/a647c750dfb95d621807607c75939b8286c7acae/chromeos/login/auth/mock_auth_status_consumer.cc
[modify] https://crrev.com/a647c750dfb95d621807607c75939b8286c7acae/chromeos/login/auth/mock_auth_status_consumer.h
Project Member

Comment 16 by bugdroid1@chromium.org, Jun 29 2018 

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

commit b035cfda675fd58c44789cc3779f2b247f613cb6
Author: Wez <wez@chromium.org>
Date: Fri Jun 29 16:06:17 2018

Disallow QuitCurrent*Deprecated() on RunLoops with Quit*Closure()s.

Use of RunLoop::QuitCurrent*Deprecated() is deprecated, and risky,
potentially causing unexpected early-exits if run under a different
RunLoop to the one intended.

If a caller has taken a Quit*Closure() from the RunLoop then we now
assume that they intend to use it to quit the loop, and therefore use
of QuitCurrent*Deprecated() with it must be unintentional.

Bug:  844016 
Change-Id: Ib181d0cb34cdba2af29f557646cbe631101bc6b0
Reviewed-on: https://chromium-review.googlesource.com/1082980
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571499}
[modify] https://crrev.com/b035cfda675fd58c44789cc3779f2b247f613cb6/base/files/file_path_watcher_unittest.cc
[modify] https://crrev.com/b035cfda675fd58c44789cc3779f2b247f613cb6/base/message_loop/message_loop_unittest.cc
[modify] https://crrev.com/b035cfda675fd58c44789cc3779f2b247f613cb6/base/run_loop.cc
[modify] https://crrev.com/b035cfda675fd58c44789cc3779f2b247f613cb6/base/run_loop.h
[modify] https://crrev.com/b035cfda675fd58c44789cc3779f2b247f613cb6/chrome/browser/extensions/api/tab_capture/tab_capture_apitest.cc
[modify] https://crrev.com/b035cfda675fd58c44789cc3779f2b247f613cb6/chrome/browser/sync_file_system/local/syncable_file_operation_runner_unittest.cc
[modify] https://crrev.com/b035cfda675fd58c44789cc3779f2b247f613cb6/chrome/common/mac/mock_launchd.cc
[modify] https://crrev.com/b035cfda675fd58c44789cc3779f2b247f613cb6/chrome/common/mac/mock_launchd.h
[modify] https://crrev.com/b035cfda675fd58c44789cc3779f2b247f613cb6/chrome/common/service_process_util_mac_unittest.mm

Comment 17 by w...@chromium.org, Jun 29 2018

Status: Fixed (was: Started)

Comment 18 by w...@chromium.org, Jun 29 2018 

Filed  issue 859095  to track some remaining QuitCurrent*Deprecated() cleanups.
Project Member

Comment 19 by bugdroid1@chromium.org, Jun 29 2018 

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

commit b397350defc64b2c786cb7d209250ca92fa80733
Author: Yi Gu <yigu@chromium.org>
Date: Fri Jun 29 19:32:31 2018

Revert "Disallow QuitCurrent*Deprecated() on RunLoops with Quit*Closure()s."

This reverts commit b035cfda675fd58c44789cc3779f2b247f613cb6.

Reason for revert: Failing DistillablePageUtilsBrowserTestAlways.TestDelegate on Linux dbg. See  crbug.com/859162  for crash log.

Original change's description:
> Disallow QuitCurrent*Deprecated() on RunLoops with Quit*Closure()s.
> 
> Use of RunLoop::QuitCurrent*Deprecated() is deprecated, and risky,
> potentially causing unexpected early-exits if run under a different
> RunLoop to the one intended.
> 
> If a caller has taken a Quit*Closure() from the RunLoop then we now
> assume that they intend to use it to quit the loop, and therefore use
> of QuitCurrent*Deprecated() with it must be unintentional.
> 
> Bug:  844016 
> Change-Id: Ib181d0cb34cdba2af29f557646cbe631101bc6b0
> Reviewed-on: https://chromium-review.googlesource.com/1082980
> Commit-Queue: Nico Weber <thakis@chromium.org>
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Reviewed-by: Gabriel Charette <gab@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#571499}

TBR=wez@chromium.org,gab@chromium.org,thakis@chromium.org

Change-Id: Ib5e62d0ae045dd7014d3b546d5970c9dac92edd7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  844016 ,  859162 
Reviewed-on: https://chromium-review.googlesource.com/1121037
Reviewed-by: Yi Gu <yigu@chromium.org>
Commit-Queue: Yi Gu <yigu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571591}
[modify] https://crrev.com/b397350defc64b2c786cb7d209250ca92fa80733/base/files/file_path_watcher_unittest.cc
[modify] https://crrev.com/b397350defc64b2c786cb7d209250ca92fa80733/base/message_loop/message_loop_unittest.cc
[modify] https://crrev.com/b397350defc64b2c786cb7d209250ca92fa80733/base/run_loop.cc
[modify] https://crrev.com/b397350defc64b2c786cb7d209250ca92fa80733/base/run_loop.h
[modify] https://crrev.com/b397350defc64b2c786cb7d209250ca92fa80733/chrome/browser/extensions/api/tab_capture/tab_capture_apitest.cc
[modify] https://crrev.com/b397350defc64b2c786cb7d209250ca92fa80733/chrome/browser/sync_file_system/local/syncable_file_operation_runner_unittest.cc
[modify] https://crrev.com/b397350defc64b2c786cb7d209250ca92fa80733/chrome/common/mac/mock_launchd.cc
[modify] https://crrev.com/b397350defc64b2c786cb7d209250ca92fa80733/chrome/common/mac/mock_launchd.h
[modify] https://crrev.com/b397350defc64b2c786cb7d209250ca92fa80733/chrome/common/service_process_util_mac_unittest.mm

Comment 20 by w...@chromium.org, Jun 29 2018

Status: Started (was: Fixed)
Project Member

Comment 22 by bugdroid1@chromium.org, Jul 5 

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

commit e36e56dc743f9f231d38517d5e94d076f042793e
Author: Wez <wez@chromium.org>
Date: Thu Jul 05 18:02:23 2018

Migrate GeolocationProviderTest off of QuitCurrent*Deprecated().

Bug:  844016 
Change-Id: I3e8ca5aba12fa7cf7954f945d9626f22d7fd9cdf
Reviewed-on: https://chromium-review.googlesource.com/1126562
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Tim Volodine <timvolodine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572828}
[modify] https://crrev.com/e36e56dc743f9f231d38517d5e94d076f042793e/services/device/geolocation/geolocation_provider_impl_unittest.cc
Project Member

Comment 23 by bugdroid1@chromium.org, Jul 6 

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

commit b14c3045a21f38436c7bda0e22d303c2155ef53c
Author: Wez <wez@chromium.org>
Date: Fri Jul 06 05:02:51 2018

Migrate some browser tests off of QuitCurrent*Deprecated().

Bug:  844016 
Change-Id: Ifbec297a849c83509ea6d7d12eabd37b2262d63a
Reviewed-on: https://chromium-review.googlesource.com/1126575
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Alexandr Ilin <alexilin@chromium.org>
Reviewed-by: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572897}
[modify] https://crrev.com/b14c3045a21f38436c7bda0e22d303c2155ef53c/chrome/browser/net/predictor_browsertest.cc
[modify] https://crrev.com/b14c3045a21f38436c7bda0e22d303c2155ef53c/chrome/browser/ui/webui/constrained_web_dialog_ui_browsertest.cc
Project Member

Comment 24 by bugdroid1@chromium.org, Jul 6 

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

commit c0998d0892e0e328b6bdb553f58f092b0c90fb8e
Author: Wez <wez@chromium.org>
Date: Fri Jul 06 17:50:18 2018

Migrate PolicyTest off of QuitCurrent*Deprecated.

Bug:  844016 
Change-Id: I900353041ecc56881786637592f0bac7377b9600
Reviewed-on: https://chromium-review.googlesource.com/1127460
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573009}
[modify] https://crrev.com/c0998d0892e0e328b6bdb553f58f092b0c90fb8e/chrome/browser/policy/policy_browsertest.cc
Project Member

Comment 25 by bugdroid1@chromium.org, Jul 6 

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

commit 3e00a2d71325ed7374a5db08c491f6ed15e3968f
Author: Wez <wez@chromium.org>
Date: Fri Jul 06 19:24:25 2018

Fix some //components tests not to use QuitCurrent*Deprecated().

- Migrate SequencedModelWorkerTest, GaiaCookieManagerServiceTest and
  HistoryBackendDBBaseTest off QuitCurrent*Deprecated().
- Fix net::TestDelegate to maintain a |use_legacy_on_complete_| flag
  that causes use of RunLoop::QuitCurrentWhenIdleDeprecated(), rather
  than always creating and storing a QuitCurrent*ClosureDeprecated(),

These changes are required so that we can enable run-time checks against
mixing of QuitCurrent*() and Quit[Closure]() on the same RunLoop.

Bug:  844016 
Change-Id: I36b7d7a8fee9545269a854208c8736d60636cebd
Reviewed-on: https://chromium-review.googlesource.com/1127467
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Eric Roman <eroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573039}
[modify] https://crrev.com/3e00a2d71325ed7374a5db08c491f6ed15e3968f/components/history/core/test/history_backend_db_base_test.cc
[modify] https://crrev.com/3e00a2d71325ed7374a5db08c491f6ed15e3968f/components/signin/core/browser/gaia_cookie_manager_service_unittest.cc
[modify] https://crrev.com/3e00a2d71325ed7374a5db08c491f6ed15e3968f/components/sync/engine/sequenced_model_worker_unittest.cc
[modify] https://crrev.com/3e00a2d71325ed7374a5db08c491f6ed15e3968f/net/url_request/url_request_test_util.cc
[modify] https://crrev.com/3e00a2d71325ed7374a5db08c491f6ed15e3968f/net/url_request/url_request_test_util.h
Project Member

Comment 26 by bugdroid1@chromium.org, Jul 6 

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

commit 0cb8dbab0beeafe2b1ccba1c0e99e672abe795ab
Author: Wez <wez@chromium.org>
Date: Fri Jul 06 19:39:05 2018

Migrate DistillablePageUtils tests off QuitCurrent*Deprecated().

Bug:  844016 ,  859162 
Change-Id: If3e2f53808c5536d10f721b8c004e808769dbc86
Reviewed-on: https://chromium-review.googlesource.com/1125506
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573044}
[modify] https://crrev.com/0cb8dbab0beeafe2b1ccba1c0e99e672abe795ab/chrome/browser/dom_distiller/distillable_page_utils_browsertest.cc
Project Member

Comment 27 by bugdroid1@chromium.org, Jul 6 

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

commit b04806e8179889910c6141ca84ce97114b917bfc
Author: Wez <wez@chromium.org>
Date: Fri Jul 06 22:53:03 2018

Migrate PluginPowerSaverTest off of QuitCurrent*Deprecated().

Also make |visual_state_callback_| contain OnceClosures, and replace
use of insert(make_pair()) with emplace(), in the RenderFrameHostImpl.

Bug:  844016 
Change-Id: I3dffd88dc1ff3b5cd05071bee08bf11101c975a6
Reviewed-on: https://chromium-review.googlesource.com/1126563
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573105}
[modify] https://crrev.com/b04806e8179889910c6141ca84ce97114b917bfc/chrome/browser/plugins/plugin_power_saver_browsertest.cc
[modify] https://crrev.com/b04806e8179889910c6141ca84ce97114b917bfc/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/b04806e8179889910c6141ca84ce97114b917bfc/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/b04806e8179889910c6141ca84ce97114b917bfc/content/public/browser/render_frame_host.h
Project Member

Comment 28 by bugdroid1@chromium.org, Jul 7 

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

commit 53262b5285efbbfdbbcc17fbf5200890f1ee295c
Author: Wez <wez@chromium.org>
Date: Sat Jul 07 00:52:51 2018

Migrate ServiceProcessControl tests off of QuitCurrent*Deprecated().

Bug:  844016 
Change-Id: I9403b850456c8ee06cd2539f7cec9599302e81a0
Reviewed-on: https://chromium-review.googlesource.com/1126576
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573131}
[modify] https://crrev.com/53262b5285efbbfdbbcc17fbf5200890f1ee295c/chrome/browser/service_process/service_process_control_browsertest.cc
Project Member

Comment 29 by bugdroid1@chromium.org, Jul 9 

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

commit b9ef2f5a512ed2beba769761848c0cdd472a61e9
Author: Wez <wez@chromium.org>
Date: Mon Jul 09 21:20:55 2018

Migrate ServiceProcess and MockLaunchd to use RunLoop::QuitClosure().

Use of RunLoop::QuitCurrent*Deprecated() is deprecated, and risky,
potentially causing unexpected early-exits if run under a different
RunLoop to the one intended.

Bug:  844016 ,  859151 
Change-Id: Ib38310c0feeec09dbaac6cb74ffd061476e30cf3
Reviewed-on: https://chromium-review.googlesource.com/1123763
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573449}
[modify] https://crrev.com/b9ef2f5a512ed2beba769761848c0cdd472a61e9/chrome/browser/printing/cloud_print/test/cloud_print_proxy_process_browsertest.cc
[modify] https://crrev.com/b9ef2f5a512ed2beba769761848c0cdd472a61e9/chrome/common/mac/mock_launchd.cc
[modify] https://crrev.com/b9ef2f5a512ed2beba769761848c0cdd472a61e9/chrome/common/mac/mock_launchd.h
[modify] https://crrev.com/b9ef2f5a512ed2beba769761848c0cdd472a61e9/chrome/common/service_process_util_mac_unittest.mm
[modify] https://crrev.com/b9ef2f5a512ed2beba769761848c0cdd472a61e9/chrome/service/service_main.cc
[modify] https://crrev.com/b9ef2f5a512ed2beba769761848c0cdd472a61e9/chrome/service/service_process.cc
[modify] https://crrev.com/b9ef2f5a512ed2beba769761848c0cdd472a61e9/chrome/service/service_process.h
Project Member

Comment 30 by bugdroid1@chromium.org, Jul 10 

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

commit c368ca64ca66430968b15e07541a5c975deaab2e
Author: Wez <wez@chromium.org>
Date: Tue Jul 10 16:04:57 2018

Migrate DownloadContentTest off of QuitCurrent*Deprecated.

TBR: qinmin
Bug:  844016 
Change-Id: I118f20a980c31e164f7b0783d42c8daaa07a933e
Reviewed-on: https://chromium-review.googlesource.com/1128466
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573756}
[modify] https://crrev.com/c368ca64ca66430968b15e07541a5c975deaab2e/content/browser/download/download_browsertest.cc
Project Member

Comment 31 by bugdroid1@chromium.org, Jul 10 

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

commit 01954136cd0d67f3b5fd841031ae0a9a66593184
Author: Wez <wez@chromium.org>
Date: Tue Jul 10 18:34:15 2018

Migrate GestureProvider tests off of QuitCurrent*Deprecated.

Bug:  844016 
Change-Id: Iff5392ee536134bffa1522c4cb8bfc3ffbdb1864
Reviewed-on: https://chromium-review.googlesource.com/1129212
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573824}
[modify] https://crrev.com/01954136cd0d67f3b5fd841031ae0a9a66593184/ui/events/gesture_detection/gesture_provider_unittest.cc
Project Member

Comment 32 by bugdroid1@chromium.org, Jul 11 

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

commit e2ce0798b5673f52c442ef4ebecafdd120ee11be
Author: Wez <wez@chromium.org>
Date: Wed Jul 11 00:39:58 2018

Migrate Jingle tests off of QuitCurrent*Deprecated()

Bug:  844016 
Change-Id: Ia1d3238506ff95c565a2bcf309d94da7b527e490
Reviewed-on: https://chromium-review.googlesource.com/1132519
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573995}
[modify] https://crrev.com/e2ce0798b5673f52c442ef4ebecafdd120ee11be/jingle/glue/thread_wrapper_unittest.cc
Project Member

Comment 33 by bugdroid1@chromium.org, Jul 12 

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

commit fdb9848c74490ec3cfe0faddb95e3a26d08f51de
Author: Wez <wez@chromium.org>
Date: Thu Jul 12 01:21:39 2018

Migrate //ipc test off of QuitCurrent*Deprecated().

Bug:  844016 
Change-Id: If763ff6c6dc73de39e48710c3ecb7e1014a97d0f
Reviewed-on: https://chromium-review.googlesource.com/1132473
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574443}
[modify] https://crrev.com/fdb9848c74490ec3cfe0faddb95e3a26d08f51de/ipc/ipc_channel_mojo_unittest.cc
Project Member

Comment 35 by bugdroid1@chromium.org, Jul 17 

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

commit 325eafc631f396944801f17675a7092b216d60ca
Author: Wez <wez@chromium.org>
Date: Tue Jul 17 17:01:49 2018

Reland "Disallow QuitCurrent*Deprecated() on RunLoops with Quit*Closure()s."

This is a partial reland of b035cfda675fd58c44789cc3779f2b247f613cb6

Original change's description:
> Disallow QuitCurrent*Deprecated() on RunLoops with Quit*Closure()s.
>
> Use of RunLoop::QuitCurrent*Deprecated() is deprecated, and risky,
> potentially causing unexpected early-exits if run under a different
> RunLoop to the one intended.
>
> If a caller has taken a Quit*Closure() from the RunLoop then we now
> assume that they intend to use it to quit the loop, and therefore use
> of QuitCurrent*Deprecated() with it must be unintentional.
>
> Bug:  844016 
> Change-Id: Ib181d0cb34cdba2af29f557646cbe631101bc6b0
> Reviewed-on: https://chromium-review.googlesource.com/1082980
> Commit-Queue: Nico Weber <thakis@chromium.org>
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Reviewed-by: Gabriel Charette <gab@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#571499}

Bug:  844016 
Change-Id: I0666631fa736244c18f8848b317b41762881c748
Reviewed-on: https://chromium-review.googlesource.com/1121296
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575687}
[modify] https://crrev.com/325eafc631f396944801f17675a7092b216d60ca/base/message_loop/message_loop_unittest.cc
[modify] https://crrev.com/325eafc631f396944801f17675a7092b216d60ca/base/run_loop.cc
[modify] https://crrev.com/325eafc631f396944801f17675a7092b216d60ca/base/run_loop.h
Project Member

Comment 36 by bugdroid1@chromium.org, Jul 17 

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

commit b3f05beb4b0b87874d29896735afe76e51745aee
Author: Wez <wez@chromium.org>
Date: Tue Jul 17 18:10:56 2018

Migrate AudioInputController test off of QuitCurrent*Deprecated.

Bug:  844016 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I20115592c139201348980f835bc16154f656eadf
Reviewed-on: https://chromium-review.googlesource.com/1136658
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575719}
[modify] https://crrev.com/b3f05beb4b0b87874d29896735afe76e51745aee/media/audio/audio_input_controller_unittest.cc

Comment 37 by w...@chromium.org, Jul 22

Status: Fixed (was: Started)
DCHECKs in RunLoop::QuitCurrent*Deprecated() have landed, so closing this out, with further cleanup of production code tracked in  issue 859095 .
►

Sign in to add a comment