Bug 87581

Created attachment 144207 [details] Patch

Comment on attachment 144207 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144207&action=review r+ assuming you change the !ASSERTION bits to GC validation, and can reasonably answer the static cast questions :D > Source/JavaScriptCore/API/JSCallbackConstructor.cpp:64 > - jsCast<JSCallbackConstructor*>(cell)->JSCallbackConstructor::~JSCallbackConstructor(); > + static_cast<JSCallbackConstructor*>(cell)->JSCallbackConstructor::~JSCallbackConstructor(); Why are you making this change? > Source/JavaScriptCore/API/JSCallbackObject.cpp:57 > - jsCast<JSCallbackObject*>(cell)->JSCallbackObject::~JSCallbackObject(); > + static_cast<JSCallbackObject*>(cell)->JSCallbackObject::~JSCallbackObject(); ditto > Source/JavaScriptCore/API/JSCallbackObject.cpp:63 > - JSObjectRef thisRef = toRef(asObject(handle.get())); > + JSObjectRef thisRef = toRef(static_cast<JSObject*>(handle.get().asCell())); if a static cast is valid, a jsCast should be as well -- why isn't it? > Source/JavaScriptCore/heap/MarkedBlock.cpp:71 > +#if !ASSERT_DISABLED Make this conditional on GC validation, not assertions. There are times where it's nice to be able to test stuff in release builds. > Source/JavaScriptCore/heap/WeakSetInlines.h:53 > +#if !ASSERT_DISABLED > + weakImpl->jsValue().asCell()->clearStructure(); GC validation rather than assertion based... can you have multiple weak handles to a single object? might this break everything?

> > Source/JavaScriptCore/API/JSCallbackConstructor.cpp:64 > > - jsCast<JSCallbackConstructor*>(cell)->JSCallbackConstructor::~JSCallbackConstructor(); > > + static_cast<JSCallbackConstructor*>(cell)->JSCallbackConstructor::~JSCallbackConstructor(); > > Why are you making this change? jsCast does Structure-based validation, and our Structure is not guaranteed to be alive when we get finalized. In particular, if our Structure has been recycled, the jsCast will probably ASSERT, and if our Structure has been unmapped from memory, the jsCast will segfault. static_cast allows us to access our object just enough to deref / free its C++ pointers. Perhaps we can clarify this interface in the future. > > Source/JavaScriptCore/heap/MarkedBlock.cpp:71 > > +#if !ASSERT_DISABLED > > Make this conditional on GC validation, not assertions. Added || ENABLE(GC_VALIDATION) > can you have multiple weak handles to a single object? might this break everything? Yes, you can. No, it doesn't break anything. If one handle is dead, they're all dead, so scribbling this structure is correct for them all.

Committed r118616: <http://trac.webkit.org/changeset/118616>