+2011-09-27 Eric Uhrhane <ericu@chromium.org>
+
+ [Chromium/FileWriter] race condition in FileWriter completion can lead to assert
+ https://bugs.webkit.org/show_bug.cgi?id=67684
+
+ Reviewed by David Levin.
+
+ * fast/filesystem/file-writer-abort-continue-expected.txt: Added.
+ * fast/filesystem/file-writer-abort-continue.html: Added.
+ * fast/filesystem/file-writer-abort-expected.txt: Added.
+ * fast/filesystem/file-writer-abort.html: Added.
+ * fast/filesystem/resources/file-writer-abort-continue.js: Added.
+ * fast/filesystem/resources/file-writer-abort.js: Added.
+ * fast/filesystem/resources/file-writer-events.js: Fixed a copy-paste error.
+
2011-09-27 Tim Horton <timothy_horton@apple.com>
svg/custom/oversized-pattern-scale.svg is useless because the interesting part of the test is off the screen
--- /dev/null
+Test that FileWriter can continue immediately after an abort.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+starting test
+PASS 1100000 is blobSize
+PASS Calling abort
+PASS Saw abort
+PASS Saw writeend 0.
+PASS writer.length is 0
+PASS followOnAction == 1 || followOnAction == 3 is true
+PASS 1100000 is blobSize
+PASS writer.length is 1100000
+PASS Saw writeend 1.
+PASS 1100000 is blobSize
+PASS Calling abort
+PASS Saw abort
+PASS Saw writeend 0.
+PASS writer.length is 1100000
+PASS followOnAction == 1 || followOnAction == 3 is true
+PASS writer.length is 7
+PASS All tests complete.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
--- /dev/null
+<!DOCTYPE HTML>
+<html>
+ <head>
+ <link rel="stylesheet" href="../js/resources/js-test-style.css">
+ <title>File Writer Abort and Continue</title>
+ <script src="../js/resources/js-test-pre.js"></script>
+ <script src="resources/file-writer-utils.js"></script>
+ </head>
+ <body>
+ <div id="description"></div>
+ <div id="console"></div>
+ <script src="resources/file-writer-abort-continue.js"></script>
+ <script src="../js/resources/js-test-post.js"></script>
+ </body>
+</html>
+
--- /dev/null
+Test that FileWriter defends against infinite recursion via abort.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+starting test
+PASS Calling write.
+PASS Calling abort
+PASS Saw abort
+PASS Calling write.
+PASS Calling abort
+PASS Saw abort
+PASS Calling write.
+PASS Saw security error
+PASS Saw writeend.
+PASS Saw writeend.
+PASS Calling truncate.
+PASS Calling abort
+PASS Saw abort
+PASS Calling truncate.
+PASS Calling abort
+PASS Saw abort
+PASS Calling truncate.
+PASS Saw security error
+PASS Saw writeend.
+PASS Saw writeend.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
--- /dev/null
+<!DOCTYPE HTML>
+<html>
+ <head>
+ <link rel="stylesheet" href="../js/resources/js-test-style.css">
+ <title>File Writer Abort Depth</title>
+ <script src="../js/resources/js-test-pre.js"></script>
+ <script src="resources/file-writer-utils.js"></script>
+ </head>
+ <body>
+ <div id="description"></div>
+ <div id="console"></div>
+ <script src="resources/file-writer-abort-depth.js"></script>
+ <script src="../js/resources/js-test-post.js"></script>
+ </body>
+</html>
+
--- /dev/null
+Test that FileWriter handles abort properly.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+starting test
+PASS Calling abort
+PASS Saw abort
+PASS Saw writeend.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
--- /dev/null
+<!DOCTYPE HTML>
+<html>
+ <head>
+ <link rel="stylesheet" href="../js/resources/js-test-style.css">
+ <title>File Writer Abort</title>
+ <script src="../js/resources/js-test-pre.js"></script>
+ <script src="resources/file-writer-utils.js"></script>
+ </head>
+ <body>
+ <div id="description"></div>
+ <div id="console"></div>
+ <script src="resources/file-writer-abort.js"></script>
+ <script src="../js/resources/js-test-post.js"></script>
+ </body>
+</html>
+
--- /dev/null
+if (this.importScripts) {
+ importScripts('fs-worker-common.js');
+ importScripts('file-writer-utils.js');
+}
+
+description("Test that FileWriter can continue immediately after an abort.");
+
+var sawWriteStart;
+var sawAbort;
+var sawWriteEnd;
+var writer;
+var expectedLength;
+var truncateLength = 7;
+var blobSize = 1100000;
+
+var methodSet = [
+ { // Setup method set that writes, then aborts that write before completion.
+ action : startWrite,
+ verifyLength : 0,
+ onwritestart : abortWrite,
+ onwrite : onError,
+ onabort : logAbort,
+ onwriteend : checkLengthAndCallFollowOnAction
+ },
+ { // Method set that does a complete write.
+ action : startWrite,
+ verifyLength : blobSize,
+ onwritestart : nop,
+ onwrite : nop,
+ onabort : onError,
+ onwriteend : checkLengthAndStartNextTest
+ },
+ { // Setup method set that writes, then aborts that write before completion.
+ action : startWrite,
+ verifyLength : blobSize, // Left over from the previous test.
+ onwritestart : abortWrite,
+ onwrite : onError,
+ onabort : logAbort,
+ onwriteend : checkLengthAndCallFollowOnAction
+ },
+ { // Method set that does a complete truncate.
+ action : startTruncate,
+ verifyLength : truncateLength,
+ onwritestart : nop,
+ onwrite : nop,
+ onabort : onError,
+ onwriteend : checkLengthAndCompleteTest
+ }];
+
+function nop() {
+}
+
+function tenXBlob(blob) {
+ var bb = new WebKitBlobBuilder();
+ for (var i = 0; i < 10; ++i) {
+ bb.append(blob);
+ }
+ return bb.getBlob();
+}
+
+// These methods set up a write, abort it as soon as it starts, then initiate
+// the follow on action.
+function abortWrite(e) {
+ testPassed("Calling abort");
+ writer.abort();
+}
+
+function logAbort(e) {
+ testPassed("Saw abort");
+}
+
+function checkLengthAndCallFollowOnAction(e) {
+ testPassed("Saw writeend 0.");
+ shouldBe("writer.length", "" + expectedLength);
+ doFollowOnAction();
+}
+
+// For the second method set, verify completion and move on to the next test.
+function checkLengthAndStartNextTest(e) {
+ shouldBe("writer.length", "" + expectedLength);
+ testPassed("Saw writeend 1.");
+ runTest(2, 3);
+}
+
+function checkLengthAndCompleteTest(e) {
+ shouldBe("writer.length", "" + expectedLength);
+ testPassed("All tests complete.");
+ cleanUp();
+}
+
+function startWrite() {
+ // Let's make it about a megabyte.
+ var bb = new WebKitBlobBuilder();
+ bb.append("lorem ipsum");
+ var blob = tenXBlob(bb.getBlob());
+ blob = tenXBlob(blob);
+ blob = tenXBlob(blob);
+ blob = tenXBlob(blob);
+ blob = tenXBlob(blob);
+ var size = blob.size;
+ shouldBe("" + size, "blobSize");
+ writer.write(blob);
+}
+
+function startTruncate() {
+ writer.truncate(truncateLength);
+}
+
+function setupWriter(methodSetIndex, writer) {
+ writer.onerror = onError;
+
+ var methods = methodSet[methodSetIndex];
+ writer.onabort = methods.onabort;
+ writer.onwritestart = methods.onwritestart;
+ writer.onwrite = methods.onwrite;
+ writer.onwriteend = methods.onwriteend;
+ expectedLength = methods.verifyLength;
+ methods.action();
+}
+
+function runTest(initIndex, testIndex) {
+ followOnAction = testIndex;
+ setupWriter(initIndex, writer);
+}
+
+function doFollowOnAction() {
+ shouldBeTrue("followOnAction == 1 || followOnAction == 3");
+ setupWriter(followOnAction, writer);
+}
+
+var jsTestIsAsync = true;
+setupAndRunTest(2*1024*1024, 'file-writer-abort',
+ function (fileEntry, fileWriter) {
+ fileEntryForCleanup = fileEntry;
+ writer = fileWriter;
+ runTest(0, 1);
+ });
+var successfullyParsed = true;
--- /dev/null
+if (this.importScripts) {
+ importScripts('fs-worker-common.js');
+ importScripts('file-writer-utils.js');
+}
+
+description("Test that FileWriter defends against infinite recursion via abort.");
+
+var sawWriteStart;
+var sawAbort;
+var sawWriteEnd;
+var writer;
+var bb = new WebKitBlobBuilder();
+bb.append("lorem ipsum");
+var blob = bb.getBlob();
+var recursionDepth = 0;
+var method;
+var testsRun = 0;
+
+function onWriteStart(e) {
+ testPassed("Calling abort");
+ ++recursionDepth;
+ writer.abort();
+}
+
+// We should always abort before completion.
+function onWrite(e) {
+ testFailed("In onWrite.");
+}
+
+function onAbort(e) {
+ testPassed("Saw abort");
+ try {
+ method();
+ } catch (ex) {
+ assert(ex.code == 2); // Security error
+ testPassed("Saw security error");
+ }
+}
+
+function onWriteEnd(e) {
+ --recursionDepth;
+ testPassed("Saw writeend.");
+ if (!recursionDepth) {
+ ++testsRun;
+ if (testsRun == 1) {
+ method = function() {
+ testPassed("Calling truncate.");
+ writer.truncate(7);
+ }
+ setTimeout(method, 0); // Invoke from the top level, so that we're not already in a handler.
+ } else {
+ cleanUp();
+ }
+ }
+}
+
+function runTest(unusedFileEntry, fileWriter) {
+ writer = fileWriter;
+ method = function () {
+ testPassed("Calling write.");
+ writer.write(blob);
+ }
+ fileWriter.onerror = onError;
+ fileWriter.onabort = onAbort;
+ fileWriter.onwritestart = onWriteStart;
+ fileWriter.onwrite = onWrite;
+ fileWriter.onwriteend = onWriteEnd;
+ method();
+}
+
+var jsTestIsAsync = true;
+setupAndRunTest(2*1024*1024, 'file-writer-abort-depth', runTest);
+var successfullyParsed = true;
--- /dev/null
+if (this.importScripts) {
+ importScripts('fs-worker-common.js');
+ importScripts('file-writer-utils.js');
+}
+
+description("Test that FileWriter handles abort properly.");
+
+var sawWriteStart;
+var sawAbort;
+var sawWriteEnd;
+var writer;
+
+function tenXBlob(blob) {
+ var bb = new WebKitBlobBuilder();
+ for (var i = 0; i < 10; ++i) {
+ bb.append(blob);
+ }
+ return bb.getBlob();
+}
+
+function onWriteStart(e) {
+ assert(writer);
+ assert(writer.readyState == writer.WRITING);
+ assert(e.type == "writestart");
+ assert(!sawWriteStart);
+ assert(!sawWriteEnd);
+ assert(!e.loaded);
+ sawWriteStart = true;
+ testPassed("Calling abort");
+ writer.abort();
+}
+
+// We should always abort before completion.
+function onWrite(e) {
+ testFailed("In onWrite.");
+}
+
+function onAbort(e) {
+ assert(writer.readyState == writer.DONE);
+ assert(writer.error.code == writer.error.ABORT_ERR);
+ assert(sawWriteStart);
+ assert(!sawWriteEnd);
+ assert(!sawAbort);
+ assert(e.type == "abort");
+ sawAbort = true;
+ testPassed("Saw abort");
+}
+
+function onWriteEnd(e) {
+ assert(writer.readyState == writer.DONE);
+ assert(writer.error.code == writer.error.ABORT_ERR);
+ assert(sawWriteStart);
+ assert(sawAbort);
+ assert(!sawWriteEnd);
+ assert(e.type == "writeend");
+ sawWriteEnd = true;
+ testPassed("Saw writeend.");
+ writer.abort(); // Verify that this does nothing in readyState DONE.
+ cleanUp();
+}
+
+function startWrite(fileWriter) {
+ // Let's make it about a megabyte.
+ var bb = new WebKitBlobBuilder();
+ bb.append("lorem ipsum");
+ var blob = tenXBlob(bb.getBlob());
+ blob = tenXBlob(blob);
+ blob = tenXBlob(blob);
+ blob = tenXBlob(blob);
+ blob = tenXBlob(blob);
+ writer = fileWriter;
+ fileWriter.onerror = onError;
+ fileWriter.onabort = onAbort;
+ fileWriter.onwritestart = onWriteStart;
+ fileWriter.onwrite = onWrite;
+ fileWriter.onwriteend = onWriteEnd;
+ fileWriter.abort(); // Verify that this does nothing in readyState INIT.
+ fileWriter.write(blob);
+}
+
+function runTest(unusedFileEntry, fileWriter) {
+ startWrite(fileWriter);
+}
+var jsTestIsAsync = true;
+setupAndRunTest(2*1024*1024, 'file-writer-abort', runTest);
+var successfullyParsed = true;
startWrite(fileWriter);
}
var jsTestIsAsync = true;
-setupAndRunTest(2*1024*1024, 'file-writer-gc-blob', runTest);
+setupAndRunTest(2*1024*1024, 'file-writer-events', runTest);
var successfullyParsed = true;
+2011-09-27 Eric Uhrhane <ericu@chromium.org>
+
+ [Chromium/FileWriter] race condition in FileWriter completion can lead to assert
+ https://bugs.webkit.org/show_bug.cgi?id=67684
+
+ Reviewed by David Levin.
+
+ Tests: fast/filesystem/file-writer-abort-continue.html
+ fast/filesystem/file-writer-abort.html
+
+ Track the state of the backend and be prepared for reentrant user
+ requests. Limit recursion depth to an arbitrary small constant.
+ * fileapi/FileWriter.cpp: Lots of event-handling changes.
+ * fileapi/FileWriter.h:
+
2011-09-27 Mihai Parparita <mihaip@chromium.org>
Unreviewed, rolling out r96141.
namespace WebCore {
+static const int kMaxRecursionDepth = 3;
+
FileWriter::FileWriter(ScriptExecutionContext* context)
: ActiveDOMObject(context, this)
, m_readyState(INIT)
- , m_startedWriting(false)
+ , m_isOperationInProgress(false)
+ , m_queuedOperation(OperationNone)
, m_bytesWritten(0)
, m_bytesToWrite(0)
, m_truncateLength(-1)
+ , m_numAborts(0)
+ , m_recursionDepth(0)
{
}
FileWriter::~FileWriter()
{
+ ASSERT(!m_recursionDepth);
if (m_readyState == WRITING)
stop();
}
void FileWriter::stop()
{
- if (writer() && m_readyState == WRITING)
+ // Make sure we've actually got something to stop, and haven't already called abort().
+ if (writer() && m_readyState == WRITING && m_isOperationInProgress && m_queuedOperation == OperationNone)
writer()->abort();
m_blobBeingWritten.clear();
m_readyState = DONE;
void FileWriter::write(Blob* data, ExceptionCode& ec)
{
ASSERT(writer());
+ ASSERT(data);
+ ASSERT(m_truncateLength == -1);
if (m_readyState == WRITING) {
setError(FileError::INVALID_STATE_ERR, ec);
return;
setError(FileError::TYPE_MISMATCH_ERR, ec);
return;
}
-
+ if (m_recursionDepth > kMaxRecursionDepth) {
+ setError(FileError::SECURITY_ERR, ec);
+ return;
+ }
m_blobBeingWritten = data;
m_readyState = WRITING;
- m_startedWriting = false;
m_bytesWritten = 0;
m_bytesToWrite = data->size();
- writer()->write(position(), data);
+ ASSERT(m_queuedOperation == OperationNone);
+ if (m_isOperationInProgress) // We must be waiting for an abort to complete, since m_readyState wasn't WRITING.
+ m_queuedOperation = OperationWrite;
+ else
+ writer()->write(position(), m_blobBeingWritten.get());
+ m_isOperationInProgress = true;
+ fireEvent(eventNames().writestartEvent);
}
void FileWriter::seek(long long position, ExceptionCode& ec)
return;
}
- m_bytesWritten = 0;
- m_bytesToWrite = 0;
+ ASSERT(m_truncateLength == -1);
+ ASSERT(m_queuedOperation == OperationNone);
seekInternal(position);
}
void FileWriter::truncate(long long position, ExceptionCode& ec)
{
ASSERT(writer());
+ ASSERT(m_truncateLength == -1);
if (m_readyState == WRITING || position < 0) {
setError(FileError::INVALID_STATE_ERR, ec);
return;
}
+ if (m_recursionDepth > kMaxRecursionDepth) {
+ setError(FileError::SECURITY_ERR, ec);
+ return;
+ }
m_readyState = WRITING;
m_bytesWritten = 0;
m_bytesToWrite = 0;
m_truncateLength = position;
- writer()->truncate(position);
+ ASSERT(m_queuedOperation == OperationNone);
+ if (m_isOperationInProgress) // We must be waiting for an abort to complete.
+ m_queuedOperation = OperationTruncate;
+ else
+ writer()->truncate(m_truncateLength);
+ m_isOperationInProgress = true;
+ fireEvent(eventNames().writestartEvent);
}
void FileWriter::abort(ExceptionCode& ec)
{
ASSERT(writer());
if (m_readyState != WRITING) {
- setError(FileError::INVALID_STATE_ERR, ec);
return;
}
+ ++m_numAborts;
- writer()->abort();
+ if (m_isOperationInProgress)
+ writer()->abort();
+ m_queuedOperation = OperationNone;
+ m_blobBeingWritten.clear();
+ m_truncateLength = -1;
+ signalCompletion(FileError::ABORT_ERR);
}
void FileWriter::didWrite(long long bytes, bool complete)
{
+ ASSERT(m_readyState == WRITING);
+ ASSERT(m_truncateLength == -1);
+ ASSERT(m_isOperationInProgress);
ASSERT(bytes + m_bytesWritten > 0);
ASSERT(bytes + m_bytesWritten <= m_bytesToWrite);
- if (!m_startedWriting) {
- fireEvent(eventNames().writestartEvent);
- m_startedWriting = true;
- }
m_bytesWritten += bytes;
ASSERT((m_bytesWritten == m_bytesToWrite) || !complete);
setPosition(position() + bytes);
if (position() > length())
setLength(position());
+ // TODO: Throttle to no more frequently than every 50ms.
+ int numAborts = m_numAborts;
fireEvent(eventNames().progressEvent);
- if (complete) {
+ // We could get an abort in the handler for this event. If we do, it's
+ // already handled the cleanup and signalCompletion call.
+ if (complete && numAborts == m_numAborts) {
m_blobBeingWritten.clear();
- scriptExecutionContext()->postTask(FileWriterCompletionEventTask::create(this, FileError::OK));
+ m_isOperationInProgress = false;
+ signalCompletion(FileError::OK);
}
}
void FileWriter::didTruncate()
{
ASSERT(m_truncateLength >= 0);
- fireEvent(eventNames().writestartEvent);
setLength(m_truncateLength);
if (position() > length())
setPosition(length());
- m_truncateLength = -1;
- scriptExecutionContext()->postTask(FileWriterCompletionEventTask::create(this, FileError::OK));
+ m_isOperationInProgress = false;
+ signalCompletion(FileError::OK);
}
void FileWriter::didFail(FileError::ErrorCode code)
{
+ ASSERT(m_isOperationInProgress);
+ m_isOperationInProgress = false;
ASSERT(code != FileError::OK);
- m_blobBeingWritten.clear();
- scriptExecutionContext()->postTask(FileWriterCompletionEventTask::create(this, code));
+ if (code == FileError::ABORT_ERR) {
+ Operation operation = m_queuedOperation;
+ m_queuedOperation = OperationNone;
+ doOperation(operation);
+ } else {
+ ASSERT(m_queuedOperation == OperationNone);
+ ASSERT(m_readyState == WRITING);
+ m_blobBeingWritten.clear();
+ signalCompletion(code);
+ }
+}
+
+void FileWriter::doOperation(Operation operation)
+{
+ ASSERT(m_queuedOperation == OperationNone);
+ ASSERT(!m_isOperationInProgress);
+ ASSERT(operation == OperationNone || operation == OperationWrite || operation == OperationTruncate);
+ switch (operation) {
+ case OperationWrite:
+ ASSERT(m_truncateLength == -1);
+ ASSERT(m_blobBeingWritten.get());
+ ASSERT(m_readyState == WRITING);
+ writer()->write(position(), m_blobBeingWritten.get());
+ m_isOperationInProgress = true;
+ break;
+ case OperationTruncate:
+ ASSERT(m_truncateLength >= 0);
+ ASSERT(m_readyState == WRITING);
+ writer()->truncate(m_truncateLength);
+ m_isOperationInProgress = true;
+ break;
+ case OperationNone:
+ ASSERT(m_truncateLength == -1);
+ ASSERT(!m_blobBeingWritten.get());
+ ASSERT(m_readyState == DONE);
+ break;
+ }
}
void FileWriter::signalCompletion(FileError::ErrorCode code)
{
m_readyState = DONE;
+ m_truncateLength = -1;
if (FileError::OK != code) {
m_error = FileError::create(code);
- fireEvent(eventNames().errorEvent);
if (FileError::ABORT_ERR == code)
fireEvent(eventNames().abortEvent);
+ else
+ fireEvent(eventNames().errorEvent);
} else
fireEvent(eventNames().writeEvent);
fireEvent(eventNames().writeendEvent);
void FileWriter::fireEvent(const AtomicString& type)
{
+ ++m_recursionDepth;
dispatchEvent(ProgressEvent::create(type, true, m_bytesWritten, m_bytesToWrite));
+ --m_recursionDepth;
+ ASSERT(m_recursionDepth >= 0);
}
void FileWriter::setError(FileError::ErrorCode errorCode, ExceptionCode& ec)
DEFINE_ATTRIBUTE_EVENT_LISTENER(writeend);
private:
+ enum Operation {
+ OperationNone,
+ OperationWrite,
+ OperationTruncate
+ };
+
FileWriter(ScriptExecutionContext*);
virtual ~FileWriter();
virtual EventTargetData* eventTargetData() { return &m_eventTargetData; }
virtual EventTargetData* ensureEventTargetData() { return &m_eventTargetData; }
- void fireEvent(const AtomicString& type);
-
- void setError(FileError::ErrorCode, ExceptionCode&);
+ void doOperation(Operation);
void signalCompletion(FileError::ErrorCode);
- class FileWriterCompletionEventTask : public ScriptExecutionContext::Task {
- public:
- static PassOwnPtr<FileWriterCompletionEventTask> create(PassRefPtr<FileWriter> fileWriter, FileError::ErrorCode code)
- {
- return adoptPtr(new FileWriterCompletionEventTask(fileWriter, code));
- }
-
-
- virtual void performTask(ScriptExecutionContext*)
- {
- m_fileWriter->signalCompletion(m_code);
- }
- private:
- FileWriterCompletionEventTask(PassRefPtr<FileWriter> fileWriter, FileError::ErrorCode code)
- : m_fileWriter(fileWriter)
- , m_code(code)
- {
- }
-
- RefPtr<FileWriter> m_fileWriter;
- FileError::ErrorCode m_code;
- };
+ void fireEvent(const AtomicString& type);
+
+ void setError(FileError::ErrorCode, ExceptionCode&);
RefPtr<FileError> m_error;
EventTargetData m_eventTargetData;
ReadyState m_readyState;
- bool m_startedWriting;
+ bool m_isOperationInProgress;
+ Operation m_queuedOperation;
long long m_bytesWritten;
long long m_bytesToWrite;
long long m_truncateLength;
+ long long m_numAborts;
+ long long m_recursionDepth;
RefPtr<Blob> m_blobBeingWritten;
};
projects / profile / ivi / webkit-efl.git / commitdiff