Don't call Timer::Stop() off-sequence in SyncBrowserThreadModelWorkerTest

components/sync/engine/browser_thread_model_worker_unittest.cc

 // Copyright (c) 2012 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 "components/sync/engine/browser_thread_model_worker.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/location.h"
 #include "base/memory/weak_ptr.h"
@@ skipping 32 matching lines @@
     WorkCallback c = base::Bind(&SyncBrowserThreadModelWorkerTest::DoWork,
                                 base::Unretained(this));
     timer()->Start(FROM_HERE, TestTimeouts::action_timeout(), this,
                    &SyncBrowserThreadModelWorkerTest::Timeout);
     worker()->DoWorkAndWaitUntilDone(c);
   }
 
   // This is the work that will be scheduled to be done on the DB thread.
   SyncerError DoWork() {
     EXPECT_TRUE(db_thread_.task_runner()->BelongsToCurrentThread());
-    timer_.Stop();  // Stop the failure timer so the test succeeds.

[stanisc, 2017/01/25 00:55:16]

Would it make sense to remove timer_ completely and just rely on test timeout?

[gab, 2017/01/25 14:51:47]

Yes it does but since there are many erroneous cases around I was hoping to just
do the minimal change to get things correct and have owners improve further on
their own timeline should they care

     main_message_loop_.task_runner()->PostTask(
         FROM_HERE, base::MessageLoop::QuitWhenIdleClosure());
     did_do_work_ = true;
     return SYNCER_OK;
   }
 
   // This will be called by the OneShotTimer and make the test fail unless
   // DoWork is called first.
   void Timeout() {
     ADD_FAILURE() << "Timed out waiting for work to be done on the DB thread.";
@@ skipping 26 matching lines @@
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::Bind(&SyncBrowserThreadModelWorkerTest::ScheduleWork,
                             factory()->GetWeakPtr()));
   base::RunLoop().Run();
   EXPECT_TRUE(did_do_work());
 }
 
 }  // namespace
 
 }  // namespace syncer