Bug 193126 – AtomicStringImpl::isInAtomicStringTable used without check ASSERT_DISABLED

• Status: NEW
• Component: Web Template Framework

[Zhifei Fang]

We declare isInAtomicStringTable like this:


#if !ASSERT_DISABLED
    WTF_EXPORT_STRING_API static bool isInAtomicStringTable(StringImpl*);
#endif


however we use it like this:


private:
    AtomicStringImpl() = delete;

    ALWAYS_INLINE static Ref<AtomicStringImpl> add(StringImpl& string)
    {
        if (string.isAtomic()) {
>           ASSERT_WITH_MESSAGE(!string.length() || isInAtomicStringTable(&string), "The atomic string comes from an other thread!");
                                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            return static_cast<AtomicStringImpl&>(string);
        }
        return addSlowCase(string);
    }


if we define ASSERT_DISABLED this will cause a build failure

[Zhifei Fang]

root cause is 

ASSERT_WITH_MESSAGE not controlled by ASSERT_DISABLED

[Alexey Proskuryakov]

It doesn't look like there is a reason why ASSERT_DISABLED and ASSERT_MSG_DISABLED are separate. Perhaps we should unify them, or at least disable ASSERT_WITH_MESSAGE when ASSERT_DISABLED is set.