fix multithread related crashes in skpdiff

tools/skpdiff/SkImageDiffer.h

 /*
  * Copyright 2013 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #ifndef SkImageDiffer_DEFINED
 #define SkImageDiffer_DEFINED
 
 class SkBitmap;
 struct SkIPoint;
 
 /**
  * Encapsulates an image difference metric algorithm that can be potentially run asynchronously.
  */
 class SkImageDiffer {

[mtklein, 2013/11/06 14:24:36]

This doesn't seem like a particularly good interface.  Why not just expose the
underlying reality that we're doing this synchronously?  Then a lot of this
stuff becomes less bothersome threading-wise.  All the state would stay on the
stack; no mutexes needed at all.

const char* getName() const;
bool requiresOpenCL() const;
bool diffFile(const char[], const char[], DiffResult*) const;
bool diff(const SkBitmap&, const SkBitmap&, DiffResult*) const;

[djsollen, 2013/11/06 15:01:58]

Agreed. Is it Ok if I do this refactor in a second CL or do you prefer to have
it done now?

[mtklein, 2013/11/06 15:09:41]

Given how much of this CL becomes moot, I'd personally do it now.

 public:
     SkImageDiffer();
     virtual ~SkImageDiffer();
 
     /**
      * Gets a unique and descriptive name of this differ
      * @return A statically allocated null terminated string that is the name of this differ
      */
     virtual const char* getName() = 0;
 
@@ skipping 56 matching lines @@
      * Gets an array of the points of interest for the diff of the given id. The results are only
      * meaningful after the queued diff has finished.
      * @param  id The id of the queued diff to query
      */
     virtual SkIPoint* getPointsOfInterest(int id) = 0;
 
 
 
 protected:
     bool fIsGood;
+
+    // Used to ensure that only one thread is able to queue new entries at a time
+    SkMutex fQueueMutex;

[mtklein, 2013/11/06 14:24:36]

Pretty weird to have this on the base class.

 };
 
 
 #endif