[webkit-reviews] review granted: [Bug 215832] REGRESSION (r265908): Crash under Blob::arrayBuffer() / Blob::text() in stress GC : [Attachment 407258] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 215832: REGRESSION (r265908): Crash under Blob::arrayBuffer() /
Blob::text() in stress GC
https://bugs.webkit.org/show_bug.cgi?id=215832

Attachment 407258: Patch

https://bugs.webkit.org/attachment.cgi?id=407258&action=review




--- Comment #3 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 407258
  --> https://bugs.webkit.org/attachment.cgi?id=407258
Patch

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

> Source/WebCore/ChangeLog:17
> +	   No new tests, already covered by existing WPT tests that are
crashing on stress GC bots.

Can we add a new test that uses GCController to reliably reproduce this issue
without stress GC?

> Source/WebCore/dom/DataTransfer.cpp:371
> -FileList& DataTransfer::files() const
> +FileList& DataTransfer::files(ScriptExecutionContext* document) const

I'm a bit confused here. Why pointer? And why do we need to take
ScriptExecutionContext instead of Document??

> Source/WebCore/fileapi/Blob.h:64
> +	   auto blob = adoptRef(*new Blob(context));
> +	   blob->suspendIfNeeded();

Can we do this in the constructor instead like we do elsewhere?