Title: [96177] trunk
Revision
96177
Author
[email protected]
Date
2011-09-27 18:46:08 -0700 (Tue, 27 Sep 2011)

Log Message

[Chromium/FileWriter] race condition in FileWriter completion can lead to assert
https://bugs.webkit.org/show_bug.cgi?id=67684

Reviewed by David Levin.

Source/WebCore:

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:

LayoutTests:

* 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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (96176 => 96177)


--- trunk/LayoutTests/ChangeLog	2011-09-28 01:12:10 UTC (rev 96176)
+++ trunk/LayoutTests/ChangeLog	2011-09-28 01:46:08 UTC (rev 96177)
@@ -1,3 +1,18 @@
+2011-09-27  Eric Uhrhane  <[email protected]>
+
+        [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  <[email protected]>
 
         svg/custom/oversized-pattern-scale.svg is useless because the interesting part of the test is off the screen

Added: trunk/LayoutTests/fast/filesystem/file-writer-abort-continue-expected.txt (0 => 96177)


--- trunk/LayoutTests/fast/filesystem/file-writer-abort-continue-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/filesystem/file-writer-abort-continue-expected.txt	2011-09-28 01:46:08 UTC (rev 96177)
@@ -0,0 +1,26 @@
+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
+

Added: trunk/LayoutTests/fast/filesystem/file-writer-abort-continue.html (0 => 96177)


--- trunk/LayoutTests/fast/filesystem/file-writer-abort-continue.html	                        (rev 0)
+++ trunk/LayoutTests/fast/filesystem/file-writer-abort-continue.html	2011-09-28 01:46:08 UTC (rev 96177)
@@ -0,0 +1,16 @@
+<!DOCTYPE HTML>
+<html>
+ <head>
+    <link rel="stylesheet" href=""
+    <title>File Writer Abort and Continue</title>
+    <script src=""
+    <script src=""
+ </head>
+ <body>
+    <div id="description"></div>
+    <div id="console"></div>
+    <script src=""
+    <script src=""
+ </body>
+</html>
+

Added: trunk/LayoutTests/fast/filesystem/file-writer-abort-depth-expected.txt (0 => 96177)


--- trunk/LayoutTests/fast/filesystem/file-writer-abort-depth-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/filesystem/file-writer-abort-depth-expected.txt	2011-09-28 01:46:08 UTC (rev 96177)
@@ -0,0 +1,29 @@
+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
+

Added: trunk/LayoutTests/fast/filesystem/file-writer-abort-depth.html (0 => 96177)


--- trunk/LayoutTests/fast/filesystem/file-writer-abort-depth.html	                        (rev 0)
+++ trunk/LayoutTests/fast/filesystem/file-writer-abort-depth.html	2011-09-28 01:46:08 UTC (rev 96177)
@@ -0,0 +1,16 @@
+<!DOCTYPE HTML>
+<html>
+ <head>
+    <link rel="stylesheet" href=""
+    <title>File Writer Abort Depth</title>
+    <script src=""
+    <script src=""
+ </head>
+ <body>
+    <div id="description"></div>
+    <div id="console"></div>
+    <script src=""
+    <script src=""
+ </body>
+</html>
+

Added: trunk/LayoutTests/fast/filesystem/file-writer-abort-expected.txt (0 => 96177)


--- trunk/LayoutTests/fast/filesystem/file-writer-abort-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/filesystem/file-writer-abort-expected.txt	2011-09-28 01:46:08 UTC (rev 96177)
@@ -0,0 +1,12 @@
+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
+

Added: trunk/LayoutTests/fast/filesystem/file-writer-abort.html (0 => 96177)


--- trunk/LayoutTests/fast/filesystem/file-writer-abort.html	                        (rev 0)
+++ trunk/LayoutTests/fast/filesystem/file-writer-abort.html	2011-09-28 01:46:08 UTC (rev 96177)
@@ -0,0 +1,16 @@
+<!DOCTYPE HTML>
+<html>
+ <head>
+    <link rel="stylesheet" href=""
+    <title>File Writer Abort</title>
+    <script src=""
+    <script src=""
+ </head>
+ <body>
+    <div id="description"></div>
+    <div id="console"></div>
+    <script src=""
+    <script src=""
+ </body>
+</html>
+

Added: trunk/LayoutTests/fast/filesystem/resources/file-writer-abort-continue.js (0 => 96177)


--- trunk/LayoutTests/fast/filesystem/resources/file-writer-abort-continue.js	                        (rev 0)
+++ trunk/LayoutTests/fast/filesystem/resources/file-writer-abort-continue.js	2011-09-28 01:46:08 UTC (rev 96177)
@@ -0,0 +1,138 @@
+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;

Added: trunk/LayoutTests/fast/filesystem/resources/file-writer-abort-depth.js (0 => 96177)


--- trunk/LayoutTests/fast/filesystem/resources/file-writer-abort-depth.js	                        (rev 0)
+++ trunk/LayoutTests/fast/filesystem/resources/file-writer-abort-depth.js	2011-09-28 01:46:08 UTC (rev 96177)
@@ -0,0 +1,73 @@
+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;

Copied: trunk/LayoutTests/fast/filesystem/resources/file-writer-abort.js (from rev 96176, trunk/LayoutTests/fast/filesystem/resources/file-writer-events.js) (0 => 96177)


--- trunk/LayoutTests/fast/filesystem/resources/file-writer-abort.js	                        (rev 0)
+++ trunk/LayoutTests/fast/filesystem/resources/file-writer-abort.js	2011-09-28 01:46:08 UTC (rev 96177)
@@ -0,0 +1,86 @@
+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;

Modified: trunk/LayoutTests/fast/filesystem/resources/file-writer-events.js (96176 => 96177)


--- trunk/LayoutTests/fast/filesystem/resources/file-writer-events.js	2011-09-28 01:12:10 UTC (rev 96176)
+++ trunk/LayoutTests/fast/filesystem/resources/file-writer-events.js	2011-09-28 01:46:08 UTC (rev 96177)
@@ -98,5 +98,5 @@
     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;

Modified: trunk/Source/WebCore/ChangeLog (96176 => 96177)


--- trunk/Source/WebCore/ChangeLog	2011-09-28 01:12:10 UTC (rev 96176)
+++ trunk/Source/WebCore/ChangeLog	2011-09-28 01:46:08 UTC (rev 96177)
@@ -1,3 +1,18 @@
+2011-09-27  Eric Uhrhane  <[email protected]>
+
+        [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  <[email protected]>
 
         Unreviewed, rolling out r96141.

Modified: trunk/Source/WebCore/fileapi/FileWriter.cpp (96176 => 96177)


--- trunk/Source/WebCore/fileapi/FileWriter.cpp	2011-09-28 01:12:10 UTC (rev 96176)
+++ trunk/Source/WebCore/fileapi/FileWriter.cpp	2011-09-28 01:46:08 UTC (rev 96177)
@@ -43,18 +43,24 @@
 
 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();
 }
@@ -72,7 +78,8 @@
 
 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;
@@ -81,6 +88,8 @@
 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;
@@ -89,13 +98,21 @@
         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)
@@ -106,82 +123,140 @@
         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);
@@ -189,7 +264,10 @@
 
 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)

Modified: trunk/Source/WebCore/fileapi/FileWriter.h (96176 => 96177)


--- trunk/Source/WebCore/fileapi/FileWriter.h	2011-09-28 01:12:10 UTC (rev 96176)
+++ trunk/Source/WebCore/fileapi/FileWriter.h	2011-09-28 01:46:08 UTC (rev 96177)
@@ -90,6 +90,12 @@
     DEFINE_ATTRIBUTE_EVENT_LISTENER(writeend);
 
 private:
+    enum Operation {
+        OperationNone,
+        OperationWrite,
+        OperationTruncate
+    };
+
     FileWriter(ScriptExecutionContext*);
 
     virtual ~FileWriter();
@@ -100,42 +106,24 @@
     virtual EventTargetData* eventTargetData() { return &m_eventTargetData; }
     virtual EventTargetData* ensureEventTargetData() { return &m_eventTargetData; }
 
-    void fireEvent(const AtomicString& type);
+    void doOperation(Operation);
 
-    void setError(FileError::ErrorCode, ExceptionCode&);
-
     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));
-        }
+    void fireEvent(const AtomicString& type);
 
+    void setError(FileError::ErrorCode, ExceptionCode&);
 
-        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;
-    };
-
     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;
 };
 
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to