Add assertion, off by default, for when you forget to do adoptRef
Created attachment 60133 [details] Patch
Attachment 60133 [details] did not build on mac: Build output: http://webkit-commit-queue.appspot.com/results/3370085
Attachment 60133 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/3283986
Attachment 60133 [details] did not build on gtk: Build output: http://webkit-commit-queue.appspot.com/results/3375060
Created attachment 60151 [details] Patch
New patch fixes the release build.
Should we do style cleanup in a separate patch?
(In reply to comment #7) > Should we do style cleanup in a separate patch? The style changes here are small and should be easy to look at, and this is a patch that limits itself to a small number of source files, but if I have an enthusiastic reviewer who wants a style-only patch for the same files, that’s OK with me too.
Comment on attachment 60151 [details] Patch Looks good. Not sure why I went off to template land before for this patch. JavaScriptCore/wtf/PassRefPtr.h:32 + inline void adopted(const void*) One line? JavaScriptCore/wtf/RefCounted.h:70 + ASSERT(m_deletionHasBegun); This means we'll need to delete via deref(), right?
Comment on attachment 60151 [details] Patch I'm curious if there is a benefit to making adopted() a free function instead of a member function?
(In reply to comment #9) > JavaScriptCore/wtf/PassRefPtr.h:32 > + inline void adopted(const void*) > One line? Sure, that would be OK. > JavaScriptCore/wtf/RefCounted.h:70 > + ASSERT(m_deletionHasBegun); > This means we'll need to delete via deref(), right? Yes. I think this goes hand in hand with requiring adoptRef. It found some good things when I turned it on. I had these assertions on for a while and was fixing all the things they found, but then I decided it was better to do a simple patch that makes it so other could work on it in parallel if they like.
(In reply to comment #10) > I'm curious if there is a benefit to making adopted() a free function instead of a member function? I don't think it could be a member function. A member function of what class?
(In reply to comment #12) > (In reply to comment #10) > > I'm curious if there is a benefit to making adopted() a free function instead of a member function? > > I don't think it could be a member function. A member function of what class? I see. RefCounted is not the only way to make something play nice in the RefPtr system. Makes sense now.
Committed r62213: <http://trac.webkit.org/changeset/62213>