[webkit-reviews] review granted: [Bug 53716] JSC::Bindings m_rootObject->isValid() assert fails when running layout tests : [Attachment 81683] Patch to change failing ASSERT to conditionally remove RuntimeObject

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Michael Saboff <msaboff at apple.com>'s
request for review:
Bug 53716: JSC::Bindings m_rootObject->isValid() assert fails when running
layout tests
https://bugs.webkit.org/show_bug.cgi?id=53716

Attachment 81683: Patch to change failing ASSERT to conditionally remove
RuntimeObject
https://bugs.webkit.org/attachment.cgi?id=81683&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=81683&action=review

review+ but I would like the patch better if it didn’t do the same thing twice.


> Source/WebCore/bridge/jsc/BridgeJSC.cpp:110
> -    ASSERT(m_rootObject->isValid());
> -    m_rootObject->removeRuntimeObject(object);
> +    if (m_rootObject->isValid())
> +	   m_rootObject->removeRuntimeObject(object);

Good to remove the assertion, but why check isValid here since you already
added the check to removeRuntimeObject?