Add a test for ITunesDataProvider (try two).

chrome/browser/media_galleries/fileapi/itunes_data_provider_browsertest.cc

+// 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 <string>
+#include <vector>
+
+#include "base/bind.h"
+#include "base/file_util.h"
+#include "base/files/file_path.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/message_loop.h"
+#include "base/run_loop.h"
+#include "base/strings/stringprintf.h"
+#include "chrome/browser/media_galleries/fileapi/itunes_data_provider.h"
+#include "chrome/browser/media_galleries/fileapi/media_file_system_backend.h"
+#include "chrome/browser/media_galleries/imported_media_gallery_registry.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "content/public/browser/browser_thread.h"
+#include "url/gurl.h"
+
+using chrome::MediaFileSystemBackend;
+
+namespace itunes {
+
+namespace {
+
+struct LibraryEntry {
+  LibraryEntry(const std::string& artist, const std::string& album,
+               const base::FilePath& location)
+      : artist(artist),
+        album(album),
+        location(location) {
+  }
+  std::string artist;
+  std::string album;
+  base::FilePath location;
+};
+
+}  // namespace
+
+class TestITunesDataProvider : public ITunesDataProvider {
+ public:
+  TestITunesDataProvider(const base::FilePath& xml_library_path,
+                         const base::Closure& callback)
+      : ITunesDataProvider(xml_library_path),
+        callback_(callback) {
+  }
+  virtual ~TestITunesDataProvider() {}
+
+ private:
+  virtual void OnLibraryChanged(const base::FilePath& path,
+                                bool error) OVERRIDE {
+    ITunesDataProvider::OnLibraryChanged(path, error);
+    callback_.Run();
+  }
+
+  base::Closure callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestITunesDataProvider);
+};
+
+class ITunesDataProviderTest : public InProcessBrowserTest {
+ public:

[Lei Zhang, 2013/07/17 02:21:35]

None of this actually needs to be public.

[vandebo (ex-Chrome), 2013/07/17 04:22:18]

Done.

+  ITunesDataProviderTest() {}
+  virtual ~ITunesDataProviderTest() {}
+
+  virtual void SetUp() OVERRIDE {
+    ASSERT_TRUE(library_dir_.CreateUniqueTempDir());
+    WriteLibraryInternal(SetUpLibrary());
+    // The ImportedMediaGalleryRegistry is created on which ever thread calls
+    // GetInstance() first.  It shouldn't matter what thread creates, however
+    // in practice it is always created on the UI thread, so this calls
+    // GetInstance here to mirror those real conditions.
+    chrome::ImportedMediaGalleryRegistry::GetInstance();
+    InProcessBrowserTest::SetUp();
+  }
+
+  void RunTestOnMediaTaskRunner() {

[Lei Zhang, 2013/07/17 02:21:35]

How about just RunTest(). RunTestOnMediaTaskRunner makes it sound like it's
suppose to be running on the MTR.

[vandebo (ex-Chrome), 2013/07/17 04:22:18]

Done.

+    DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+    base::RunLoop loop;
+    quit_closure_ = loop.QuitClosure();
+    MediaFileSystemBackend::MediaTaskRunner()->PostTask(
+        FROM_HERE,
+        base::Bind(&ITunesDataProviderTest::StartTestOnMediaTaskRunner,
+                   base::Unretained(this)));
+    loop.Run();
+  }
+
+  void WriteLibrary(const std::vector<LibraryEntry>& entries,
+                    const base::Closure& callback) {
+    SetLibraryChangeCallback(callback);
+    WriteLibraryInternal(entries);
+  }
+
+  void SetLibraryChangeCallback(const base::Closure& callback) {
+    EXPECT_TRUE(library_changed_callback_.is_null());
+    library_changed_callback_ = callback;
+  }
+
+  ITunesDataProvider* data_provider() const {
+    return chrome::ImportedMediaGalleryRegistry::ITunesDataProvider();
+  }
+
+  base::FilePath library_dir() const {

[Lei Zhang, 2013/07/17 02:21:35]

This can return a const base::FilePath&

[vandebo (ex-Chrome), 2013/07/17 04:22:18]

Done.

+    return library_dir_.path();
+  }
+
+  base::FilePath XmlFile() const {
+    return library_dir_.path().AppendASCII("library.xml");
+  }
+
+ protected:
+  // Get the initial set of library entries, called by SetUp.  If no entries
+  // are returned the xml file is not created.
+  virtual std::vector<LibraryEntry> SetUpLibrary() {
+    return std::vector<LibraryEntry>();
+  }
+
+  // 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() {

[Lei Zhang, 2013/07/17 02:21:35]

make sure you are on the MTR.

[vandebo (ex-Chrome), 2013/07/17 04:22:18]

Done.

+    chrome::ImportedMediaGalleryRegistry* imported_registry =
+        chrome::ImportedMediaGalleryRegistry::GetInstance();
+    imported_registry->itunes_data_provider_.reset();
+    content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
+                                     quit_closure_);
+  }
+
+ private:
+  void StartTestOnMediaTaskRunner() {

[Lei Zhang, 2013/07/17 02:21:35]

Check you are on the MTR.

[vandebo (ex-Chrome), 2013/07/17 04:22:18]

Done.

+    chrome::ImportedMediaGalleryRegistry* imported_registry =
+        chrome::ImportedMediaGalleryRegistry::GetInstance();
+    imported_registry->itunes_data_provider_.reset(
+        new TestITunesDataProvider(
+            XmlFile(),
+            base::Bind(&ITunesDataProviderTest::OnLibraryChanged,
+                       base::Unretained(this))));
+    data_provider()->RefreshData(base::Bind(&ITunesDataProviderTest::StartTest,
+                                            base::Unretained(this)));
+  };
+
+  void OnLibraryChanged(void) {

[Lei Zhang, 2013/07/17 02:21:35]

nit: We usually don't write (void). Same on line 290.

[vandebo (ex-Chrome), 2013/07/17 04:22:18]

Done.

+    DCHECK(
+        MediaFileSystemBackend::CurrentlyOnMediaTaskRunnerThread());

[Lei Zhang, 2013/07/17 02:21:35]

nit: fits on previous line.

[vandebo (ex-Chrome), 2013/07/17 04:22:18]

Done.

+    if (!library_changed_callback_.is_null()) {
+      library_changed_callback_.Run();
+      library_changed_callback_.Reset();
+    }
+  }
+
+  void WriteLibraryInternal(const std::vector<LibraryEntry>& entries) {
+    if (!entries.size())
+      return;
+    std::string xml = "<plist><dict><key>Tracks</key><dict>\n";
+    for (size_t i = 0; i < entries.size(); ++i) {
+      GURL location("file://localhost/" + entries[i].location.AsUTF8Unsafe());
+      // Visual studio doesn't like %zd, so cast to int instead.

[Lei Zhang, 2013/07/17 02:21:35]

Look for PRIuS in base/format_macros.h.

[vandebo (ex-Chrome), 2013/07/17 04:22:18]

Done.

+      int id = static_cast<int>(i) + 1;
+      std::string entry_string = base::StringPrintf(
+          "<key>%d</key><dict>\n"
+          "  <key>Track ID</key><integer>%d</integer>\n"
+          "  <key>Location</key><string>%s</string>\n"
+          "  <key>Artist</key><string>%s</string>\n"
+          "  <key>Album</key><string>%s</string>\n"
+          "</dict>\n",
+          id, id, location.spec().c_str(), entries[i].artist.c_str(),
+          entries[i].album.c_str());
+      xml += entry_string;
+    }
+    xml += "</dict></dict></plist>\n";
+    file_util::WriteFile(XmlFile(), xml.c_str(), xml.size());

[Lei Zhang, 2013/07/17 02:21:35]

Make sure we wrote the right # of bytes. Ditto for line 347.

[vandebo (ex-Chrome), 2013/07/17 04:22:18]

Done.

+  }
+
+  base::ScopedTempDir library_dir_;
+
+  base::Closure library_changed_callback_;
+
+  base::Closure quit_closure_;
+
+  DISALLOW_COPY_AND_ASSIGN(ITunesDataProviderTest);
+};
+
+class ITunesDataProviderBasicTest : public ITunesDataProviderTest {
+ public:
+  ITunesDataProviderBasicTest() {}
+  virtual ~ITunesDataProviderBasicTest() {}
+
+  virtual std::vector<LibraryEntry> SetUpLibrary() OVERRIDE {
+    base::FilePath track = library_dir().AppendASCII("Track.mp3");
+    std::vector<LibraryEntry> entries;
+    entries.push_back(LibraryEntry("Artist", "Album", track));
+    return entries;
+  }
+
+  virtual void StartTest(bool parse_success) OVERRIDE {
+    EXPECT_TRUE(parse_success);
+
+    // KnownArtist
+    EXPECT_TRUE(data_provider()->KnownArtist("Artist"));
+    EXPECT_FALSE(data_provider()->KnownArtist("Artist2"));
+
+    // KnownAlbum
+    EXPECT_TRUE(data_provider()->KnownAlbum("Artist", "Album"));
+    EXPECT_FALSE(data_provider()->KnownAlbum("Artist", "Album2"));
+    EXPECT_FALSE(data_provider()->KnownAlbum("Artist2", "Album"));
+
+    // GetTrackLocation
+    base::FilePath track =
+        library_dir().AppendASCII("Track.mp3").NormalizePathSeparators();
+    EXPECT_EQ(track.value(),
+              data_provider()->GetTrackLocation(
+                  "Artist", "Album",
+                  "Track.mp3").NormalizePathSeparators().value());
+    EXPECT_TRUE(data_provider()->GetTrackLocation("Artist", "Album",
+                                                  "Track2.mp3").empty());
+    EXPECT_TRUE(data_provider()->GetTrackLocation("Artist", "Album2",
+                                                  "Track.mp3").empty());
+    EXPECT_TRUE(data_provider()->GetTrackLocation("Artist2", "Album",
+                                                  "Track.mp3").empty());
+
+    // GetArtistNames
+    std::set<ITunesDataProvider::ArtistName> artists =
+      data_provider()->GetArtistNames();
+    EXPECT_EQ(1U, artists.size());

[Lei Zhang, 2013/07/17 02:21:35]

This should be an ASSERT_EQ(), otherwise the next line might have a NULL deref.
Ditto below.

[vandebo (ex-Chrome), 2013/07/17 04:22:18]

Done.

+    EXPECT_EQ(std::string("Artist"), *artists.begin());

[Lei Zhang, 2013/07/17 02:21:35]

do you need the explicit std::string()? Ditto below.

[vandebo (ex-Chrome), 2013/07/17 04:22:18]

Done.

+
+    // GetAlbumNames
+    std::set<ITunesDataProvider::AlbumName> albums =
+        data_provider()->GetAlbumNames("Artist");
+    EXPECT_EQ(1U, albums.size());
+    EXPECT_EQ(std::string("Album"), *albums.begin());
+
+    albums = data_provider()->GetAlbumNames("Artist2");
+    EXPECT_EQ(0U, albums.size());
+
+    // GetAlbum
+    ITunesDataProvider::Album album =
+        data_provider()->GetAlbum("Artist", "Album");
+    EXPECT_EQ(1U, album.size());
+    EXPECT_EQ(track.BaseName().AsUTF8Unsafe(), album.begin()->first);
+    EXPECT_EQ(track.value(),
+              album.begin()->second.NormalizePathSeparators().value());
+
+    album = data_provider()->GetAlbum("Artist", "Album2");
+    EXPECT_EQ(0U, album.size());
+
+    album = data_provider()->GetAlbum("Artist2", "Album");
+    EXPECT_EQ(0U, album.size());
+
+    TestDone();
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ITunesDataProviderBasicTest);
+};
+
+class ITunesDataProviderRefreshTest : public ITunesDataProviderTest {
+ public:
+  ITunesDataProviderRefreshTest() {}
+  virtual ~ITunesDataProviderRefreshTest() {}
+
+  virtual std::vector<LibraryEntry> SetUpLibrary() OVERRIDE {
+    base::FilePath track = library_dir().AppendASCII("Track.mp3");
+    std::vector<LibraryEntry> entries;
+    entries.push_back(LibraryEntry("Artist", "Album", track));
+    return entries;
+  }
+
+  virtual void StartTest(bool parse_success) OVERRIDE {
+    EXPECT_TRUE(parse_success);
+
+    // Initial contents.
+    ExpectTrackLocation("Artist", "Album", "Track.mp3");
+    ExpectNoTrack("Artist2", "Album2", "Track2.mp3");
+
+    // New file.
+    base::FilePath track2 = library_dir().AppendASCII("Track2.mp3");
+    std::vector<LibraryEntry> entries;
+    entries.push_back(LibraryEntry("Artist2", "Album2", track2));
+    WriteLibrary(entries,
+                 base::Bind(&ITunesDataProviderRefreshTest::CheckAfterWrite,
+                            base::Unretained(this)));
+  }
+
+  void CheckAfterWrite(void) {
+    // Content the same.
+    ExpectTrackLocation("Artist", "Album", "Track.mp3");
+    ExpectNoTrack("Artist2", "Album2", "Track2.mp3");
+
+    data_provider()->RefreshData(
+        base::Bind(&ITunesDataProviderRefreshTest::CheckRefresh,
+                   base::Unretained(this)));
+  }
+
+  void CheckRefresh(bool is_valid) {
+    EXPECT_TRUE(is_valid);
+
+    ExpectTrackLocation("Artist2", "Album2", "Track2.mp3");
+    ExpectNoTrack("Artist", "Album", "Track.mp3");
+    TestDone();
+  }
+
+ private:
+  void ExpectTrackLocation(const std::string& artist, const std::string& album,

[Lei Zhang, 2013/07/17 02:21:35]

If you put this and ExpectNoTrack() in ITunesDataProviderTest, you can use it in
other tests as well.

[vandebo (ex-Chrome), 2013/07/17 04:22:18]

Done.

+                           const std::string& track_name) {
+    base::FilePath track =
+        library_dir().AppendASCII(track_name).NormalizePathSeparators();
+    EXPECT_EQ(track.value(),
+              data_provider()->GetTrackLocation(
+                  artist, album, track_name).NormalizePathSeparators().value());
+  }
+
+  void ExpectNoTrack(const std::string& artist, const std::string& album,
+                     const std::string& track_name) {
+    EXPECT_TRUE(data_provider()->GetTrackLocation(
+          artist, album, track_name).empty()) << track_name;
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(ITunesDataProviderRefreshTest);
+};
+
+class ITunesDataProviderInvalidTest : public ITunesDataProviderTest {
+ public:
+  ITunesDataProviderInvalidTest() {}
+  virtual ~ITunesDataProviderInvalidTest() {}
+
+  virtual std::vector<LibraryEntry> SetUpLibrary() OVERRIDE {
+    base::FilePath track = library_dir().AppendASCII("Track.mp3");
+    std::vector<LibraryEntry> entries;
+    entries.push_back(LibraryEntry("Artist", "Album", track));
+    return entries;
+  }
+
+  virtual void StartTest(bool parse_success) OVERRIDE {
+    EXPECT_TRUE(parse_success);
+
+    SetLibraryChangeCallback(
+        base::Bind(&ITunesDataProvider::RefreshData,
+                   base::Unretained(data_provider()),
+                   base::Bind(&ITunesDataProviderInvalidTest::CheckInvalid,
+                              base::Unretained(this))));
+    file_util::WriteFile(XmlFile(), " ", 1);
+  }
+
+  void CheckInvalid(bool is_valid) {
+    EXPECT_FALSE(is_valid);
+    TestDone();
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ITunesDataProviderInvalidTest);
+};
+
+class ITunesDataProviderUniqueNameTest : public ITunesDataProviderTest {
+ public:
+  ITunesDataProviderUniqueNameTest() {}
+  virtual ~ITunesDataProviderUniqueNameTest() {}
+
+  virtual std::vector<LibraryEntry> SetUpLibrary() OVERRIDE {
+    base::FilePath track = library_dir().AppendASCII("Track.mp3");
+    std::vector<LibraryEntry> entries;
+    // Dupe album names should get uniquified with the track id, which in the
+    // test framework is the vector index.
+    entries.push_back(LibraryEntry("Artist", "Album", track));
+    entries.push_back(LibraryEntry("Artist", "Album", track));
+    entries.push_back(LibraryEntry("Artist", "Album2", track));
+    return entries;
+  }
+
+  virtual void StartTest(bool parse_success) OVERRIDE {
+    EXPECT_TRUE(parse_success);
+
+    base::FilePath track =
+        library_dir().AppendASCII("Track.mp3").NormalizePathSeparators();
+    EXPECT_EQ(track.value(),
+              data_provider()->GetTrackLocation(
+                  "Artist", "Album",
+                  "Track (1).mp3").NormalizePathSeparators().value());
+    EXPECT_EQ(track.value(),
+              data_provider()->GetTrackLocation(
+                  "Artist", "Album",
+                  "Track (2).mp3").NormalizePathSeparators().value());
+    EXPECT_EQ(track.value(),
+              data_provider()->GetTrackLocation(
+                  "Artist", "Album2",
+                  "Track.mp3").NormalizePathSeparators().value());
+
+    TestDone();
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ITunesDataProviderUniqueNameTest);
+};
+
+IN_PROC_BROWSER_TEST_F(ITunesDataProviderBasicTest, BasicTest) {
+  RunTestOnMediaTaskRunner();
+}
+
+IN_PROC_BROWSER_TEST_F(ITunesDataProviderRefreshTest, RefreshTest) {
+  RunTestOnMediaTaskRunner();
+}
+
+IN_PROC_BROWSER_TEST_F(ITunesDataProviderInvalidTest, InvalidTest) {
+  RunTestOnMediaTaskRunner();
+}
+
+IN_PROC_BROWSER_TEST_F(ITunesDataProviderUniqueNameTest, UniqueNameTest) {
+  RunTestOnMediaTaskRunner();
+}
+
+}  // namespace itunes