iTunes file util and data provider for media galleries

chrome/browser/media_galleries/imported_media_gallery_registry.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 "chrome/browser/media_galleries/imported_media_gallery_registry.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/threading/sequenced_worker_pool.h"
+#include "chrome/browser/media_galleries/fileapi/itunes_data_provider.h"
 #include "chrome/browser/media_galleries/fileapi/picasa/picasa_data_provider.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "content/public/browser/browser_thread.h"
 #include "webkit/browser/fileapi/file_system_task_runners.h"
 #include "webkit/browser/fileapi/isolated_context.h"
 
 using base::Bind;
 
 namespace chrome {
 
@@ skipping 19 matching lines @@
 static base::LazyInstance<ImportedMediaGalleryRegistry>::Leaky
 g_imported_media_gallery_registry = LAZY_INSTANCE_INITIALIZER;
 
 }
 
 // static
 ImportedMediaGalleryRegistry* ImportedMediaGalleryRegistry::GetInstance() {
   return g_imported_media_gallery_registry.Pointer();
 }
 
-// static
 std::string ImportedMediaGalleryRegistry::RegisterPicasaFilesystemOnUIThread(
     const base::FilePath& database_path) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
   std::string fsid =
       fileapi::IsolatedContext::GetInstance()->RegisterFileSystemForVirtualPath(
            fileapi::kFileSystemTypePicasa,
            extension_misc::kMediaFileSystemPathPart,
            base::FilePath());
 
   if (fsid.empty())
     return "";
 
-  MediaTaskRunner()->PostTask(
-      FROM_HERE,
-      Bind(&ImportedMediaGalleryRegistry::RegisterPicasaFileSystem,
-           base::Unretained(GetInstance()), database_path));
+  picasa_fsids_.insert(fsid);
+
+  if (picasa_fsids_.size() == 1) {
+    MediaTaskRunner()->PostTask(
+        FROM_HERE,
+        Bind(&ImportedMediaGalleryRegistry::RegisterPicasaFileSystem,
+             base::Unretained(GetInstance()), database_path));

[Lei Zhang, 2013/06/06 03:48:47]

Since this is no longer static, use this instead of GetInstance() ?

[vandebo (ex-Chrome), 2013/06/06 19:49:18]

Done.

+#ifndef NDEBUG
+    picasa_database_path_ = database_path;
+  } else {
+    DCHECK_EQ(picasa_database_path_, database_path);

[Lei Zhang, 2013/06/06 03:48:47]

If these were FilePaths, you need to compare their .value()s instead. Sigh.

[vandebo (ex-Chrome), 2013/06/06 19:49:18]

Done.

+#endif
+  }
 
   return fsid;
 }
 
-// static
-bool ImportedMediaGalleryRegistry::RevokePicasaFilesystemOnUIThread(
+std::string ImportedMediaGalleryRegistry::RegisterITunesFilesystemOnUIThread(
+    const base::FilePath& library_xml_path) {

[Lei Zhang, 2013/06/06 03:48:47]

DCHECK the path is not empty?

[vandebo (ex-Chrome), 2013/06/06 19:49:18]

Done.

+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+  std::string fsid =
+      fileapi::IsolatedContext::GetInstance()->RegisterFileSystemForVirtualPath(
+           fileapi::kFileSystemTypeItunes,
+           extension_misc::kMediaFileSystemPathPart,
+           base::FilePath());
+
+  if (fsid.empty())
+    return "";

[Lei Zhang, 2013/06/06 03:48:47]

FWIW, this can't actually be reached. If we reach this line, we'll just fail a
CHECK() in the current caller.

We should return std::string() BTW.

[vandebo (ex-Chrome), 2013/06/06 19:49:18]

It can fail in IsolatedContext, so I fixed the callers.
Done.

[Lei Zhang, 2013/06/07 03:29:07]

With the current implementation and the parameters we pass in, it can't fail.

I think there are multiple layers where we all have DCHECKS that the fsid is
non-empty, now that you removed the one CHECK.

[vandebo (ex-Chrome), 2013/06/07 03:46:52]

Are you saying that we should add a CHECK in IsolatedContext and not have the
failure case?  That seems fine to me.

+
+  itunes_fsids_.insert(fsid);
+
+  if (itunes_fsids_.size() == 1) {
+    MediaTaskRunner()->PostTask(
+        FROM_HERE,
+        Bind(&ImportedMediaGalleryRegistry::RegisterITunesFileSystem,
+             base::Unretained(GetInstance()), library_xml_path));
+#ifndef NDEBUG
+    itunes_xml_library_path_ = library_xml_path;
+  } else {
+    DCHECK_EQ(itumes_xml_library_path_, library_xml_path);

[Lei Zhang, 2013/06/06 03:48:47]

typo: itumes

[vandebo (ex-Chrome), 2013/06/06 19:49:18]

Done.

+#endif
+  }
+
+  return fsid;
+}
+
+bool ImportedMediaGalleryRegistry::RevokeImportedFilesystemOnUIThread(
     const std::string& fsid) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
   if (!fileapi::IsolatedContext::GetInstance()->RevokeFileSystem(fsid))
     return false;
 
-  MediaTaskRunner()->PostTask(
-      FROM_HERE,
-      Bind(&ImportedMediaGalleryRegistry::RevokePicasaFileSystem,
-           base::Unretained(GetInstance())));
+  if (picasa_fsids_.erase(fsid) && !picasa_fsids_.size()) {

[Lei Zhang, 2013/06/06 03:48:47]

!foo_set.size() -> foo_set.empty()

[Lei Zhang, 2013/06/06 03:48:47]

If |picasa_fsids_| has 2 elements, and you erase 1, you shouldn't need to go on
and check |itunes_fsids_| below.

[vandebo (ex-Chrome), 2013/06/06 19:49:18]

Done.

[vandebo (ex-Chrome), 2013/06/06 19:49:18]

Done.

+    MediaTaskRunner()->PostTask(
+        FROM_HERE,
+        Bind(&ImportedMediaGalleryRegistry::RevokePicasaFileSystem,
+             base::Unretained(GetInstance())));
+    return true;
+  }
 
-  return true;
+  if (itunes_fsids_.erase(fsid) && !itunes_fsids_.size()) {
+    MediaTaskRunner()->PostTask(
+        FROM_HERE,
+        Bind(&ImportedMediaGalleryRegistry::RevokeITunesFileSystem,
+             base::Unretained(GetInstance())));
+    return true;
+  }
+
+  NOTREACHED();
+  return false;
 }
 
 // static
 picasa::PicasaDataProvider*
 ImportedMediaGalleryRegistry::picasa_data_provider() {
   DCHECK(CurrentlyOnMediaTaskRunnerThread());
   DCHECK(GetInstance()->picasa_data_provider_);
   return GetInstance()->picasa_data_provider_.get();
 }
 
-ImportedMediaGalleryRegistry::ImportedMediaGalleryRegistry()
-    : picasa_filesystems_count_(0) {
+// static
+itunes::ITunesDataProvider*
+ImportedMediaGalleryRegistry::itunes_data_provider() {

[Lei Zhang, 2013/06/06 03:48:47]

nit: These are not simple getters, and should not be named as such.

[vandebo (ex-Chrome), 2013/06/06 19:49:18]

Done.

+  DCHECK(CurrentlyOnMediaTaskRunnerThread());
+  DCHECK(GetInstance()->itunes_data_provider_);
+  return GetInstance()->itunes_data_provider_.get();
 }
 
+ImportedMediaGalleryRegistry::ImportedMediaGalleryRegistry() {}
+
 ImportedMediaGalleryRegistry::~ImportedMediaGalleryRegistry() {
-  DCHECK_EQ(0, picasa_filesystems_count_);
+  DCHECK_EQ(0U, picasa_fsids_.size());
+  DCHECK_EQ(0U, itunes_fsids_.size());
 }
 
 void ImportedMediaGalleryRegistry::RegisterPicasaFileSystem(
     const base::FilePath& database_path) {
   DCHECK(CurrentlyOnMediaTaskRunnerThread());
-
-  if (++picasa_filesystems_count_ == 1) {
-    DCHECK(!picasa_data_provider_);
-    picasa_data_provider_.reset(new picasa::PicasaDataProvider(database_path));
-  }
+  DCHECK(!picasa_data_provider_);
+  picasa_data_provider_.reset(new picasa::PicasaDataProvider(database_path));
 }
 
 void ImportedMediaGalleryRegistry::RevokePicasaFileSystem() {
   DCHECK(CurrentlyOnMediaTaskRunnerThread());
+  DCHECK(picasa_data_provider_);
+  picasa_data_provider_.reset();
+}
 
-  if (--picasa_filesystems_count_ == 0) {
-    DCHECK(picasa_data_provider_);
-    picasa_data_provider_.reset(NULL);
-  }
+void ImportedMediaGalleryRegistry::RegisterITunesFileSystem(
+    const base::FilePath& xml_library_path) {
+  DCHECK(CurrentlyOnMediaTaskRunnerThread());
+  DCHECK(!itunes_data_provider_);
+  itunes_data_provider_.reset(new itunes::ITunesDataProvider(xml_library_path));
+}
+
+void ImportedMediaGalleryRegistry::RevokeITunesFileSystem() {
+  DCHECK(CurrentlyOnMediaTaskRunnerThread());
+  DCHECK(itunes_data_provider_);
+  itunes_data_provider_.reset();
 }
 
 }  // namespace chrome