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

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

Index: chrome/browser/media_galleries/fileapi/picasa/picasa_data_provider_browsertest.cc
diff --git a/chrome/browser/media_galleries/fileapi/picasa/picasa_data_provider_browsertest.cc b/chrome/browser/media_galleries/fileapi/picasa/picasa_data_provider_browsertest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..a1cf332c6f0504faf03acac3366cf2ca8795df97
--- /dev/null
+++ b/chrome/browser/media_galleries/fileapi/picasa/picasa_data_provider_browsertest.cc
@@ -0,0 +1,313 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/file_util.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "chrome/browser/media_galleries/fileapi/media_file_system_backend.h"
+#include "chrome/browser/media_galleries/fileapi/picasa/picasa_data_provider.h"
+#include "chrome/browser/media_galleries/fileapi/safe_picasa_albums_indexer.h"
+#include "chrome/common/media_galleries/picasa_types.h"
+#include "chrome/common/media_galleries/pmp_test_helper.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "content/public/test/test_browser_thread.h"
+
+using chrome::MediaFileSystemBackend;
+
+namespace picasa {
+
+class PicasaDataProviderTest : public InProcessBrowserTest {
+ public:
+  PicasaDataProviderTest() : test_helper_(kPicasaAlbumTableName) {}
+  virtual ~PicasaDataProviderTest() {}
+
+ protected:
+  virtual void SetUp() OVERRIDE { InProcessBrowserTest::SetUp(); }

[vandebo (ex-Chrome), 2013/08/14 23:48:11]

Omit since all you do is call the parent class's instance.

[tommycli, 2013/08/15 22:52:36]

Done.

+
+  // Runs on the MediaTaskRunner and designed to be overridden by subclasses.
+  virtual void InitializeTestData() { ASSERT_TRUE(test_helper_.Init()); }
+
+  void RunTest() {
+    DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+    base::RunLoop loop;
+    quit_closure_ = loop.QuitClosure();
+    MediaFileSystemBackend::MediaTaskRunner()->PostTask(
+        FROM_HERE,
+        base::Bind(&PicasaDataProviderTest::StartTestOnMediaTaskRunner,
+                   base::Unretained(this)));
+    loop.Run();
+  }
+
+  virtual PicasaDataProvider::DataType RequestedDataType() const = 0;
+
+  // Start the test. The data provider is refreshed before calling StartTest
+  // and the result of the refresh is passed in.
+  virtual void StartTest(bool parse_success) = 0;
+
+  void TestDone() {
+    DCHECK(MediaFileSystemBackend::CurrentlyOnMediaTaskRunnerThread());
+
+    // The data provider must be destructed on the MediaTaskRunner. This is done
+    // in a posted task rather than directly because TestDone is called by
+    // PicasaDataProvider. The callee should not destroy the caller.
+    MediaFileSystemBackend::MediaTaskRunner()->PostTask(
+        FROM_HERE,
+        base::Bind(&PicasaDataProviderTest::DestructDataProviderThenQuit,
+                   base::Unretained(this)));
+  }
+
+  PmpTestHelper* test_helper() { return &test_helper_; }
+
+  PicasaDataProvider* data_provider() const {
+    return picasa_data_provider_.get();
+  }
+
+ private:
+  void StartTestOnMediaTaskRunner() {
+    DCHECK(MediaFileSystemBackend::CurrentlyOnMediaTaskRunnerThread());
+
+    InitializeTestData();
+
+    picasa_data_provider_.reset(
+        new PicasaDataProvider(test_helper_.GetTempDirPath()));
+    data_provider()->RefreshData(
+        RequestedDataType(),
+        base::Bind(&PicasaDataProviderTest::StartTest, base::Unretained(this)));
+  }
+
+  void DestructDataProviderThenQuit() {
+    DCHECK(MediaFileSystemBackend::CurrentlyOnMediaTaskRunnerThread());
+    picasa_data_provider_.reset();
+    content::BrowserThread::PostTask(
+        content::BrowserThread::UI, FROM_HERE, quit_closure_);
+  }
+
+  PmpTestHelper test_helper_;
+  scoped_ptr<PicasaDataProvider> picasa_data_provider_;
+
+  base::Closure quit_closure_;
+
+  DISALLOW_COPY_AND_ASSIGN(PicasaDataProviderTest);
+};
+
+class PicasaDataProviderNoDatabaseGetListTest : public PicasaDataProviderTest {
+ public:
+  virtual PicasaDataProvider::DataType RequestedDataType() const OVERRIDE {
+    return PicasaDataProvider::LIST_OF_ALBUMS_AND_FOLDERS_DATA;
+  }
+  virtual void StartTest(bool parse_success) OVERRIDE {
+    EXPECT_FALSE(parse_success);
+    TestDone();
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(PicasaDataProviderNoDatabaseGetListTest,
+                       NoDatabaseGetList) {
+  RunTest();
+}
+
+class PicasaDataProviderNoDatabaseGetAlbumsImagesTest
+    : public PicasaDataProviderTest {
+ protected:
+  virtual PicasaDataProvider::DataType RequestedDataType() const OVERRIDE {
+    return PicasaDataProvider::ALBUMS_IMAGES_DATA;
+  }
+  virtual void StartTest(bool parse_success) OVERRIDE {
+    EXPECT_FALSE(parse_success);
+    TestDone();
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(PicasaDataProviderNoDatabaseGetAlbumsImagesTest,
+                       NoDatabaseGetAlbumsImages) {
+  RunTest();
+}
+
+class PicasaDataProviderGetListTest : public PicasaDataProviderTest {
+ protected:
+  virtual void InitializeTestData() OVERRIDE {
+    PicasaDataProviderTest::InitializeTestData();
+
+    ASSERT_TRUE(test_folder_1_.CreateUniqueTempDir());
+    ASSERT_TRUE(test_folder_2_.CreateUniqueTempDir());
+
+    std::vector<uint32> category_vector;
+    category_vector.push_back(kAlbumCategoryFolder);
+    category_vector.push_back(kAlbumCategoryInvalid);
+    category_vector.push_back(kAlbumCategoryAlbum);
+    category_vector.push_back(kAlbumCategoryFolder);
+    category_vector.push_back(kAlbumCategoryAlbum);
+
+    std::vector<double> date_vector;
+    date_vector.push_back(0.0);
+    date_vector.push_back(0.0);
+    date_vector.push_back(0.0);
+    date_vector.push_back(0.0);
+    date_vector.push_back(0.0);
+
+    std::vector<std::string> filename_vector;
+    filename_vector.push_back(test_folder_1_.path().AsUTF8Unsafe());
+    filename_vector.push_back("");
+    filename_vector.push_back("");
+    filename_vector.push_back(test_folder_2_.path().AsUTF8Unsafe());
+    filename_vector.push_back("");
+
+    std::vector<std::string> name_vector;
+    name_vector.push_back(test_folder_1_.path().BaseName().AsUTF8Unsafe());
+    name_vector.push_back("");
+    name_vector.push_back("Album 1 Name");
+    name_vector.push_back(test_folder_2_.path().BaseName().AsUTF8Unsafe());
+    name_vector.push_back("Album 2 Name");
+
+    std::vector<std::string> token_vector;
+    token_vector.push_back("");
+    token_vector.push_back("");
+    token_vector.push_back(std::string(kAlbumTokenPrefix) + "uid3");
+    token_vector.push_back("");
+    token_vector.push_back(std::string(kAlbumTokenPrefix) + "uid5");
+
+    std::vector<std::string> uid_vector;
+    uid_vector.push_back("uid1");
+    uid_vector.push_back("uid2");
+    uid_vector.push_back("uid3");
+    uid_vector.push_back("uid4");
+    uid_vector.push_back("uid5");
+
+    ASSERT_TRUE(test_helper()->WriteColumnFileFromVector(
+        "category", PMP_TYPE_UINT32, category_vector));
+    ASSERT_TRUE(test_helper()->WriteColumnFileFromVector(
+        "date", PMP_TYPE_DOUBLE64, date_vector));
+    ASSERT_TRUE(test_helper()->WriteColumnFileFromVector(
+        "filename", PMP_TYPE_STRING, filename_vector));
+    ASSERT_TRUE(test_helper()->WriteColumnFileFromVector(
+        "name", PMP_TYPE_STRING, name_vector));
+    ASSERT_TRUE(test_helper()->WriteColumnFileFromVector(
+        "token", PMP_TYPE_STRING, token_vector));
+    ASSERT_TRUE(test_helper()->WriteColumnFileFromVector(
+        "uid", PMP_TYPE_STRING, uid_vector));
+  }
+
+  virtual PicasaDataProvider::DataType RequestedDataType() const OVERRIDE {
+    return PicasaDataProvider::LIST_OF_ALBUMS_AND_FOLDERS_DATA;
+  }
+
+  virtual void StartTest(bool parse_success) OVERRIDE {
+    ASSERT_TRUE(parse_success);
+
+    scoped_ptr<AlbumMap> folders = data_provider()->GetFolders();
+    ASSERT_TRUE(folders.get());
+    EXPECT_EQ(2u, folders->size());
+
+    AlbumMap::const_iterator folder_1 = folders->find(
+        test_folder_1_.path().BaseName().AsUTF8Unsafe() + " 1899-12-30");
+    EXPECT_NE(folders->end(), folder_1);
+    EXPECT_EQ(test_folder_1_.path().BaseName().AsUTF8Unsafe(),
+              folder_1->second.name);
+    EXPECT_EQ(test_folder_1_.path(), folder_1->second.path);
+    EXPECT_EQ("uid1", folder_1->second.uid);
+
+    AlbumMap::const_iterator folder_2 = folders->find(
+        test_folder_2_.path().BaseName().AsUTF8Unsafe() + " 1899-12-30");
+    EXPECT_NE(folders->end(), folder_2);
+    EXPECT_EQ(test_folder_2_.path().BaseName().AsUTF8Unsafe(),
+              folder_2->second.name);
+    EXPECT_EQ(test_folder_2_.path(), folder_2->second.path);
+    EXPECT_EQ("uid4", folder_2->second.uid);
+
+    scoped_ptr<AlbumMap> albums = data_provider()->GetAlbums();
+    ASSERT_TRUE(albums.get());
+    EXPECT_EQ(2u, albums->size());
+
+    AlbumMap::const_iterator album_1 = albums->find("Album 1 Name 1899-12-30");
+    EXPECT_NE(albums->end(), album_1);
+    EXPECT_EQ("Album 1 Name", album_1->second.name);
+    EXPECT_EQ(base::FilePath(), album_1->second.path);
+    EXPECT_EQ("uid3", album_1->second.uid);
+
+    AlbumMap::const_iterator album_2 = albums->find("Album 2 Name 1899-12-30");
+    EXPECT_NE(albums->end(), album_2);
+    EXPECT_EQ("Album 2 Name", album_2->second.name);
+    EXPECT_EQ(base::FilePath(), album_2->second.path);
+    EXPECT_EQ("uid5", album_2->second.uid);
+
+    TestDone();
+  }
+
+  base::ScopedTempDir test_folder_1_;
+  base::ScopedTempDir test_folder_2_;
+};
+
+IN_PROC_BROWSER_TEST_F(PicasaDataProviderGetListTest, GetListTest) {
+  RunTest();
+}
+
+class PicasaDataProviderGetAlbumsImagesTest
+    : public PicasaDataProviderGetListTest {
+ protected:
+  virtual void InitializeTestData() OVERRIDE {
+    PicasaDataProviderGetListTest::InitializeTestData();
+
+    const char folder_1_test_ini[] =
+        "[InBoth.jpg]\n"
+        "albums=uid3,uid5\n"
+        "[InSecondAlbumOnly.jpg]\n"
+        "albums=uid5\n";
+    ASSERT_TRUE(file_util::WriteFile(
+        test_folder_1_.path().AppendASCII(kPicasaINIFilename),
+        folder_1_test_ini,
+        arraysize(folder_1_test_ini)));
+
+    const char folder_2_test_ini[] =
+        "[InFirstAlbumOnly.jpg]\n"
+        "albums=uid3\n";
+    ASSERT_TRUE(file_util::WriteFile(
+        test_folder_2_.path().AppendASCII(kPicasaINIFilename),
+        folder_2_test_ini,
+        arraysize(folder_2_test_ini)));
+  }
+
+  virtual PicasaDataProvider::DataType RequestedDataType() const OVERRIDE {
+    return PicasaDataProvider::ALBUMS_IMAGES_DATA;
+  }
+
+  virtual void StartTest(bool parse_success) OVERRIDE {
+    ASSERT_TRUE(parse_success);
+
+    scoped_ptr<AlbumImagesMap> album_images =
+        data_provider()->GetAlbumsImages();
+    ASSERT_TRUE(album_images.get());
+
+    AlbumImagesMap::const_iterator album_1_images = album_images->find("uid3");
+    ASSERT_NE(album_images->end(), album_1_images);
+    EXPECT_EQ(2u, album_1_images->second.size());
+    EXPECT_EQ(1u,
+              album_1_images->second.count(
+                  test_folder_1_.path().AppendASCII("InBoth.jpg")));
+    EXPECT_EQ(1u,
+              album_1_images->second.count(
+                  test_folder_2_.path().AppendASCII("InFirstAlbumOnly.jpg")));
+
+    AlbumImagesMap::const_iterator album_2_images = album_images->find("uid5");
+    ASSERT_NE(album_images->end(), album_2_images);
+    EXPECT_EQ(2u, album_2_images->second.size());
+    EXPECT_EQ(1u,
+              album_2_images->second.count(
+                  test_folder_1_.path().AppendASCII("InBoth.jpg")));
+    EXPECT_EQ(1u,
+              album_2_images->second.count(
+                  test_folder_1_.path().AppendASCII("InSecondAlbumOnly.jpg")));
+
+    TestDone();
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(PicasaDataProviderGetAlbumsImagesTest,
+                       GetAlbumsImagesTest) {
+  RunTest();
+}
+

[vandebo (ex-Chrome), 2013/08/14 23:48:11]

There are a lot more tests that can be added.... I think it would be worthwhile
to add at least tests that did the following:

Multiple pending requests.
An invalidation.
An invalidation while the reader/indexer is outstanding.

[tommycli, 2013/08/15 22:52:36]

Done.

+}  // namespace picasa