Generalize ExtensionIconSet to store icon paths for custom size sets

chrome/common/extensions/extension_icon_set.cc

 // Copyright (c) 2012 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/common/extensions/extension_icon_set.h"
 
 #include "base/logging.h"
 
-ExtensionIconSet::ExtensionIconSet() {}
+namespace {
+
+const int kExtensionIconSizes[] = {

[Aaron Boodman, 2012/08/10 23:01:57]

Now that this class is more general purpose, move these constants (which refer
to a particular set of icons) out to someplace like extension_constants.h.

[tbarzic, 2012/08/11 00:58:45]

Done.

+  ExtensionIconSet::EXTENSION_ICON_GIGANTOR,  // 512
+  ExtensionIconSet::EXTENSION_ICON_EXTRA_LARGE,  // 256
+  ExtensionIconSet::EXTENSION_ICON_LARGE,  // 128
+  ExtensionIconSet::EXTENSION_ICON_MEDIUM,  // 48
+  ExtensionIconSet::EXTENSION_ICON_SMALL,  // 32
+  ExtensionIconSet::EXTENSION_ICON_SMALLISH,  // 24
+  ExtensionIconSet::EXTENSION_ICON_BITTY  // 16
+};
+
+bool IsInArray(const int* const array, size_t array_size, int value) {
+  for (size_t i = 0; i < array_size; i++) {
+    if (array[i] == value)
+      return true;
+  }
+  return false;
+}
+
+}  // namespace
+
+ExtensionIconSet::ExtensionIconSet(IconSetType type)
+    : allowed_sizes_(NULL),
+      num_allowed_sizes_(0){
+  switch (type) {
+    case ICON_SET_MANIFEST_ICONS:
+      allowed_sizes_ = kExtensionIconSizes;

[Aaron Boodman, 2012/08/10 23:01:57]

I do not think it is necessary to validate that the icons added are within the
set of allowed sizes. You can remove IconSetType.

[tbarzic, 2012/08/11 00:58:45]

Done.

+      num_allowed_sizes_ = arraysize(kExtensionIconSizes);
+      break;
+    default:
+      NOTREACHED();
+  }
+
+}
 
 ExtensionIconSet::~ExtensionIconSet() {}
 
-const ExtensionIconSet::Icons ExtensionIconSet::kIconSizes[] = {
-  EXTENSION_ICON_GIGANTOR,
-  EXTENSION_ICON_EXTRA_LARGE,
-  EXTENSION_ICON_LARGE,
-  EXTENSION_ICON_MEDIUM,
-  EXTENSION_ICON_SMALL,
-  EXTENSION_ICON_SMALLISH,
-  EXTENSION_ICON_BITTY
-};
-
-const size_t ExtensionIconSet::kNumIconSizes =
-    arraysize(ExtensionIconSet::kIconSizes);
-
 void ExtensionIconSet::Clear() {
   map_.clear();
 }
 
-void ExtensionIconSet::Add(Icons size, const std::string& path) {
+void ExtensionIconSet::Add(int size, const std::string& path) {
   DCHECK(!path.empty() && path[0] != '/');
+  CHECK(IsInArray(allowed_sizes_, num_allowed_sizes_, size));
+
   map_[size] = path;
 }
 
 std::string ExtensionIconSet::Get(int size, MatchType match_type) const {
   // The searches for MATCH_BIGGER and MATCH_SMALLER below rely on the fact that
   // std::map is sorted. This is per the spec, so it should be safe to rely on.
   if (match_type == MATCH_EXACTLY) {
-    IconMap::const_iterator result = map_.find(static_cast<Icons>(size));
+    IconMap::const_iterator result = map_.find(size);
     return result == map_.end() ? std::string() : result->second;
   } else if (match_type == MATCH_SMALLER) {
     IconMap::const_reverse_iterator result = map_.rend();
     for (IconMap::const_reverse_iterator iter = map_.rbegin();
          iter != map_.rend(); ++iter) {
       if (iter->first <= size) {
         result = iter;
         break;
       }
     }
     return result == map_.rend() ? std::string() : result->second;
   } else {
     DCHECK(match_type == MATCH_BIGGER);
     IconMap::const_iterator result = map_.end();
     for (IconMap::const_iterator iter = map_.begin(); iter != map_.end();
          ++iter) {
       if (iter->first >= size) {
         result = iter;
         break;
       }
     }
     return result == map_.end() ? std::string() : result->second;
   }
 }
 
 bool ExtensionIconSet::ContainsPath(const std::string& path) const {
-  return GetIconSizeFromPath(path) != EXTENSION_ICON_INVALID;
+  return GetIconSizeFromPath(path) != 0;
 }
 
-ExtensionIconSet::Icons ExtensionIconSet::GetIconSizeFromPath(
-    const std::string& path) const {
+int ExtensionIconSet::GetIconSizeFromPath(const std::string& path) const {
   if (path.empty())
-    return EXTENSION_ICON_INVALID;
+    return 0;
 
   DCHECK(path[0] != '/') <<
       "ExtensionIconSet stores icon paths without leading slash.";
 
   for (IconMap::const_iterator iter = map_.begin(); iter != map_.end();
        ++iter) {
     if (iter->second == path)
       return iter->first;
   }
 
-  return EXTENSION_ICON_INVALID;
+  return 0;
 }