Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
REGRESSION (r265908): Crash under Blob::arrayBuffer() / Blob::text() … 
…in stress GC

https://bugs.webkit.org/show_bug.cgi?id=215832
<rdar://problem/67741677>

Reviewed by Ryosuke Niwa.

Source/WebCore:

r265908 added support for Blob::arrayBuffer() / Blob::text() which are asynchronous operations
returning promises. The crash is due to the fact that the Blob's JS wrapper may get garbage
collected before the promise is settled.

To address the issue, this patch makes Blob an ActiveDOMObject and creates an
ActiveDOMObject::pendingActivity whenever there is a pending promise so that the JS wrapper
does not get garbage collected too early.

Test: fast/files/blob-text-gc.html

* Modules/async-clipboard/Clipboard.cpp:
(WebCore::Clipboard::getType):
* Modules/async-clipboard/ClipboardImageReader.h:
(WebCore::ClipboardImageReader::ClipboardImageReader):
* Modules/async-clipboard/ClipboardItem.cpp:
(WebCore::ClipboardItem::blobFromString):
* Modules/async-clipboard/ClipboardItem.h:
* Modules/async-clipboard/ClipboardItemBindingsDataSource.cpp:
(WebCore::ClipboardItemBindingsDataSource::getType):
* Modules/async-clipboard/ios/ClipboardImageReaderIOS.mm:
(WebCore::ClipboardImageReader::readBuffer):
* Modules/async-clipboard/mac/ClipboardImageReaderMac.mm:
(WebCore::ClipboardImageReader::readBuffer):
* Modules/entriesapi/DOMFileSystem.cpp:
(WebCore::DOMFileSystem::getFile):
* Modules/entriesapi/DOMFileSystem.h:
* Modules/entriesapi/FileSystemFileEntry.cpp:
(WebCore::FileSystemFileEntry::file):
* Modules/entriesapi/FileSystemFileEntry.h:
* Modules/entriesapi/FileSystemFileEntry.idl:
* Modules/fetch/FetchBody.cpp:
(WebCore::FetchBody::fromFormData):
* Modules/fetch/FetchBody.h:
* Modules/fetch/FetchBodyConsumer.cpp:
(WebCore::blobFromData):
(WebCore::packageFormData):
(WebCore::resolveWithTypeAndData):
(WebCore::FetchBodyConsumer::resolve):
(WebCore::FetchBodyConsumer::takeAsBlob):
* Modules/fetch/FetchBodyConsumer.h:
* Modules/fetch/FetchBodyOwner.cpp:
(WebCore::FetchBodyOwner::blob):
* Modules/mediarecorder/MediaRecorder.cpp:
(WebCore::MediaRecorder::stopRecording):
(WebCore::MediaRecorder::requestData):
* Modules/mediastream/RTCDataChannel.cpp:
(WebCore::RTCDataChannel::didReceiveRawData):
* Modules/websockets/WebSocket.cpp:
(WebCore::WebSocket::didReceiveBinaryData):
* Modules/websockets/WorkerThreadableWebSocketChannel.cpp:
(WebCore::WorkerThreadableWebSocketChannel::Bridge::send):
* bindings/js/SerializedScriptValue.cpp:
(WebCore::CloneDeserializer::readFile):
(WebCore::CloneDeserializer::readTerminal):
* dom/DataTransfer.cpp:
(WebCore::DataTransfer::updateFileList):
(WebCore::DataTransfer::items):
(WebCore::DataTransfer::filesFromPasteboardAndItemList const):
(WebCore::DataTransfer::files const):
* dom/DataTransfer.h:
* dom/DataTransfer.idl:
* dom/DataTransferItemList.cpp:
(WebCore::DataTransferItemList::DataTransferItemList):
(WebCore::DataTransferItemList::remove):
(WebCore::DataTransferItemList::clear):
(WebCore::DataTransferItemList::ensureItems const):
(WebCore::DataTransferItemList::document const):
* dom/DataTransferItemList.h:
* dom/DataTransferItemList.idl:
* editing/WebCorePasteboardFileReader.cpp:
(WebCore::WebCorePasteboardFileReader::readFilename):
(WebCore::WebCorePasteboardFileReader::readBuffer):
* editing/WebCorePasteboardFileReader.h:
* editing/cocoa/WebContentReaderCocoa.mm:
(WebCore::createFragmentForImageAttachment):
(WebCore::replaceRichContentWithAttachments):
(WebCore::createFragmentAndAddResources):
(WebCore::sanitizeMarkupWithArchive):
(WebCore::WebContentReader::readImage):
(WebCore::attachmentForFilePath):
(WebCore::attachmentForData):
* editing/markup.cpp:
(WebCore::restoreAttachmentElementsInFragment):
* fileapi/Blob.cpp:
(WebCore::Blob::Blob):
(WebCore::Blob::loadBlob):
(WebCore::Blob::activeDOMObjectName const):
* fileapi/Blob.h:
(WebCore::Blob::create):
(WebCore::Blob::deserialize):
(WebCore::Blob::slice const):
* fileapi/Blob.idl:
* fileapi/File.cpp:
(WebCore::File::createWithRelativePath):
(WebCore::File::create):
(WebCore::File::File):
(WebCore::File::activeDOMObjectName const):
* fileapi/File.h:
* fileapi/File.idl:
* html/DOMFormData.cpp:
(WebCore::DOMFormData::createFileEntry):
* html/DirectoryFileListCreator.cpp:
(WebCore::FileInformation::isolatedCopy const):
(WebCore::appendDirectoryFiles):
(WebCore::gatherFileInformation):
(WebCore::toFileList):
(WebCore::DirectoryFileListCreator::start):
* html/DirectoryFileListCreator.h:
* html/FileInputType.cpp:
(WebCore::FileInputType::appendFormData const):
(WebCore::FileInputType::filesChosen):
* html/HTMLAttachmentElement.cpp:
(WebCore::HTMLAttachmentElement::updateEnclosingImageWithData):
* html/HTMLCanvasElement.cpp:
(WebCore::HTMLCanvasElement::toBlob):
* testing/Internals.cpp:
(WebCore::Internals::createFile):
* testing/ServiceWorkerInternals.cpp:
(WebCore::ServiceWorkerInternals::createOpaqueWithBlobBodyResponse):
* workers/service/context/ServiceWorkerFetch.cpp:
(WebCore::ServiceWorkerFetch::dispatchFetchEvent):
* xml/XMLHttpRequest.cpp:
(WebCore::XMLHttpRequest::createResponseBlob):

Source/WebKit:

* WebProcess/Automation/WebAutomationSessionProxy.cpp:
(WebKit::WebAutomationSessionProxy::setFilesForInputFileUpload):

LayoutTests:

Add better test coverage.

* fast/files/blob-text-gc-expected.txt: Added.
* fast/files/blob-text-gc.html: Added.


Canonical link: https://commits.webkit.org/228637@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@266168 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
@cdumez
cdumez committed Aug 26, 2020
1 parent 257b3b5 commit 6c68ec8
Show file tree
Hide file tree
Showing 56 changed files with 589 additions and 185 deletions.
13 changes: 13 additions & 0 deletions LayoutTests/ChangeLog
View file
@@ -1,3 +1,16 @@
2020-08-26 Chris Dumez <cdumez@apple.com>

REGRESSION (r265908): Crash under Blob::arrayBuffer() / Blob::text() in stress GC
https://bugs.webkit.org/show_bug.cgi?id=215832
<rdar://problem/67741677>

Reviewed by Ryosuke Niwa.

Add better test coverage.

* fast/files/blob-text-gc-expected.txt: Added.
* fast/files/blob-text-gc.html: Added.

2020-08-26 Youenn Fablet <youenn@apple.com>

enumerateDevices should expose audiooutput devices that are tied to an audio input device
Expand Down
109 changes: 109 additions & 0 deletions LayoutTests/fast/files/blob-text-gc-expected.txt
View file
@@ -0,0 +1,109 @@
Make sure the Blob.text() promise gets resolved even if the Blob object is not kept alive by JS.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS text is "foobar0"
PASS text is "foobar1"
PASS text is "foobar2"
PASS text is "foobar3"
PASS text is "foobar4"
PASS text is "foobar5"
PASS text is "foobar6"
PASS text is "foobar7"
PASS text is "foobar8"
PASS text is "foobar9"
PASS text is "foobar10"
PASS text is "foobar11"
PASS text is "foobar12"
PASS text is "foobar13"
PASS text is "foobar14"
PASS text is "foobar15"
PASS text is "foobar16"
PASS text is "foobar17"
PASS text is "foobar18"
PASS text is "foobar19"
PASS text is "foobar20"
PASS text is "foobar21"
PASS text is "foobar22"
PASS text is "foobar23"
PASS text is "foobar24"
PASS text is "foobar25"
PASS text is "foobar26"
PASS text is "foobar27"
PASS text is "foobar28"
PASS text is "foobar29"
PASS text is "foobar30"
PASS text is "foobar31"
PASS text is "foobar32"
PASS text is "foobar33"
PASS text is "foobar34"
PASS text is "foobar35"
PASS text is "foobar36"
PASS text is "foobar37"
PASS text is "foobar38"
PASS text is "foobar39"
PASS text is "foobar40"
PASS text is "foobar41"
PASS text is "foobar42"
PASS text is "foobar43"
PASS text is "foobar44"
PASS text is "foobar45"
PASS text is "foobar46"
PASS text is "foobar47"
PASS text is "foobar48"
PASS text is "foobar49"
PASS text is "foobar50"
PASS text is "foobar51"
PASS text is "foobar52"
PASS text is "foobar53"
PASS text is "foobar54"
PASS text is "foobar55"
PASS text is "foobar56"
PASS text is "foobar57"
PASS text is "foobar58"
PASS text is "foobar59"
PASS text is "foobar60"
PASS text is "foobar61"
PASS text is "foobar62"
PASS text is "foobar63"
PASS text is "foobar64"
PASS text is "foobar65"
PASS text is "foobar66"
PASS text is "foobar67"
PASS text is "foobar68"
PASS text is "foobar69"
PASS text is "foobar70"
PASS text is "foobar71"
PASS text is "foobar72"
PASS text is "foobar73"
PASS text is "foobar74"
PASS text is "foobar75"
PASS text is "foobar76"
PASS text is "foobar77"
PASS text is "foobar78"
PASS text is "foobar79"
PASS text is "foobar80"
PASS text is "foobar81"
PASS text is "foobar82"
PASS text is "foobar83"
PASS text is "foobar84"
PASS text is "foobar85"
PASS text is "foobar86"
PASS text is "foobar87"
PASS text is "foobar88"
PASS text is "foobar89"
PASS text is "foobar90"
PASS text is "foobar91"
PASS text is "foobar92"
PASS text is "foobar93"
PASS text is "foobar94"
PASS text is "foobar95"
PASS text is "foobar96"
PASS text is "foobar97"
PASS text is "foobar98"
PASS text is "foobar99"
PASS successfullyParsed is true

TEST COMPLETE

29 changes: 29 additions & 0 deletions LayoutTests/fast/files/blob-text-gc.html
View file
@@ -0,0 +1,29 @@
<!DOCTYPE html>
<html>
<body>
<script src="../../resources/js-test.js"></script>
<script>
description("Make sure the Blob.text() promise gets resolved even if the Blob object is not kept alive by JS.");
jsTestIsAsync = true;

let promisesResolved = 0;
const testCount = 100;

function test(i)
{
new Blob(["foo", "bar", i]).text().then((_text) => {
text = _text;
shouldBeEqualToString("text", "foobar" + i);
promisesResolved++;
if (promisesResolved == testCount)
finishJSTest();
});
gc();
}

for (let i = 0; i < testCount; i++)
test(i);

</script>
</body>
</html>
132 changes: 132 additions & 0 deletions Source/WebCore/ChangeLog
View file
@@ -1,3 +1,135 @@
2020-08-26 Chris Dumez <cdumez@apple.com>

REGRESSION (r265908): Crash under Blob::arrayBuffer() / Blob::text() in stress GC
https://bugs.webkit.org/show_bug.cgi?id=215832
<rdar://problem/67741677>

Reviewed by Ryosuke Niwa.

r265908 added support for Blob::arrayBuffer() / Blob::text() which are asynchronous operations
returning promises. The crash is due to the fact that the Blob's JS wrapper may get garbage
collected before the promise is settled.

To address the issue, this patch makes Blob an ActiveDOMObject and creates an
ActiveDOMObject::pendingActivity whenever there is a pending promise so that the JS wrapper
does not get garbage collected too early.

Test: fast/files/blob-text-gc.html

* Modules/async-clipboard/Clipboard.cpp:
(WebCore::Clipboard::getType):
* Modules/async-clipboard/ClipboardImageReader.h:
(WebCore::ClipboardImageReader::ClipboardImageReader):
* Modules/async-clipboard/ClipboardItem.cpp:
(WebCore::ClipboardItem::blobFromString):
* Modules/async-clipboard/ClipboardItem.h:
* Modules/async-clipboard/ClipboardItemBindingsDataSource.cpp:
(WebCore::ClipboardItemBindingsDataSource::getType):
* Modules/async-clipboard/ios/ClipboardImageReaderIOS.mm:
(WebCore::ClipboardImageReader::readBuffer):
* Modules/async-clipboard/mac/ClipboardImageReaderMac.mm:
(WebCore::ClipboardImageReader::readBuffer):
* Modules/entriesapi/DOMFileSystem.cpp:
(WebCore::DOMFileSystem::getFile):
* Modules/entriesapi/DOMFileSystem.h:
* Modules/entriesapi/FileSystemFileEntry.cpp:
(WebCore::FileSystemFileEntry::file):
* Modules/entriesapi/FileSystemFileEntry.h:
* Modules/entriesapi/FileSystemFileEntry.idl:
* Modules/fetch/FetchBody.cpp:
(WebCore::FetchBody::fromFormData):
* Modules/fetch/FetchBody.h:
* Modules/fetch/FetchBodyConsumer.cpp:
(WebCore::blobFromData):
(WebCore::packageFormData):
(WebCore::resolveWithTypeAndData):
(WebCore::FetchBodyConsumer::resolve):
(WebCore::FetchBodyConsumer::takeAsBlob):
* Modules/fetch/FetchBodyConsumer.h:
* Modules/fetch/FetchBodyOwner.cpp:
(WebCore::FetchBodyOwner::blob):
* Modules/mediarecorder/MediaRecorder.cpp:
(WebCore::MediaRecorder::stopRecording):
(WebCore::MediaRecorder::requestData):
* Modules/mediastream/RTCDataChannel.cpp:
(WebCore::RTCDataChannel::didReceiveRawData):
* Modules/websockets/WebSocket.cpp:
(WebCore::WebSocket::didReceiveBinaryData):
* Modules/websockets/WorkerThreadableWebSocketChannel.cpp:
(WebCore::WorkerThreadableWebSocketChannel::Bridge::send):
* bindings/js/SerializedScriptValue.cpp:
(WebCore::CloneDeserializer::readFile):
(WebCore::CloneDeserializer::readTerminal):
* dom/DataTransfer.cpp:
(WebCore::DataTransfer::updateFileList):
(WebCore::DataTransfer::items):
(WebCore::DataTransfer::filesFromPasteboardAndItemList const):
(WebCore::DataTransfer::files const):
* dom/DataTransfer.h:
* dom/DataTransfer.idl:
* dom/DataTransferItemList.cpp:
(WebCore::DataTransferItemList::DataTransferItemList):
(WebCore::DataTransferItemList::remove):
(WebCore::DataTransferItemList::clear):
(WebCore::DataTransferItemList::ensureItems const):
(WebCore::DataTransferItemList::document const):
* dom/DataTransferItemList.h:
* dom/DataTransferItemList.idl:
* editing/WebCorePasteboardFileReader.cpp:
(WebCore::WebCorePasteboardFileReader::readFilename):
(WebCore::WebCorePasteboardFileReader::readBuffer):
* editing/WebCorePasteboardFileReader.h:
* editing/cocoa/WebContentReaderCocoa.mm:
(WebCore::createFragmentForImageAttachment):
(WebCore::replaceRichContentWithAttachments):
(WebCore::createFragmentAndAddResources):
(WebCore::sanitizeMarkupWithArchive):
(WebCore::WebContentReader::readImage):
(WebCore::attachmentForFilePath):
(WebCore::attachmentForData):
* editing/markup.cpp:
(WebCore::restoreAttachmentElementsInFragment):
* fileapi/Blob.cpp:
(WebCore::Blob::Blob):
(WebCore::Blob::loadBlob):
(WebCore::Blob::activeDOMObjectName const):
* fileapi/Blob.h:
(WebCore::Blob::create):
(WebCore::Blob::deserialize):
(WebCore::Blob::slice const):
* fileapi/Blob.idl:
* fileapi/File.cpp:
(WebCore::File::createWithRelativePath):
(WebCore::File::create):
(WebCore::File::File):
(WebCore::File::activeDOMObjectName const):
* fileapi/File.h:
* fileapi/File.idl:
* html/DOMFormData.cpp:
(WebCore::DOMFormData::createFileEntry):
* html/DirectoryFileListCreator.cpp:
(WebCore::FileInformation::isolatedCopy const):
(WebCore::appendDirectoryFiles):
(WebCore::gatherFileInformation):
(WebCore::toFileList):
(WebCore::DirectoryFileListCreator::start):
* html/DirectoryFileListCreator.h:
* html/FileInputType.cpp:
(WebCore::FileInputType::appendFormData const):
(WebCore::FileInputType::filesChosen):
* html/HTMLAttachmentElement.cpp:
(WebCore::HTMLAttachmentElement::updateEnclosingImageWithData):
* html/HTMLCanvasElement.cpp:
(WebCore::HTMLCanvasElement::toBlob):
* testing/Internals.cpp:
(WebCore::Internals::createFile):
* testing/ServiceWorkerInternals.cpp:
(WebCore::ServiceWorkerInternals::createOpaqueWithBlobBodyResponse):
* workers/service/context/ServiceWorkerFetch.cpp:
(WebCore::ServiceWorkerFetch::dispatchFetchEvent):
* xml/XMLHttpRequest.cpp:
(WebCore::XMLHttpRequest::createResponseBlob):

2020-08-26 Andres Gonzalez <andresg_22@apple.com>

When caching the AccessibilityText property in AXIsolatedObject, the Strings need to be isolatedCopied.
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/Modules/async-clipboard/Clipboard.cpp
View file
Expand Up @@ -217,7 +217,7 @@ void Clipboard::getType(ClipboardItem& item, const String& type, Ref<DeferredPro
}

if (type == "image/png"_s) {
ClipboardImageReader imageReader { type };
ClipboardImageReader imageReader { frame->document(), type };
activePasteboard().read(imageReader, itemIndex);
auto imageBlob = imageReader.takeResult();
if (updateSessionValidity() == SessionIsValid::Yes && imageBlob)
Expand Down Expand Up @@ -252,7 +252,7 @@ void Clipboard::getType(ClipboardItem& item, const String& type, Ref<DeferredPro
return;
}

promise->resolve<IDLInterface<Blob>>(ClipboardItem::blobFromString(resultAsString, type));
promise->resolve<IDLInterface<Blob>>(ClipboardItem::blobFromString(frame->document(), resultAsString, type));
}

Clipboard::SessionIsValid Clipboard::updateSessionValidity()
Expand Down
4 changes: 3 additions & 1 deletion Source/WebCore/Modules/async-clipboard/ClipboardImageReader.h
View file
Expand Up @@ -33,8 +33,9 @@ namespace WebCore {
class SharedBuffer;

struct ClipboardImageReader : PasteboardFileReader {
ClipboardImageReader(const String& mimeType)
ClipboardImageReader(Document* document, const String& mimeType)
: PasteboardFileReader()
, m_document(document)
, m_mimeType(mimeType)
{
}
Expand All @@ -47,6 +48,7 @@ struct ClipboardImageReader : PasteboardFileReader {
bool shouldReadBuffer(const String&) const final;
void readBuffer(const String& filename, const String& type, Ref<SharedBuffer>&&) final;

RefPtr<Document> m_document;
String m_mimeType;
RefPtr<Blob> m_result;
};
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/Modules/async-clipboard/ClipboardItem.cpp
View file
Expand Up @@ -39,10 +39,10 @@ namespace WebCore {

ClipboardItem::~ClipboardItem() = default;

Ref<Blob> ClipboardItem::blobFromString(const String& stringData, const String& type)
Ref<Blob> ClipboardItem::blobFromString(ScriptExecutionContext* context, const String& stringData, const String& type)
{
auto utf8 = stringData.utf8();
return Blob::create(SharedBuffer::create(utf8.data(), utf8.length()), Blob::normalizedContentType(type));
return Blob::create(context, SharedBuffer::create(utf8.data(), utf8.length()), Blob::normalizedContentType(type));
}

static ClipboardItem::PresentationStyle clipboardItemPresentationStyle(const PasteboardItemInfo& info)
Expand Down
3 changes: 2 additions & 1 deletion Source/WebCore/Modules/async-clipboard/ClipboardItem.h
View file
Expand Up @@ -41,6 +41,7 @@ class DeferredPromise;
class DOMPromise;
class Navigator;
class PasteboardCustomData;
class ScriptExecutionContext;
struct PasteboardItemInfo;

class ClipboardItem : public RefCounted<ClipboardItem> {
Expand All @@ -55,7 +56,7 @@ class ClipboardItem : public RefCounted<ClipboardItem> {

static Ref<ClipboardItem> create(Vector<KeyValuePair<String, RefPtr<DOMPromise>>>&&, const Options&);
static Ref<ClipboardItem> create(Clipboard&, const PasteboardItemInfo&);
static Ref<Blob> blobFromString(const String& stringData, const String& type);
static Ref<Blob> blobFromString(ScriptExecutionContext*, const String& stringData, const String& type);

Vector<String> types() const;
void getType(const String&, Ref<DeferredPromise>&&);
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/Modules/async-clipboard/ClipboardItemBindingsDataSource.cpp
View file
Expand Up @@ -107,7 +107,7 @@ void ClipboardItemBindingsDataSource::getType(const String& type, Ref<DeferredPr
String string;
result.getString(itemPromise->globalObject(), string);
if (!string.isNull()) {
promise->resolve<IDLInterface<Blob>>(ClipboardItem::blobFromString(string, type));
promise->resolve<IDLInterface<Blob>>(ClipboardItem::blobFromString(promise->scriptExecutionContext(), string, type));
return;
}

Expand Down
3 changes: 2 additions & 1 deletion Source/WebCore/Modules/async-clipboard/ios/ClipboardImageReaderIOS.mm
View file
Expand Up @@ -28,6 +28,7 @@

#if PLATFORM(IOS_FAMILY)

#import "Document.h"
#import "SharedBuffer.h"
#import <pal/ios/UIKitSoftLink.h>

Expand All @@ -38,7 +39,7 @@
if (m_mimeType == "image/png") {
auto image = adoptNS([PAL::allocUIImageInstance() initWithData:buffer->createNSData().get()]);
if (auto nsData = UIImagePNGRepresentation(image.get()))
m_result = Blob::create(SharedBuffer::create(nsData), m_mimeType);
m_result = Blob::create(m_document.get(), SharedBuffer::create(nsData), m_mimeType);
}
}

Expand Down

0 comments on commit 6c68ec8

Please sign in to comment.