Followup Feedback on https://codereview.chromium.org/2726073002/

base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/task_scheduler/scheduler_single_thread_task_runner_manager.h"
 
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
 #include "base/synchronization/lock.h"
 #include "base/synchronization/waitable_event.h"
@@ skipping 262 matching lines @@
   void Run() override {
     run_started_event_.Signal();
     manager_to_join_->JoinForTesting();
   }
 
   void WaitForRunToStart() { run_started_event_.Wait(); }
 
  private:
   SchedulerSingleThreadTaskRunnerManager* const manager_to_join_;
   WaitableEvent run_started_event_;
+
   DISALLOW_COPY_AND_ASSIGN(CallJoinFromDifferentThread);
 };
 
 class TaskSchedulerSingleThreadTaskRunnerManagerJoinTest
     : public TaskSchedulerSingleThreadTaskRunnerManagerTest {
  public:
   TaskSchedulerSingleThreadTaskRunnerManagerJoinTest() = default;
   ~TaskSchedulerSingleThreadTaskRunnerManagerJoinTest() override = default;
 
  protected:
   void TearDownSingleThreadTaskRunnerManager() override {
     // The tests themselves are responsible for calling JoinForTesting().
     single_thread_task_runner_manager_.reset();
   }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(TaskSchedulerSingleThreadTaskRunnerManagerJoinTest);
 };
 
 }  // namespace
 
 TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerJoinTest, ConcurrentJoin) {
+  // Exercises the codepath where the workers are unavailable for unregistration
+  // because of a Join call.
   WaitableEvent task_running(WaitableEvent::ResetPolicy::MANUAL,
                              WaitableEvent::InitialState::NOT_SIGNALED);
   WaitableEvent task_blocking(WaitableEvent::ResetPolicy::MANUAL,
                               WaitableEvent::InitialState::NOT_SIGNALED);
 
   {
     auto task_runner = single_thread_task_runner_manager_
                            ->CreateSingleThreadTaskRunnerWithTraits(
                                TaskTraits().WithBaseSyncPrimitives());
     EXPECT_TRUE(task_runner->PostTask(
         FROM_HERE, Bind(&WaitableEvent::Signal, Unretained(&task_running))));
     EXPECT_TRUE(task_runner->PostTask(
         FROM_HERE, Bind(&WaitableEvent::Wait, Unretained(&task_blocking))));
   }
 
   task_running.Wait();
   CallJoinFromDifferentThread join_from_different_thread(
       single_thread_task_runner_manager_.get());
   join_from_different_thread.Start();
   join_from_different_thread.WaitForRunToStart();
   task_blocking.Signal();
   join_from_different_thread.Join();
 }
 
 TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerJoinTest,
        ConcurrentJoinExtraSkippedTask) {
+  // Tests to make sure that tasks are properly cleaned up at Join, allowing
+  // SingleThreadTaskRunners to unregister themselves.
   WaitableEvent task_running(WaitableEvent::ResetPolicy::MANUAL,
                              WaitableEvent::InitialState::NOT_SIGNALED);
   WaitableEvent task_blocking(WaitableEvent::ResetPolicy::MANUAL,
                               WaitableEvent::InitialState::NOT_SIGNALED);
 
   {
     auto task_runner = single_thread_task_runner_manager_
                            ->CreateSingleThreadTaskRunnerWithTraits(
                                TaskTraits().WithBaseSyncPrimitives());
     EXPECT_TRUE(task_runner->PostTask(
         FROM_HERE, Bind(&WaitableEvent::Signal, Unretained(&task_running))));
     EXPECT_TRUE(task_runner->PostTask(
         FROM_HERE, Bind(&WaitableEvent::Wait, Unretained(&task_blocking))));
     EXPECT_TRUE(task_runner->PostTask(FROM_HERE, Bind(&DoNothing)));

[gab, 2017/03/15 20:45:12]

Then wouldn't you want to bind something here that makes sure it's deleted (i.e.
that task was freed)?

[robliao, 2017/03/15 20:49:30]

If the task isn't deleted, the SingleThreadTaskRunner remains alive and we'll
assert later on. Adding machinery to replicate this loop seemed unnecessary.

   }
 
   task_running.Wait();
   CallJoinFromDifferentThread join_from_different_thread(
       single_thread_task_runner_manager_.get());
   join_from_different_thread.Start();
   join_from_different_thread.WaitForRunToStart();
   task_blocking.Signal();
   join_from_different_thread.Join();
 }
 
 }  // namespace internal
 }  // namespace base