[webkit-reviews] review granted: [Bug 70902] CRASH() in ScriptExecutionContext::destroyedActiveDOMObject() : [Attachment 112498] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Yuta Kitamura <yutak at chromium.org>'s
request for review:
Bug 70902: CRASH() in ScriptExecutionContext::destroyedActiveDOMObject()
https://bugs.webkit.org/show_bug.cgi?id=70902

Attachment 112498: Patch
https://bugs.webkit.org/attachment.cgi?id=112498&action=review

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


Is always delaying a good thing? Will this hurt performance?

> Source/WebCore/websockets/WebSocket.cpp:454
> +    virtual void performTask(ScriptExecutionContext*)
> +    {
> +	   if (m_webSocket->hasPendingActivity())
> +	       m_webSocket->unsetPendingActivity(m_webSocket);
> +    }
> +
> +    virtual bool isCleanupTask() const { return true; }

These two functions should be private.

> Source/WebCore/websockets/WebSocket.cpp:469
> +	   scriptExecutionContext()->postTask(adoptPtr(new
DelayedWebSocketCleanupTask(this)));

I’d like to see a “why” comment explaining why this is delayed.

> Source/WebCore/xml/XMLHttpRequest.cpp:819
> +    scriptExecutionContext()->postTask(adoptPtr(new
DelayedXMLHttpRequestCleanupTask(this)));

I’d like to see a “why” comment explaining why this is delayed.