Media Galleries: Move gallery name generation back to StorageMonitor.

chrome/browser/storage_monitor/storage_info.cc

Index: chrome/browser/storage_monitor/storage_info.cc
===================================================================
--- chrome/browser/storage_monitor/storage_info.cc	(revision 213035)
+++ chrome/browser/storage_monitor/storage_info.cc	(working copy)
@@ -5,6 +5,11 @@
 #include "chrome/browser/storage_monitor/storage_info.h"
 
 #include "base/logging.h"
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/storage_monitor/media_storage_util.h"
+#include "grit/generated_resources.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/base/text/bytes_formatting.h"
 
 namespace chrome {
 
@@ -19,20 +24,53 @@
 const char kITunesPrefix[] = "itunes:";
 const char kPicasaPrefix[] = "picasa:";
 
+string16 GetDisplayNameForDevice(uint64 storage_size_in_bytes,
+                                 const string16& name) {
+  DCHECK(!name.empty());
+  return (storage_size_in_bytes == 0) ?
+      name :
+      ui::FormatBytes(storage_size_in_bytes) + base::ASCIIToUTF16(" ") + name;
+}
+
+// For a device with |device_name| and a relative path |sub_folder|, construct
+// a display name. If |sub_folder| is empty, then just return |device_name|.
+string16 GetDisplayNameForSubFolder(const string16& device_name,
+                                    const base::FilePath& sub_folder) {
+  if (sub_folder.empty())
+    return device_name;
+  return (sub_folder.BaseName().LossyDisplayName() +
+          base::ASCIIToUTF16(" - ") +
+          device_name);
+}
+
+string16 GetFullProductName(const string16& vendor_name,
+                            const string16& model_name) {
+  if (vendor_name.empty() && model_name.empty())
+    return string16();
+
+  string16 product_name;
+  if (vendor_name.empty())
+    product_name = model_name;
+  else if (model_name.empty())
+    product_name = vendor_name;
+  else if (!vendor_name.empty() && !model_name.empty())
+    product_name = vendor_name + base::UTF8ToUTF16(", ") + model_name;
+
+  return product_name;
+}
+
 }  // namespace
 
 StorageInfo::StorageInfo() : total_size_in_bytes_(0) {
 }
 
 StorageInfo::StorageInfo(const std::string& device_id_in,
-                         const string16& device_name,
                          const base::FilePath::StringType& device_location,
                          const string16& label,
                          const string16& vendor,
                          const string16& model,
                          uint64 size_in_bytes)
     : device_id_(device_id_in),
-      name_(device_name),
       location_(device_location),
       storage_label_(label),
       vendor_name_(vendor),
@@ -142,4 +180,41 @@
   return CrackDeviceId(device_id, &type, NULL) && type == PICASA;
 }
 
+base::string16 StorageInfo::GetDisplayName(bool with_size) const {
+  return GetDisplayNameWithOverride(base::string16(), with_size);
+}
+
+base::string16 StorageInfo::GetDisplayNameWithOverride(
+    const base::string16& override_display_name, bool with_size) const {
+  if (!IsRemovableDevice(device_id_)) {
+    // For fixed storage, the name is the directory name, or, in the case
+    // of a root directory, the root directory name.
+    // TODO(gbillock): Using only the BaseName can lead to ambiguity. A tooltip
+    // will resolves it. Is that enough?
+    if (!override_display_name.empty())
+      return override_display_name;
+    base::FilePath path = base::FilePath(location_);
+    if (path == path.DirName())
+      return path.LossyDisplayName();
+    return path.BaseName().LossyDisplayName();
+  }
+
+  string16 name = override_display_name;
+  if (name.empty())
+    name = storage_label_;
+  if (name.empty())
+    name = GetFullProductName(vendor_name_, model_name_);
+  if (name.empty())
+    name = l10n_util::GetStringUTF16(IDS_MEDIA_GALLERIES_UNLABELED_DEVICE);
+
+  if (with_size)
+    name = GetDisplayNameForDevice(total_size_in_bytes_, name);
+
+  base::FilePath subfolder;
+  base::FilePath base_path = MediaStorageUtil::FindDevicePathById(device_id_);
+  if (!base_path.empty() && (base_path.value() != location_))

[vandebo (ex-Chrome), 2013/07/23 23:28:55]

This is kind of ugly.  Generally this should happen, but it does since you've
abused location in the MediaGalleryPrefInfo call site.  I'm not sure if there's
a cleaner way around, but it's worth thinking about.

[Lei Zhang, 2013/07/24 05:13:34]

Not sure what you mean. Let's chat tomorrow.

[Lei Zhang, 2013/07/29 20:48:27]

Ah, I think you were trying to say StorageInfo's |location_| should not include
the relative path from the path to device. I put the sub-folder display code
back where it was before.

+    CHECK(base_path.AppendRelativePath(base::FilePath(location_), &subfolder));
+  return GetDisplayNameForSubFolder(name, subfolder);
+}
+
 }  // namespace chrome