[webkit-reviews] review denied: [Bug 48985] [Qt][WK2] Left over files and shared memory segments : [Attachment 72928] proposed patch

[bugzilla-daemon at webkit.org]

Andreas Kling <kling at webkit.org> has denied Balazs Kelemen
<kbalazs at webkit.org>'s request for review:
Bug 48985: [Qt][WK2] Left over files and shared memory segments
https://bugs.webkit.org/show_bug.cgi?id=48985

Attachment 72928: proposed patch
https://bugs.webkit.org/attachment.cgi?id=72928&action=review

------- Additional Comments from Andreas Kling <kling at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=72928&action=review

Looks good in general, some problems though:

> WebKit2/Shared/qt/CrashHandler.cpp:38
> +CrashHandler::CrashHandler()
> +{

Missing initialization of m_inDeleteObjects!

> WebKit2/Shared/qt/CrashHandler.cpp:41
> +    signal(SIGABRT, &CrashHandler::signalHandler);
> +    signal(SIGSEGV, &CrashHandler::signalHandler);
> +    signal(SIGINT, &CrashHandler::signalHandler);

You probably want to catch SIGFPE, SIGILL, SIGQUIT, SIGTRAP and SIGBUS as well
here.

> WebKit2/Shared/qt/CrashHandler.cpp:54
> +    foreach(QObject* object, m_objects)
> +	   delete object;

qDeleteAll(m_objects);

> WebKit2/Shared/qt/CrashHandler.h:48
> +    void addToDeletion(QObject* object)

addToDeletion() sounds weird to me. deleteOnCrash() perhaps?
markForDeletionOnCrash()?