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

This is a partial reland of b035cfda

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/1121296Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575687}

base/message_loop/message_loop_unittest.cc

@@ -1629,6 +1629,8 @@ TEST_P(MessageLoopTypedTest, RunLoopQuitOrderAfter) {
   ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
                                           BindOnce(&FuncThatQuitsNow));
 
+  run_loop.allow_quit_current_deprecated_ = true;
+
   RunLoop outer_run_loop;
   outer_run_loop.Run();
 

base/run_loop.cc

@@ -152,6 +152,7 @@ void RunLoop::QuitWhenIdle() {
 base::Closure RunLoop::QuitClosure() {
   // TODO(gab): Fix bad usage and enable this check, http://crbug.com/715235.
   // DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  allow_quit_current_deprecated_ = false;
 
   // Need to use ProxyToTaskRunner() as WeakPtrs vended from
   // |weak_factory_| may only be accessed on |origin_task_runner_|.
@@ -163,6 +164,7 @@ base::Closure RunLoop::QuitClosure() {
 base::Closure RunLoop::QuitWhenIdleClosure() {
   // TODO(gab): Fix bad usage and enable this check, http://crbug.com/715235.
   // DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  allow_quit_current_deprecated_ = false;
 
   // Need to use ProxyToTaskRunner() as WeakPtrs vended from
   // |weak_factory_| may only be accessed on |origin_task_runner_|.
@@ -201,17 +203,29 @@ void RunLoop::RemoveNestingObserverOnCurrentThread(NestingObserver* observer) {
 // static
 void RunLoop::QuitCurrentDeprecated() {
   DCHECK(IsRunningOnCurrentThread());
-  tls_delegate.Get().Get()->active_run_loops_.top()->Quit();
+  Delegate* delegate = tls_delegate.Get().Get();
+  DCHECK(delegate->active_run_loops_.top()->allow_quit_current_deprecated_)
+      << "Please migrate off QuitCurrentDeprecated(), e.g. to QuitClosure().";
+  delegate->active_run_loops_.top()->Quit();
 }
 
 // static
 void RunLoop::QuitCurrentWhenIdleDeprecated() {
   DCHECK(IsRunningOnCurrentThread());
-  tls_delegate.Get().Get()->active_run_loops_.top()->QuitWhenIdle();
+  Delegate* delegate = tls_delegate.Get().Get();
+  DCHECK(delegate->active_run_loops_.top()->allow_quit_current_deprecated_)
+      << "Please migrate off QuitCurrentWhenIdleDeprecated(), e.g. to "
+         "QuitWhenIdleClosure().";
+  delegate->active_run_loops_.top()->QuitWhenIdle();
 }
 
 // static
 Closure RunLoop::QuitCurrentWhenIdleClosureDeprecated() {
+  // TODO(844016): Fix callsites and enable this check, or remove the API.
+  // Delegate* delegate = tls_delegate.Get().Get();
+  // DCHECK(delegate->active_run_loops_.top()->allow_quit_current_deprecated_)
+  //     << "Please migrate off QuitCurrentWhenIdleClosureDeprecated(), e.g to "
+  //        "QuitWhenIdleClosure().";
   return Bind(&RunLoop::QuitCurrentWhenIdleDeprecated);
 }
 

base/run_loop.h

@@ -248,6 +248,8 @@ class BASE_EXPORT RunLoop {
   };
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(MessageLoopTypedTest, RunLoopQuitOrderAfter);
+
 #if defined(OS_ANDROID)
   // Android doesn't support the blocking RunLoop::Run, so it calls
   // BeforeRun and AfterRun directly.
@@ -283,6 +285,11 @@ class BASE_EXPORT RunLoop {
   // rather than pushed to Delegate to support nested RunLoops.
   bool quit_when_idle_received_ = false;
 
+  // True if use of QuitCurrent*Deprecated() is allowed. Taking a Quit*Closure()
+  // from a RunLoop implicitly sets this to false, so QuitCurrent*Deprecated()
+  // cannot be used while that RunLoop is being Run().
+  bool allow_quit_current_deprecated_ = true;
+
   // RunLoop is not thread-safe. Its state/methods, unless marked as such, may
   // not be accessed from any other sequence than the thread it was constructed
   // on. Exception: RunLoop can be safely accessed from one other sequence (or