Media Galleries API Picasa: Make PicasaDataProvider handle async PMP and INI parsing robustly.

chrome/browser/media_galleries/fileapi/picasa/picasa_file_util_unittest.cc

Index: chrome/browser/media_galleries/fileapi/picasa/picasa_file_util_unittest.cc
diff --git a/chrome/browser/media_galleries/fileapi/picasa/picasa_file_util_unittest.cc b/chrome/browser/media_galleries/fileapi/picasa/picasa_file_util_unittest.cc
index cd19e3bb25eccc2463315701ad6a285bddf05126..bf5d21d29739a5365dbde69f43cc721dee567d20 100644
--- a/chrome/browser/media_galleries/fileapi/picasa/picasa_file_util_unittest.cc
+++ b/chrome/browser/media_galleries/fileapi/picasa/picasa_file_util_unittest.cc
@@ -8,12 +8,14 @@
 
 #include "base/bind_helpers.h"
 #include "base/file_util.h"
+#include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/memory/scoped_vector.h"
 #include "base/message_loop/message_loop.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
+#include "base/synchronization/waitable_event.h"
 #include "base/time/time.h"
 #include "chrome/browser/media_galleries/fileapi/media_file_system_backend.h"
 #include "chrome/browser/media_galleries/fileapi/media_path_filter.h"
@@ -151,31 +153,6 @@ void ReadDirectoryTestHelper(fileapi::FileSystemOperationRunner* runner,
 
 }  // namespace
 
-class TestPicasaDataProvider : public PicasaDataProvider {
- public:
-  TestPicasaDataProvider()
-      : PicasaDataProvider(base::FilePath(FILE_PATH_LITERAL("Fake"))),
-        initialized_(false) {
-  }
-
-  virtual ~TestPicasaDataProvider() {}
-
-  virtual void RefreshData(const base::Closure& ready_callback) OVERRIDE {
-    DCHECK(initialized_);
-    ready_callback.Run();
-  }
-
-  void Init(const std::vector<AlbumInfo>& albums,
-            const std::vector<AlbumInfo>& folders) {
-    UniquifyNames(albums, &album_map_);
-    UniquifyNames(folders, &folder_map_);
-    initialized_ = true;
-  }
-
- private:
-  bool initialized_;
-};
-
 class TestPicasaFileUtil : public PicasaFileUtil {
  public:
   TestPicasaFileUtil(chrome::MediaPathFilter* media_path_filter,
@@ -227,8 +204,15 @@ class PicasaFileUtilTest : public testing::Test {
     scoped_refptr<quota::SpecialStoragePolicy> storage_policy =
         new quota::MockSpecialStoragePolicy();
 
+    base::WaitableEvent event(true, false);

[vandebo (ex-Chrome), 2013/08/22 17:48:02]

Abstract this into void RunOnMediaTaskRunner(const base::Closure& closure);

[tommycli, 2013/08/22 22:32:51]

Done.

+    chrome::MediaFileSystemBackend::MediaTaskRunner()->PostTask(
+        FROM_HERE,
+        base::Bind(&PicasaFileUtilTest::SetUpOnMediaTaskRunner,
+                   base::Unretained(this),
+                   &event));
+    event.Wait();
+
     media_path_filter_.reset(new chrome::MediaPathFilter());
-    picasa_data_provider_.reset(new TestPicasaDataProvider());
 
     ScopedVector<fileapi::FileSystemBackend> additional_providers;
     additional_providers.push_back(new TestMediaFileSystemBackend(
@@ -247,7 +231,38 @@ class PicasaFileUtilTest : public testing::Test {
         fileapi::CreateAllowFileAccessOptions());
   }
 
+  virtual void TearDown() OVERRIDE {
+    base::WaitableEvent event(true, false);

[vandebo (ex-Chrome), 2013/08/22 17:48:02]

Why not use a runloop like in the browser test?

[tommycli, 2013/08/22 22:32:51]

I believe this was because Runloops can only be used when the Quit is called on
the same thread. WaitableEvents are threadsafe.

[vandebo (ex-Chrome), 2013/08/23 16:42:41]

If calling on the wrong thread is the only issue, that's solvable....
Post the closure
Post a task, which posts a task back to the run loop and calls the quit closure.

Maybe I'm biased against using a waitable event, but it is really blocking -
nothing can happen.  At least with a RunLoop, the message loop is getting run
when it has work to do.

[tommycli, 2013/08/26 22:06:17]

Done.

+    chrome::MediaFileSystemBackend::MediaTaskRunner()->PostTask(
+        FROM_HERE,
+        base::Bind(&PicasaFileUtilTest::TearDownOnMediaTaskRunner,
+                   base::Unretained(this),
+                   &event));
+    event.Wait();
+  }
+
  protected:
+  void SetUpOnMediaTaskRunner(base::WaitableEvent* event) {
+    picasa_data_provider_.reset(new PicasaDataProvider(base::FilePath()));
+    event->Signal();
+  }
+
+  void TearDownOnMediaTaskRunner(base::WaitableEvent* event) {
+    picasa_data_provider_.reset();
+    event->Signal();
+  }
+
+  void SetupDataProvider(PicasaDataProvider* picasa_data_provider,
+                         const std::vector<AlbumInfo>& albums,
+                         const std::vector<AlbumInfo>& folders) {
+    PicasaDataProvider::UniquifyNames(albums,
+                                      &picasa_data_provider->album_map_);
+    PicasaDataProvider::UniquifyNames(folders,
+                                      &picasa_data_provider->folder_map_);
+    picasa_data_provider->state_ =
+        PicasaDataProvider::ALBUMS_IMAGES_FRESH_STATE;
+  }
+
   // |test_folders| must be in alphabetical order for easy verification
   void SetupFolders(ScopedVector<TestFolder>* test_folders) {
     std::vector<AlbumInfo> folders;
@@ -257,7 +272,9 @@ class PicasaFileUtilTest : public testing::Test {
       ASSERT_TRUE(test_folder->Init());
       folders.push_back(test_folder->folder_info());
     }
-    picasa_data_provider_->Init(std::vector<AlbumInfo>(), folders);
+
+    SetupDataProvider(
+        picasa_data_provider_.get(), std::vector<AlbumInfo>(), folders);
   }
 
   void VerifyFolderDirectoryList(const ScopedVector<TestFolder>& test_folders) {
@@ -337,8 +354,8 @@ class PicasaFileUtilTest : public testing::Test {
   base::ScopedTempDir profile_dir_;
 
   scoped_refptr<fileapi::FileSystemContext> file_system_context_;
+  scoped_ptr<PicasaDataProvider> picasa_data_provider_;
   scoped_ptr<chrome::MediaPathFilter> media_path_filter_;
-  scoped_ptr<TestPicasaDataProvider> picasa_data_provider_;
 
   DISALLOW_COPY_AND_ASSIGN(PicasaFileUtilTest);
 };