Enable printers sync_integration_tests on chromeos

chrome/browser/sync/test/integration/printers_helper.cc

Index: chrome/browser/sync/test/integration/printers_helper.cc
diff --git a/chrome/browser/sync/test/integration/printers_helper.cc b/chrome/browser/sync/test/integration/printers_helper.cc
index 084e1d02ae2293e88a6eed2792136d7bf88200b0..82040ab22e901271f66c99df6de8314665acdc7e 100644
--- a/chrome/browser/sync/test/integration/printers_helper.cc
+++ b/chrome/browser/sync/test/integration/printers_helper.cc
@@ -13,10 +13,12 @@
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
+#include "base/threading/sequenced_worker_pool.h"
 #include "chrome/browser/chromeos/printing/printers_manager.h"
 #include "chrome/browser/chromeos/printing/printers_manager_factory.h"
 #include "chrome/browser/sync/test/integration/sync_datatype_helper.h"
 #include "chrome/browser/sync/test/integration/sync_test.h"
+#include "content/public/browser/browser_thread.h"
 
 using sync_datatype_helper::test;
 
@@ -65,6 +67,16 @@ std::string PrinterId(int index) {
   return base::StringPrintf("printer%d", index);
 }
 
+void WaitForTaskCompletion() {
+  // Since PrintersManagerFactory::BuildServiceInstanceFor() uses
+  // content::BrowserThread::GetBlockingPool() to schedule tasks, tests need to

[skym, 2017/04/06 21:03:03]

So what if the PrintersManagerFactory schedules tasks to
BrowserThread::GetBlockingPool()? Why does this integration test care about
waiting for them?

[Gang Wu, 2017/04/06 21:50:56]

Done.

+  // Wait for tasks running on content::BrowserThread::GetBlockingPool() to be

[skym, 2017/04/06 21:03:03]

Wait shouldn't be capitalized

[Gang Wu, 2017/04/06 21:50:56]

Done.

+  // completed and sent replay to UI thread.
+  content::BrowserThread::GetBlockingPool()->FlushForTesting();
+  // Wait for UI thread task completion.

[skym, 2017/04/06 21:03:03]

Can you elaborate what's going on here? What's being posted to the UI thread?
Why do we need to wait for it?

[Gang Wu, 2017/04/06 21:50:56]

Done.

+  base::RunLoop().RunUntilIdle();
+}
+
 }  // namespace
 
 void AddPrinter(chromeos::PrintersManager* manager,
@@ -110,7 +122,7 @@ chromeos::PrintersManager* GetVerifierPrinterStore() {
       chromeos::PrintersManagerFactory::GetForBrowserContext(

[skym, 2017/04/06 21:03:03]

It seems like there's more stuff that can be moved into the shared function.

[Gang Wu, 2017/04/06 21:50:56]

Done.

           sync_datatype_helper::test()->verifier());
   // Must wait for ModelTypeStore initialization.
-  base::RunLoop().RunUntilIdle();
+  WaitForTaskCompletion();

[skym, 2017/04/06 21:03:03]

Does this really need to be called on every GetVerifierPrinterStore()?

[Gang Wu, 2017/04/06 21:50:56]

no, just first one. but I think there is no harm to wait more times.

[skym, 2017/04/06 22:52:11]

I don't think I agree there is no harm done, but I've probably bikeshedded long
enough on this code review.

 
   return manager;
 }
@@ -120,7 +132,7 @@ chromeos::PrintersManager* GetPrinterStore(int index) {
       chromeos::PrintersManagerFactory::GetForBrowserContext(
           sync_datatype_helper::test()->GetProfile(index));
   // Must wait for ModelTypeStore initialization.
-  base::RunLoop().RunUntilIdle();
+  WaitForTaskCompletion();
 
   return manager;
 }