SkThreadPool: tweak two little things that have been annoying me

 1) it's pretty annoying that SkThreadPool doesn't include SkRunnable for us;
 2) add wait() so we don't have to keep using SkAutoTDelete/free() to wait for completion.

BUG=
R=scroggo@google.com, reed@google.com

Author: mtklein@google.com

Review URL: https://codereview.chromium.org/26470005

git-svn-id: http://skia.googlecode.com/svn/trunk@11711 2bbb7eff-a529-9590-31e7-b0007b416f81

4 files changed, 20 insertions, 10 deletions

diff --git a/include/utils/SkThreadPool.h b/include/utils/SkThreadPool.h
index 98657034e5..6cb8b528d3 100644
--- a/include/utils/SkThreadPool.h
+++ b/include/utils/SkThreadPool.h
@@ -9,10 +9,10 @@
 #define SkThreadPool_DEFINED
 
 #include "SkCondVar.h"
+#include "SkRunnable.h"
 #include "SkTDArray.h"
 #include "SkTInternalLList.h"
 
-class SkRunnable;
 class SkThread;
 
 class SkThreadPool {
@@ -31,6 +31,11 @@ public:
      */
     void add(SkRunnable*);
 
+    /**
+     * Block until all added SkRunnables have completed.  Once called, calling add() is undefined.
+     */
+    void wait();
+
  private:
     struct LinkedRunnable {
         // Unowned pointer.
diff --git a/src/utils/SkThreadPool.cpp b/src/utils/SkThreadPool.cpp
index 3d19d1c5b9..e078af3ba3 100644
--- a/src/utils/SkThreadPool.cpp
+++ b/src/utils/SkThreadPool.cpp
@@ -39,6 +39,12 @@ SkThreadPool::SkThreadPool(int count)
 }
 
 SkThreadPool::~SkThreadPool() {
+    if (!fDone) {
+        this->wait();
+    }
+}
+
+void SkThreadPool::wait() {
     fReady.lock();
     fDone = true;
     fReady.broadcast();
@@ -99,6 +105,7 @@ void SkThreadPool::add(SkRunnable* r) {
 
     // We have some threads.  Queue it up!
     fReady.lock();
+    SkASSERT(!fDone);  // We shouldn't be adding work to a pool that's shut down.
     LinkedRunnable* linkedRunnable = SkNEW(LinkedRunnable);
     linkedRunnable->fRunnable = r;
     fQueue.addToHead(linkedRunnable);
diff --git a/tests/OnceTest.cpp b/tests/OnceTest.cpp
index 0b2aa9a857..74c88a40e1 100644
--- a/tests/OnceTest.cpp
+++ b/tests/OnceTest.cpp
@@ -6,7 +6,6 @@
  */
 
 #include "SkOnce.h"
-#include "SkRunnable.h"
 #include "SkThreadPool.h"
 #include "Test.h"
 #include "TestClassDef.h"
@@ -56,11 +55,11 @@ DEF_TEST(SkOnce_Multithreaded, r) {
     }
 
     // Let them race.
-    SkAutoTDelete<SkThreadPool> pool(new SkThreadPool(kThreads));
+    SkThreadPool pool(kThreads);
     for (int i = 0; i < kTasks; i++) {
-        pool->add(&racers[i]);
+        pool.add(&racers[i]);
     }
-    pool.free();  // Blocks until all threads are done.
+    pool.wait();
 
     // Only one should have done the +=.
     REPORTER_ASSERT(r, 6 == x);
diff --git a/tests/skia_test.cpp b/tests/skia_test.cpp
index 620cdac9ca..b8f80e0c4b 100644
--- a/tests/skia_test.cpp
+++ b/tests/skia_test.cpp
@@ -8,7 +8,6 @@
 #include "SkCommandLineFlags.h"
 #include "SkGraphics.h"
 #include "SkOSFile.h"
-#include "SkRunnable.h"
 #include "SkTArray.h"
 #include "SkTemplates.h"
 #include "SkThreadPool.h"
@@ -229,7 +228,7 @@ int tool_main(int argc, char** argv) {
     int32_t failCount = 0;
     int skipCount = 0;
 
-    SkAutoTDelete<SkThreadPool> threadpool(SkNEW_ARGS(SkThreadPool, (FLAGS_threads)));
+    SkThreadPool threadpool(FLAGS_threads);
     SkTArray<Test*> unsafeTests;  // Always passes ownership to an SkTestRunnable
     for (int i = 0; i < total; i++) {
         SkAutoTDelete<Test> test(iter.next());
@@ -238,7 +237,7 @@ int tool_main(int argc, char** argv) {
         } else if (!test->isThreadsafe()) {
             unsafeTests.push_back() = test.detach();
         } else {
-            threadpool->add(SkNEW_ARGS(SkTestRunnable, (test.detach(), &failCount)));
+            threadpool.add(SkNEW_ARGS(SkTestRunnable, (test.detach(), &failCount)));
         }
     }
 
@@ -247,8 +246,8 @@ int tool_main(int argc, char** argv) {
         SkNEW_ARGS(SkTestRunnable, (unsafeTests[i], &failCount))->run();
     }
 
-    // Blocks until threaded tests finish.
-    threadpool.free();
+    // Block until threaded tests finish.
+    threadpool.wait();
 
     SkDebugf("Finished %d tests, %d failures, %d skipped.\n",
              toRun, failCount, skipCount);