Refactor APIResourceController to ProfileKeyedService.

chrome/browser/extensions/api/api_resource_manager.h

+// 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.
+
+#ifndef CHROME_BROWSER_EXTENSIONS_API_API_RESOURCE_MANAGER_H_
+#define CHROME_BROWSER_EXTENSIONS_API_API_RESOURCE_MANAGER_H_
+
+#include <map>
+
+#include "base/memory/linked_ptr.h"
+#include "base/threading/non_thread_safe.h"
+#include "chrome/browser/profiles/profile_keyed_service.h"
+#include "content/public/browser/browser_thread.h"
+
+using content::BrowserThread;
+
+namespace extensions {
+
+template <class T>
+class ApiResourceManager : public ProfileKeyedService,

[asargent_no_longer_on_chrome, 2012/07/16 18:08:25]

nit: please add class overview comment

+                           public base::NonThreadSafe {
+ public:
+  ApiResourceManager()
+      : next_id_(1),
+        thread_id_(static_cast<BrowserThread::ID>(-1)),
+        api_resource_map_(new std::map<int, linked_ptr<T> >()) {
+  }
+
+  virtual ~ApiResourceManager() {
+    DCHECK(CalledOnValidThread());
+
+    DCHECK(BrowserThread::IsMessageLoopValid(thread_id())) <<
+        "A unit test is using an ApiResourceManager but didn't provide "
+        "the thread message loop needed for that kind of resource. "
+        "Please ensure that the appropriate message loop is operational.";
+
+    BrowserThread::DeleteSoon(thread_id(), FROM_HERE, api_resource_map_);
+  }
+
+  BrowserThread::ID thread_id() {
+    DCHECK(thread_id_ != -1) << "Before using this ApiResourceManager, "
+        "please set thread_id to the thread (IO, FILE) that the "
+        "managed resource requires.";
+    return thread_id_;
+  }
+
+  void set_thread_id(BrowserThread::ID thread_id) {
+    thread_id_ = thread_id;
+  }
+
+  // Takes ownership.
+  int Add(T* api_resource) {
+    DCHECK(BrowserThread::CurrentlyOn(thread_id()));
+    int id = GenerateId();
+    if (id > 0) {
+      linked_ptr<T> resource_ptr(api_resource);
+      (*api_resource_map_)[id] = resource_ptr;
+      return id;
+    }
+    return 0;
+  }
+
+  void Remove(int api_resource_id) {
+    DCHECK(BrowserThread::CurrentlyOn(thread_id()));
+    api_resource_map_->erase(api_resource_id);
+  }
+
+  T* Get(int api_resource_id) {
+    DCHECK(BrowserThread::CurrentlyOn(thread_id()));
+    linked_ptr<T> ptr = (*api_resource_map_)[api_resource_id];
+    return ptr.get();
+  }
+
+ private:
+  // TODO(miket): consider partitioning the ID space by extension ID to make it
+  // harder for extensions to peek into each others' resources.

[asargent_no_longer_on_chrome, 2012/07/16 18:08:25]

Another possibility would be to have one ApiResourceController instance per
extension - that might make it simpler to avoid forgetting to check the
extension id, remember to clean up all resources on extension disable/uninstall,
etc. It might create extra complexity elsewhere though so you might need to
start writing the code to see which way is better.

Either way, I think it's important that we try to prevent apps/extensions from
interfering with each other's resources by accidentally passing the wrong
resource id. 

[miket_OOO, 2012/07/16 19:46:02]

I've entered crbug.com/137539 to track more loudly.

+  int GenerateId() {
+    return next_id_++;
+  }
+
+  int next_id_;
+  BrowserThread::ID thread_id_;
+
+  // We need finer-grained control over the lifetime of this instance than RAII
+  // can give us.
+  std::map<int, linked_ptr<T> >* api_resource_map_;
+};
+
+}  // namespace extensions
+
+#endif  // CHROME_BROWSER_EXTENSIONS_API_API_RESOURCE_MANAGER_H_