Add sequencing comment to PpdCacheImpl.

chromeos/printing/ppd_cache.cc

Index: chromeos/printing/ppd_cache.cc
diff --git a/chromeos/printing/ppd_cache.cc b/chromeos/printing/ppd_cache.cc
index 918b0e84cf9426a14b1565dab0dc7520354cf35a..25d08104a7f32fbc6e8cdceedd2918095902ca44 100644
--- a/chromeos/printing/ppd_cache.cc
+++ b/chromeos/printing/ppd_cache.cc
@@ -116,6 +116,11 @@ void StoreImpl(const base::FilePath& cache_dir,
   }
 }
 
+// Implementation of the PpdCache that uses two separate task runners for Store
+// and Fetch since the two operations have different priorities.  It is okay
+// that Store and Fetch are not synchronized because we assume that we won't
+// overwrite a file that we're trying to read simultaneously.  If this
+// assumption is invalidated, the sequencing will require revisiting.

[Carlson, 2017/06/29 18:47:09]

The more I think about it, the more I'm convinced that we've gone a bit wrong
here.

I think we don't have any real guarantee here from sequencing.  We could have
concurrent stores and find for the same file going on within a single PpdCache
-- we could

Find(foo) - miss
Store(foo) - (kicks off actual disk io to store_task_runner_)
Find(foo) - (disk io on fetch_task_runner_) hit on file, but file is not
completely written yet, so get incomplete contents.

Furthermore, we could have *multiple* ppd providers instantiated at the same
time, which means we don't actually get any guarantees from things being in the
same object -- two objects could be doing entirely different things on the same
filesystem.

I think probably the checksum mechanism is good enough to make this all ok, but
I don't think this comment is right.

[skau, 2017/06/30 20:55:49]

Okay.  It's updated.  Really, what we're saying is that the sequences don't
matter, we actually need to synchronize with the file system.  I say a little
more about expected behavior instead of assumptions now.

 class PpdCacheImpl : public PpdCache {
  public:
   explicit PpdCacheImpl(const base::FilePath& cache_base_dir)