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

extensions/browser/api/api_resource_manager.h

 // Copyright 2014 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.
 
 #ifndef EXTENSIONS_BROWSER_API_API_RESOURCE_MANAGER_H_
 #define EXTENSIONS_BROWSER_API_API_RESOURCE_MANAGER_H_
 
 #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"
 #include "base/threading/non_thread_safe.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_service.h"
 #include "extensions/browser/browser_context_keyed_api_factory.h"
@@ skipping 201 matching lines @@
     ApiResourceData() : next_id_(1) {}
 
     int Add(T* api_resource) {
       DCHECK(ThreadingTraits::IsCalledOnValidThread());
       int id = GenerateId();
       if (id > 0) {
         linked_ptr<T> resource_ptr(api_resource);
         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;
     }
 
     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);
       }
     }
 
     T* Get(const std::string& extension_id, int api_resource_id) {
       DCHECK(ThreadingTraits::IsCalledOnValidThread());
       return GetOwnedResource(extension_id, api_resource_id);
     }
 
     // Change the resource mapped to this |extension_id| at this
@@ skipping 58 matching lines @@
     T* GetOwnedResource(const std::string& extension_id, int api_resource_id) {
       linked_ptr<T> ptr = api_resource_map_[api_resource_id];
       T* resource = ptr.get();
       if (resource && extension_id == resource->owner_extension_id())
         return resource;
       return NULL;
     }
 
     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(
         const std::string& extension_id) {
       CleanupResourcesFromExtension(extension_id, true);
     }
 
     void CleanupResourcesFromSuspendedExtension(
         const std::string& extension_id) {
       CleanupResourcesFromExtension(extension_id, false);
     }
 
     void CleanupResourcesFromExtension(const std::string& extension_id,
                                        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;
         if (remove_all) {
           erase = true;
         } else {
           linked_ptr<T> ptr = api_resource_map_[*it];
           T* resource = ptr.get();
           erase = (resource && !resource->IsPersistent());
         }
@@ skipping 95 matching lines @@
         ->GetSequencedTaskRunnerWithShutdownBehavior(
             content::BrowserThread::GetBlockingPool()->GetNamedSequenceToken(
                 T::kSequenceToken),
             T::kShutdownBehavior);
   }
 };
 
 }  // namespace extensions
 
 #endif  // EXTENSIONS_BROWSER_API_API_RESOURCE_MANAGER_H_