[webkit-reviews] review denied: [Bug 43207] WeakGCPtr's need to be updated for movable objects : [Attachment 63635] patch (with patch!)

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has denied Nathan Lawrence
<nlawrence at apple.com>'s request for review:
Bug 43207: WeakGCPtr's need to be updated for movable objects
https://bugs.webkit.org/show_bug.cgi?id=43207

Attachment 63635: patch (with patch!)
https://bugs.webkit.org/attachment.cgi?id=63635&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
JavaScriptCore/runtime/Collector.cpp:938
 +	m_weakGCHandlePools.append(allocation);
I think it would be better to use a smaller block allocator for WeakGCHandles,
since there are usually so few of them. Maybe 32K instead of 256K?

JavaScriptCore/runtime/GCHandle.h:66
 +	}
The canonical names for these accessors in smart pointers are "get" and "set".

JavaScriptCore/runtime/GCHandle.h:78
 +	}
I would call these "nextInFreeList" and "setNextInFreeList".

JavaScriptCore/runtime/GCHandle.cpp:74
 +	return 0;
You can turn the "if" above into the ASSERT, and remove the ASSERT_NOT_REACHED
and bogus return at the end.