Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(34)

Unified Diff: gin/wrappable.h

Issue 105423003: gin::Wrappable shouldn't inherit from base::RefCounted (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 7 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« gin/handle.h ('K') | « gin/handle.h ('k') | gin/wrappable.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: gin/wrappable.h
diff --git a/gin/wrappable.h b/gin/wrappable.h
index bad7958adab4cae788e9b257545e99e0ade4f812..333a31cedf4a24aa2ccc31b8efdd4f18bcf2dad4 100644
--- a/gin/wrappable.h
+++ b/gin/wrappable.h
@@ -5,25 +5,15 @@
#ifndef GIN_WRAPPABLE_H_
#define GIN_WRAPPABLE_H_
-#include "base/memory/ref_counted.h"
#include "gin/converter.h"
#include "gin/public/wrapper_info.h"
namespace gin {
// Wrappable is an abstract base class for C++ objects that have cooresponding
-// v8 wrapper objects. Wrappable are RefCounted, which means they can be
-// retained either by V8's garbage collector or by a scoped_refptr.
-//
-// WARNING: If you retain a Wrappable object with a scoped_refptr, it's possible
-// that the v8 wrapper can "fall off" if the wrapper object is not
-// referenced elsewhere in the V8 heap. Although Wrappable opens a
-// handle to the wrapper object, we make that handle as weak, which
-// means V8 is free to reclaim the wrapper. (We can't make the handle
-// strong without risking introducing a memory leak if an object that
-// holds a scoped_refptr is reachable from the wrapper.)
-//
-class Wrappable : public base::RefCounted<Wrappable> {
+// v8 wrapper objects. To retain a Wrappable object on the stack, use a
+// gin::Handle.
+class Wrappable {
public:
// Subclasses must return the WrapperInfo object associated with the
// v8::ObjectTemplate for their subclass. When creating a v8 wrapper for
@@ -51,12 +41,18 @@ class Wrappable : public base::RefCounted<Wrappable> {
// instead of overriding this function.
v8::Handle<v8::Object> GetWrapper(v8::Isolate* isolate);
+ // Subclasses should have private constructors and expose a static Create
+ // function that returns a gin::Handle. Forcing creators through this static
+ // Create function will enforce that clients actually create a wrapper for
+ // the object. If clients fail to create a wrapper for a wrappable object,
+ // the object will leak because we use the weak callback from the wrapper
+ // as the signal to delete the wrapped object.
+
protected:
Wrappable();
virtual ~Wrappable();
private:
- friend class base::RefCounted<Wrappable>;
friend struct Converter<Wrappable*>;
static void WeakCallback(
« gin/handle.h ('K') | « gin/handle.h ('k') | gin/wrappable.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698