Media Galleries: Add more tests for media gallery names.

chrome/browser/media_gallery/media_file_system_registry_unittest.cc

Index: chrome/browser/media_gallery/media_file_system_registry_unittest.cc
===================================================================
--- chrome/browser/media_gallery/media_file_system_registry_unittest.cc	(revision 173002)
+++ chrome/browser/media_gallery/media_file_system_registry_unittest.cc	(working copy)
@@ -5,10 +5,12 @@
 // MediaFileSystemRegistry unit tests.
 
 #include <algorithm>
+#include <set>
 
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/files/scoped_temp_dir.h"
+#include "base/json/json_reader.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
@@ -151,6 +153,52 @@
 
 namespace {
 
+void GetGalleryNamesCB(std::set<std::string>* results,
+                       const std::vector<MediaFileSystemInfo>& file_systems) {
+  for (size_t i = 0; i < file_systems.size(); ++i) {
+    std::set<std::string>::const_iterator it =
+        results->find(file_systems[i].name);
+    ASSERT_EQ(it, results->end());
+    results->insert(file_systems[i].name);
+  }
+}
+
+

[kmadhusu, 2012/12/14 03:29:39]

nit: remove a blank line.

[Lei Zhang, 2012/12/14 03:45:31]

Done.

+void CheckGalleryJSONName(const std::string& name,
+                          bool removable,
+                          bool user_added) {
+  scoped_ptr<DictionaryValue> dict(static_cast<DictionaryValue*>(
+      base::JSONReader::Read(name)));
+  ASSERT_TRUE(dict);
+
+  // Check deviceId.
+  EXPECT_EQ(removable,
+            dict->HasKey(MediaFileSystemRegistry::kDeviceIdKey)) << name;
+  if (removable) {
+    std::string device_id;
+    EXPECT_TRUE(dict->GetString(MediaFileSystemRegistry::kDeviceIdKey,
+                               &device_id)) << name;

[kmadhusu, 2012/12/14 03:29:39]

style nit: Add one more space before &device_id

[Lei Zhang, 2012/12/14 03:45:31]

Done.

+    EXPECT_FALSE(device_id.empty()) << name;
+  }
+
+  // Check galleryId.
+  EXPECT_TRUE(dict->HasKey(MediaFileSystemRegistry::kGalleryIdKey)) << name;

[kmadhusu, 2012/12/14 03:29:39]

Can we also make sure !gallery_id.empty()?

[Lei Zhang, 2012/12/14 03:45:31]

galleryId is a number.

+
+  // Check name.
+  EXPECT_TRUE(dict->HasKey(MediaFileSystemRegistry::kNameKey)) << name;
+  std::string gallery_name;
+  EXPECT_TRUE(dict->GetString(MediaFileSystemRegistry::kNameKey,
+                             &gallery_name)) << name;

[kmadhusu, 2012/12/14 03:29:39]

style nit: Add one more space before &gallery_name

[Lei Zhang, 2012/12/14 03:45:31]

Done.

+  EXPECT_FALSE(gallery_name.empty()) << name;
+
+  // Check userAdded.
+  EXPECT_TRUE(dict->HasKey(MediaFileSystemRegistry::kUserAddedKey)) << name;
+  bool actual_user_added = !user_added;
+  EXPECT_TRUE(dict->GetBoolean(MediaFileSystemRegistry::kUserAddedKey,
+                              &actual_user_added)) << name;

[kmadhusu, 2012/12/14 03:29:39]

style nit: Add one more space before &gallery_name

[Lei Zhang, 2012/12/14 03:45:31]

Done.

+  EXPECT_EQ(user_added, actual_user_added) << name;
+}
+
 class TestMediaStorageUtil : public MediaStorageUtil {
  public:
   static void SetTestingMode();
@@ -196,6 +244,8 @@
       const std::vector<MediaFileSystemInfo>& regular_extension_galleries,
       const std::vector<MediaFileSystemInfo>& all_extension_galleries);
 
+  std::set<std::string> GetGalleryNames(extensions::Extension* extension);
+
   extensions::Extension* all_permission_extension();
   extensions::Extension* regular_permission_extension();
 
@@ -263,6 +313,13 @@
 
   void AssertAllAutoAddedGalleries();
 
+  void InitForGalleryNamesTest(std::set<std::string>* gallery_names);
+
+  void CheckNewGallery(ProfileState* profile_state,
+                       const std::set<std::string>& gallery_names,

[kmadhusu, 2012/12/14 03:29:39]

nit: std::set<std::string> is used at several places. For better readability,
can we add typedef?

[Lei Zhang, 2012/12/14 03:45:31]

I don't see what's so hard to understand about a set of strings.

+                       bool removable,
+                       bool user_added);
+
   std::vector<MediaFileSystemInfo> GetAutoAddedGalleries(
       ProfileState* profile_state);
 
@@ -453,6 +510,19 @@
   EXPECT_EQ(1, GetAndClearComparisonCount());
 }
 
+std::set<std::string> ProfileState::GetGalleryNames(
+    extensions::Extension* extension) {
+  content::RenderViewHost* rvh = single_web_contents_->GetRenderViewHost();
+  std::set<std::string> results;
+  MediaFileSystemRegistry* registry =
+      g_browser_process->media_file_system_registry();
+  registry->GetMediaFileSystemsForExtension(
+      rvh, extension,
+      base::Bind(&GetGalleryNamesCB, base::Unretained(&results)));

[kmadhusu, 2012/12/14 03:29:39]

Just curious about the "GetGalleryNamesCB", Are we allowed to use abbreviations
in the function name (Callback => CB)?

[Lei Zhang, 2012/12/14 03:45:31]

Renamed.

+  MessageLoop::current()->RunUntilIdle();
+  return results;
+}
+
 extensions::Extension* ProfileState::all_permission_extension() {
   return all_permission_extension_.get();
 }
@@ -472,7 +542,7 @@
 
   num_comparisons_++;
   ASSERT_EQ(expected.size(), actual.size()) << test;
-  for (size_t i = 0; i < expected.size() && i < actual.size(); i++) {
+  for (size_t i = 0; i < expected.size() && i < actual.size(); ++i) {
     EXPECT_EQ(expected[i].path.value(), actual[i].path.value()) << test;
     EXPECT_FALSE(actual[i].fsid.empty()) << test;
     if (!expected[i].fsid.empty())
@@ -496,7 +566,7 @@
 }
 
 void MediaFileSystemRegistryTest::CreateProfileState(size_t profile_count) {
-  for (size_t i = 0; i < profile_count; i++) {
+  for (size_t i = 0; i < profile_count; ++i) {
     ProfileState* state = new ProfileState(&rph_factory_);
     profile_states_.push_back(state);
   }
@@ -514,7 +584,7 @@
   string16 name = path.LossyDisplayName();
   DCHECK(!MediaStorageUtil::IsMediaDevice(device_id));
 
-  for (size_t i = 0; i < profile_states_.size(); i++) {
+  for (size_t i = 0; i < profile_states_.size(); ++i) {
     profile_states_[i]->GetMediaGalleriesPrefs()->AddGallery(
         device_id, name, FilePath(), true /*user_added*/);
   }
@@ -530,6 +600,11 @@
   string16 name = location.LossyDisplayName();
   base::SystemMonitor::Get()->ProcessRemovableStorageAttached(device_id, name,
                                                               location.value());
+  bool user_added = (type == MediaStorageUtil::REMOVABLE_MASS_STORAGE_NO_DCIM);
+  for (size_t i = 0; i < profile_states_.size(); ++i) {
+    profile_states_[i]->GetMediaGalleriesPrefs()->AddGallery(
+        device_id, name, FilePath(), user_added);
+  }
   MessageLoop::current()->RunUntilIdle();
   return device_id;
 }
@@ -547,13 +622,13 @@
       profile_state->GetMediaGalleriesPrefs();
   MediaGalleryPrefIdSet pref_id =
       preferences->LookUpGalleriesByDeviceId(device_id);
-  DCHECK_EQ(1U, pref_id.size());
+  ASSERT_EQ(1U, pref_id.size());
   preferences->SetGalleryPermissionForExtension(*extension, *pref_id.begin(),
                                                 has_access);
 }
 
 void MediaFileSystemRegistryTest::AssertAllAutoAddedGalleries() {
-  for (size_t i = 0; i < profile_states_.size(); i++) {
+  for (size_t i = 0; i < profile_states_.size(); ++i) {
     MediaGalleriesPreferences* prefs =
         profile_states_[0]->GetMediaGalleriesPrefs();
 
@@ -571,6 +646,38 @@
   }
 }
 
+void MediaFileSystemRegistryTest::InitForGalleryNamesTest(
+    std::set<std::string>* gallery_names) {
+  CreateProfileState(1);
+  AssertAllAutoAddedGalleries();
+
+  // Get all existing gallery names.
+  ProfileState* profile_state = GetProfileState(0U);
+  *gallery_names =
+      profile_state->GetGalleryNames(profile_state->all_permission_extension());
+  ASSERT_EQ(3U, gallery_names->size());
+}
+
+void MediaFileSystemRegistryTest::CheckNewGallery(
+    ProfileState* profile_state,
+    const std::set<std::string>& gallery_names,
+    bool removable,
+    bool user_added) {
+  // Get new galleries.
+  std::set<std::string> new_gallery_names =
+      profile_state->GetGalleryNames(profile_state->all_permission_extension());
+  ASSERT_EQ(4U, new_gallery_names.size());
+
+  // Find the new one and check it.
+  std::vector<std::string> difference;
+  std::set_symmetric_difference(
+      gallery_names.begin(), gallery_names.end(),
+      new_gallery_names.begin(), new_gallery_names.end(),
+      std::back_inserter(difference));
+  ASSERT_EQ(1U, difference.size());
+  CheckGalleryJSONName(difference[0], removable, user_added);
+}
+
 std::vector<MediaFileSystemInfo>
 MediaFileSystemRegistryTest::GetAutoAddedGalleries(
     ProfileState* profile_state) {
@@ -670,6 +777,82 @@
                                 auto_galleries);
 }
 
+TEST_F(MediaFileSystemRegistryTest, GalleryNameDefault) {
+  std::set<std::string> gallery_names;
+  InitForGalleryNamesTest(&gallery_names);
+
+  for (std::set<std::string>::const_iterator it = gallery_names.begin();
+       it != gallery_names.end();
+       ++it) {
+    CheckGalleryJSONName(*it,
+                         false /*not removable*/,
+                         false /*not user added*/);
+  }
+}
+
+TEST_F(MediaFileSystemRegistryTest, GalleryNameMTP) {
+  std::set<std::string> gallery_names;
+  InitForGalleryNamesTest(&gallery_names);
+
+  FilePath location(FILE_PATH_LITERAL("/mtp_bogus"));
+  AttachDevice(MediaStorageUtil::MTP_OR_PTP, "mtp_fake_id", location);
+  CheckNewGallery(GetProfileState(0U), gallery_names,
+                  true /*removable*/, false /*not user added*/);
+}
+
+TEST_F(MediaFileSystemRegistryTest, GalleryNameDCIM) {
+  std::set<std::string> gallery_names;
+  InitForGalleryNamesTest(&gallery_names);
+
+  FilePath location(FILE_PATH_LITERAL("/removable_dcim_bogus"));
+  AttachDevice(MediaStorageUtil::REMOVABLE_MASS_STORAGE_WITH_DCIM,
+               "removable_dcim_fake_id",
+               location);
+  CheckNewGallery(GetProfileState(0U), gallery_names,
+                  true /*removable*/, false /*not user added*/);
+}
+
+TEST_F(MediaFileSystemRegistryTest, GalleryNameNoDCIM) {
+  std::set<std::string> gallery_names;
+  InitForGalleryNamesTest(&gallery_names);
+
+  std::string device_id =
+  AttachDevice(MediaStorageUtil::REMOVABLE_MASS_STORAGE_NO_DCIM,
+               empty_dir().AsUTF8Unsafe(),
+               empty_dir());
+  std::string device_id2 =
+      AddUserGallery(MediaStorageUtil::REMOVABLE_MASS_STORAGE_NO_DCIM,
+                     empty_dir().AsUTF8Unsafe(),
+                     empty_dir());
+  ASSERT_EQ(device_id, device_id2);
+  // Add permission for new non-default gallery.
+  ProfileState* profile_state = GetProfileState(0U);
+  SetGalleryPermission(profile_state,
+                       profile_state->all_permission_extension(),
+                       device_id,
+                       true /*has access*/);
+  CheckNewGallery(profile_state, gallery_names,
+                  true /*removable*/, true /*user added*/);
+}
+
+
+TEST_F(MediaFileSystemRegistryTest, GalleryNameUserAddedPath) {
+  std::set<std::string> gallery_names;
+  InitForGalleryNamesTest(&gallery_names);
+
+  std::string device_id = AddUserGallery(MediaStorageUtil::FIXED_MASS_STORAGE,
+                                         empty_dir().AsUTF8Unsafe(),
+                                         empty_dir());
+  // Add permission for new non-default gallery.
+  ProfileState* profile_state = GetProfileState(0U);
+  SetGalleryPermission(profile_state,
+                       profile_state->all_permission_extension(),
+                       device_id,
+                       true /*has access*/);
+  CheckNewGallery(profile_state, gallery_names,
+                  false /*not removable*/, true /*user added*/);
+}
+
 }  // namespace
 
 }  // namespace chrome