JSContext has a JSWrapperMap, which has an NSMutableDictionary m_classMap, which has values that are JSObjCClassInfo objects, which have strong references to two JSValue *'s, m_prototype and m_constructor, which in turn have strong references to the JSContext, creating a reference cycle. We should make m_prototype and m_constructor Weak<JSObject>. This gets rid of the strong reference to the JSContext and also prevents clients from accidentally creating reference cycles by assigning to the prototype of the constructor. If Weak<JSObject> fields are ever garbage collected, we will reallocate them.
<rdar://problem/13079710>
Created attachment 185314 [details] Patch
Attachment 185314 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSContext.mm', u'Source/JavaScriptCore/API/JSContextInternal.h', u'Source/JavaScriptCore/API/JSWrapperMap.mm', u'Source/JavaScriptCore/ChangeLog']" exit_code: 1 Source/JavaScriptCore/API/JSContextInternal.h:70: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 185314 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=185314&action=review > Source/JavaScriptCore/API/JSWrapperMap.mm:382 > + ASSERT(!m_constructor.get()); > + ASSERT(!m_prototype.get()); Since the prototype's .constructor property is user-modifiable, it's possible for m_prototype to be NULL when m_constructor is not.
> Since the prototype's .constructor property is user-modifiable, it's possible for m_prototype to be NULL when m_constructor is not. With the way it's written here, isn't that also true of the constructor's .prototype property? I guess the fix is to do the check for each and only re-generate the one(s) that have been collected.
Comment on attachment 185314 [details] Patch Clearing flags on attachment: 185314 Committed r141176: <http://trac.webkit.org/changeset/141176>
All reviewed patches have been landed. Closing bug.
> With the way it's written here, isn't that also true of the constructor's .prototype property? Yes, I think so. > I guess the fix is to do the check for each and only re-generate the one(s) that have been collected. Sounds right.
I'm reopening since those ASSERTs are wrong, along with some of the reallocation logic they're based on.
Created attachment 185338 [details] Patch
Comment on attachment 185338 [details] Patch r=me
Comment on attachment 185338 [details] Patch Clearing flags on attachment: 185338 Committed r141199: <http://trac.webkit.org/changeset/141199>