Bug 154125 – Need WTFCrash workaround for shipping SafariForWebKitDevelopment binaries.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Mark Lam]

Presently shipping SafariForWebKitDevelopment binaries still expect to link to a WTFCrash function.  We need to provide this function as a workaround until we can update SafariForWebKitDevelopment to use the new inlined version.

[Mark Lam]

Created attachment 271079 [details]
proposed patch.

[Blaze Burg]

Comment on attachment 271079 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=271079&action=review

> Source/WTF/wtf/Assertions.cpp:507
> +// This is a workaround for presently shipping (crica early 2016) SafariForWebKitDevelopment

Nit: 'circa'

[Mark Lam]

Comment on attachment 271079 [details]
proposed patch.

I've talked with Geoff offline, and am going to re-implement this in a hopefully cleaner way.  Stay tuned.

[Mark Lam]

Created attachment 271103 [details]
proposed patch 2 based on Geoff's suggestion.

[Joseph Pecoraro]

Comment on attachment 271103 [details]
proposed patch 2 based on Geoff's suggestion.

View in context: https://bugs.webkit.org/attachment.cgi?id=271103&action=review

> Source/WTF/wtf/Assertions.h:157
> +ALWAYS_INLINE NO_RETURN_DUE_TO_CRASH void CRASH()

It doesn't seem proper to introduce a new symbol into the global namespace that is not WTF prefixed. All other symbols in WTF introduces in Assertions.h/cpp are prefixed by "WTF".

Can CRASH stay a macro, but just call to WTFCrashImpl()?

> Source/WTF/wtf/Assertions.h:177
> +#else /* not defined(NDEBUG) && OS(DARWIN) */
> +
> +WTF_EXPORT_PRIVATE NO_RETURN_DUE_TO_CRASH void WTFCrashImpl();
> +#define CRASH() WTFCrashImpl()

And it would be pretty confusing for CRASH to be a macro on some ports, but a symbol on others. It would be nice to be consistent.

[Mark Lam]

Created attachment 271107 [details]
proposed patch 3 with Joe's feedback.

[Mark Lam]

Comment on attachment 271107 [details]
proposed patch 3 with Joe's feedback.

Will commit manually.

[Mark Lam]

Thanks for the review.  Landed in r196458: <http://trac.webkit.org/r196458>.