Cleanup: Remove some std::map::operator[] usage in ApiResourceManager.

extensions/browser/api/api_resource_manager.h

Index: extensions/browser/api/api_resource_manager.h
diff --git a/extensions/browser/api/api_resource_manager.h b/extensions/browser/api/api_resource_manager.h
index e8b5d3340c7a8648b2d5cf7054c8d3d5a50adec2..6791c1b485439184f5ce2181981b052356f090f2 100644
--- a/extensions/browser/api/api_resource_manager.h
+++ b/extensions/browser/api/api_resource_manager.h
@@ -8,7 +8,6 @@
 #include <map>
 
 #include "base/containers/hash_tables.h"
-#include "base/lazy_instance.h"
 #include "base/memory/linked_ptr.h"
 #include "base/memory/ref_counted.h"
 #include "base/scoped_observer.h"
@@ -230,12 +229,13 @@ class ApiResourceManager : public BrowserContextKeyedAPI,
         api_resource_map_[id] = resource_ptr;
 
         const std::string& extension_id = api_resource->owner_extension_id();
-        if (extension_resource_map_.find(extension_id) ==
-            extension_resource_map_.end()) {
-          extension_resource_map_[extension_id] = base::hash_set<int>();
+        ExtensionToResourceMap::iterator it =
+            extension_resource_map_.find(extension_id);
+        if (it == extension_resource_map_.end()) {
+          it = extension_resource_map_.insert(
+              std::make_pair(extension_id, base::hash_set<int>())).first;
         }
-        extension_resource_map_[extension_id].insert(id);
-
+        it->second.insert(id);
         return id;
       }
       return 0;
@@ -243,10 +243,10 @@ class ApiResourceManager : public BrowserContextKeyedAPI,
 
     void Remove(const std::string& extension_id, int api_resource_id) {
       DCHECK(ThreadingTraits::IsCalledOnValidThread());
-      if (GetOwnedResource(extension_id, api_resource_id) != NULL) {
-        DCHECK(extension_resource_map_.find(extension_id) !=

[Lei Zhang, 2014/09/19 05:53:05]

std::map::operator[] has the side effect of creating the |extension_id| mapping
if it didn't exist. So in real life, you can blow through this DCHECK without a
problem. In the new code, you'll get a crash and actually be alerted to DCHECK()
failures in production, if any.

[Ken Rockot(use gerrit already), 2014/09/19 16:37:06]

Ah. Yeah. This is less a problem with operator[] and more a problem with putting
important program logic in DCHECKs.

But I don't like operator[] anyway, so... great!

-               extension_resource_map_.end());
-        extension_resource_map_[extension_id].erase(api_resource_id);
+      if (GetOwnedResource(extension_id, api_resource_id)) {
+        ExtensionToResourceMap::iterator it =
+            extension_resource_map_.find(extension_id);
+        it->second.erase(api_resource_id);
         api_resource_map_.erase(api_resource_id);
       }
     }
@@ -325,11 +325,11 @@ class ApiResourceManager : public BrowserContextKeyedAPI,
 
     base::hash_set<int>* GetOwnedResourceIds(const std::string& extension_id) {
       DCHECK(ThreadingTraits::IsCalledOnValidThread());
-      if (extension_resource_map_.find(extension_id) ==
-          extension_resource_map_.end())
+      ExtensionToResourceMap::iterator it =
+          extension_resource_map_.find(extension_id);
+      if (it == extension_resource_map_.end())
         return NULL;
-
-      return &extension_resource_map_[extension_id];
+      return &(it->second);
     }
 
     void CleanupResourcesFromUnloadedExtension(
@@ -346,14 +346,14 @@ class ApiResourceManager : public BrowserContextKeyedAPI,
                                        bool remove_all) {
       DCHECK(ThreadingTraits::IsCalledOnValidThread());
 
-      if (extension_resource_map_.find(extension_id) ==
-          extension_resource_map_.end()) {
+      ExtensionToResourceMap::iterator it =
+          extension_resource_map_.find(extension_id);
+      if (it == extension_resource_map_.end())
         return;
-      }
 
       // Remove all resources, or the non persistent ones only if |remove_all|
       // is false.
-      base::hash_set<int>& resource_ids = extension_resource_map_[extension_id];
+      base::hash_set<int>& resource_ids = it->second;
       for (base::hash_set<int>::iterator it = resource_ids.begin();
            it != resource_ids.end();) {
         bool erase = false;