Title: [268054] trunk
Revision
268054
Author
commit-qu...@webkit.org
Date
2020-10-06 09:45:24 -0700 (Tue, 06 Oct 2020)

Log Message

FileReader should transition to readyState DONE after last onprogress event
https://bugs.webkit.org/show_bug.cgi?id=217333

Patch by Alex Christensen <achristen...@webkit.org> on 2020-10-06
Reviewed by Youenn Fablet.

LayoutTests/imported/w3c:

* web-platform-tests/FileAPI/FileReader/workers-expected.txt:
* web-platform-tests/FileAPI/fileReader-expected.txt:
* web-platform-tests/FileAPI/fileReader.html:
* web-platform-tests/FileAPI/reading-data-section/FileReader-multiple-reads-expected.txt:
* web-platform-tests/FileAPI/reading-data-section/filereader_abort-expected.txt:
* web-platform-tests/FileAPI/reading-data-section/filereader_readAsArrayBuffer-expected.txt:
* web-platform-tests/FileAPI/reading-data-section/filereader_readAsBinaryString-expected.txt:
* web-platform-tests/FileAPI/reading-data-section/filereader_readAsDataURL-expected.txt:
* web-platform-tests/FileAPI/reading-data-section/filereader_readAsText-expected.txt:

Source/WebCore:

I also bring the abort function in alignment with https://w3c.github.io/FileAPI/
and the behavior of Chrome and Firefox.

As suggested by Chris, I added a bool to remember if loading is complete so we can
retain existing behavior when abort is called inside the last onprogress event.
This is not mentioned by the spec yet, but it matches the existing behavior and the
behavior of Chrome, so I added a web platform test for it.

* fileapi/FileReader.cpp:
(WebCore::FileReader::abort):
(WebCore::FileReader::didFinishLoading):
(WebCore::FileReader::didFail):
* fileapi/FileReader.h:

LayoutTests:

* fast/files/file-reader-abort-expected.txt:
* fast/files/file-reader-abort-using-open-panel-expected.txt:
Update expectations to reflect new behavior that onerror is not called before onabort,
matching Chrome, Firefox, and the specification.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (268053 => 268054)


--- trunk/LayoutTests/ChangeLog	2020-10-06 16:40:12 UTC (rev 268053)
+++ trunk/LayoutTests/ChangeLog	2020-10-06 16:45:24 UTC (rev 268054)
@@ -1,3 +1,15 @@
+2020-10-06  Alex Christensen  <achristen...@webkit.org>
+
+        FileReader should transition to readyState DONE after last onprogress event
+        https://bugs.webkit.org/show_bug.cgi?id=217333
+
+        Reviewed by Youenn Fablet.
+
+        * fast/files/file-reader-abort-expected.txt:
+        * fast/files/file-reader-abort-using-open-panel-expected.txt:
+        Update expectations to reflect new behavior that onerror is not called before onabort,
+        matching Chrome, Firefox, and the specification.
+
 2020-10-06  Diego Pino Garcia  <dp...@igalia.com>
 
         [GLIB] Unreviewed test gardening. Skip LFC tests.

Modified: trunk/LayoutTests/fast/files/file-reader-abort-expected.txt (268053 => 268054)


--- trunk/LayoutTests/fast/files/file-reader-abort-expected.txt	2020-10-06 16:40:12 UTC (rev 268053)
+++ trunk/LayoutTests/fast/files/file-reader-abort-expected.txt	2020-10-06 16:45:24 UTC (rev 268054)
@@ -1,7 +1,6 @@
 
 Test that FileReader.abort works.
 Received loadstart event
-Received error event: 20
 Received abort event
 Received loadend event
 DONE

Modified: trunk/LayoutTests/fast/files/file-reader-abort-using-open-panel-expected.txt (268053 => 268054)


--- trunk/LayoutTests/fast/files/file-reader-abort-using-open-panel-expected.txt	2020-10-06 16:40:12 UTC (rev 268053)
+++ trunk/LayoutTests/fast/files/file-reader-abort-using-open-panel-expected.txt	2020-10-06 16:45:24 UTC (rev 268054)
@@ -2,7 +2,6 @@
 
 Test that FileReader.abort works.
 Received loadstart event
-Received error event: 20
 Received abort event
 Received loadend event
 DONE

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (268053 => 268054)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2020-10-06 16:40:12 UTC (rev 268053)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2020-10-06 16:45:24 UTC (rev 268054)
@@ -1,5 +1,22 @@
 2020-10-06  Alex Christensen  <achristen...@webkit.org>
 
+        FileReader should transition to readyState DONE after last onprogress event
+        https://bugs.webkit.org/show_bug.cgi?id=217333
+
+        Reviewed by Youenn Fablet.
+
+        * web-platform-tests/FileAPI/FileReader/workers-expected.txt:
+        * web-platform-tests/FileAPI/fileReader-expected.txt:
+        * web-platform-tests/FileAPI/fileReader.html:
+        * web-platform-tests/FileAPI/reading-data-section/FileReader-multiple-reads-expected.txt:
+        * web-platform-tests/FileAPI/reading-data-section/filereader_abort-expected.txt:
+        * web-platform-tests/FileAPI/reading-data-section/filereader_readAsArrayBuffer-expected.txt:
+        * web-platform-tests/FileAPI/reading-data-section/filereader_readAsBinaryString-expected.txt:
+        * web-platform-tests/FileAPI/reading-data-section/filereader_readAsDataURL-expected.txt:
+        * web-platform-tests/FileAPI/reading-data-section/filereader_readAsText-expected.txt:
+
+2020-10-06  Alex Christensen  <achristen...@webkit.org>
+
         Align URL setters with reasonably behaving other browsers
         https://bugs.webkit.org/show_bug.cgi?id=217366
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/FileReader/workers-expected.txt (268053 => 268054)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/FileReader/workers-expected.txt	2020-10-06 16:40:12 UTC (rev 268053)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/FileReader/workers-expected.txt	2020-10-06 16:45:24 UTC (rev 268054)
@@ -1,10 +1,3 @@
-CONSOLE MESSAGE: InvalidStateError: The object is in an invalid state.
 
-Harness Error (FAIL), message = InvalidStateError: The object is in an invalid state.
+PASS FileReader created after a worker self.close()
 
-TIMEOUT FileReader created after a worker self.close() Test timed out
-
-Harness Error (FAIL), message = InvalidStateError: The object is in an invalid state.
-
-TIMEOUT FileReader created after a worker self.close() Test timed out
-

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/fileReader-expected.txt (268053 => 268054)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/fileReader-expected.txt	2020-10-06 16:40:12 UTC (rev 268053)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/fileReader-expected.txt	2020-10-06 16:45:24 UTC (rev 268054)
@@ -1,6 +1,7 @@
 
 PASS FileReader interface object
 PASS no-argument FileReader constructor
-FAIL FileReader States -- abort assert_unreached: abort event should fire sync Reached unreachable code
-FAIL FileReader States -- events assert_equals: expected 1 but got 2
+PASS FileReader States -- abort
+PASS FileReader States -- events
+PASS FileReader States -- abort during progress
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/fileReader.html (268053 => 268054)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/fileReader.html	2020-10-06 16:40:12 UTC (rev 268053)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/fileReader.html	2020-10-06 16:45:24 UTC (rev 268054)
@@ -35,6 +35,7 @@
                 assert_equals(fileReader.readyState, FileReader.DONE);
                 t_abort.done();
             });
+            fileReader._onerror_ = this.unreached_func("onerror should not be called when abort is called");
             fileReader.abort();
             fileReader._onabort_ = this.unreached_func("abort event should fire sync")
         });
@@ -62,6 +63,31 @@
                 t_event.done();
             });
         });
+
+        var t_abort_during_progress = async_test("FileReader States -- abort during progress");
+        t_abort_during_progress.step(function() {
+            var fileReader = new FileReader();
+
+            var blob = new Blob([1]);
+            fileReader.readAsArrayBuffer(blob);
+
+            var _onprogressCalled_ = false;
+            fileReader._onprogress_ = this.step_func(function(e) {
+                _onprogressCalled_ = true;
+                fileReader.abort();
+            });
+
+            fileReader._onabort_ = this.step_func(function(e) {
+                assert_unreached("onabort should not be called if abort is called during final onprogress");
+            });
+
+            fileReader._onloadend_ = this.step_func(function(e) {
+                assert_true(onprogressCalled, "onprogress should be called before onloadend");
+                setTimeout(function() {
+                    t_abort_during_progress.done();
+                }, 0);
+            });
+        });
     </script>
   </body>
 </html>

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/reading-data-section/FileReader-multiple-reads-expected.txt (268053 => 268054)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/reading-data-section/FileReader-multiple-reads-expected.txt	2020-10-06 16:40:12 UTC (rev 268053)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/reading-data-section/FileReader-multiple-reads-expected.txt	2020-10-06 16:45:24 UTC (rev 268054)
@@ -4,5 +4,5 @@
 PASS test FileReader InvalidStateError exception for readAsArrayBuffer
 PASS test FileReader InvalidStateError exception in onloadstart event for readAsArrayBuffer
 PASS test FileReader no InvalidStateError exception in loadend event handler for readAsArrayBuffer
-FAIL test abort and restart in onloadstart event for readAsText The object is in an invalid state.
+PASS test abort and restart in onloadstart event for readAsText
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/reading-data-section/filereader_abort-expected.txt (268053 => 268054)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/reading-data-section/filereader_abort-expected.txt	2020-10-06 16:40:12 UTC (rev 268053)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/reading-data-section/filereader_abort-expected.txt	2020-10-06 16:45:24 UTC (rev 268054)
@@ -1,4 +1,4 @@
 
 PASS Aborting before read
-FAIL Aborting after read assert_equals: Expected abort event, but got error event instead expected "abort" but got "error"
+PASS Aborting after read
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/reading-data-section/filereader_readAsArrayBuffer-expected.txt (268053 => 268054)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/reading-data-section/filereader_readAsArrayBuffer-expected.txt	2020-10-06 16:40:12 UTC (rev 268053)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/reading-data-section/filereader_readAsArrayBuffer-expected.txt	2020-10-06 16:45:24 UTC (rev 268054)
@@ -1,3 +1,3 @@
 
-FAIL FileAPI Test: filereader_readAsArrayBuffer assert_equals: expected 1 but got 2
+PASS FileAPI Test: filereader_readAsArrayBuffer
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/reading-data-section/filereader_readAsBinaryString-expected.txt (268053 => 268054)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/reading-data-section/filereader_readAsBinaryString-expected.txt	2020-10-06 16:40:12 UTC (rev 268053)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/reading-data-section/filereader_readAsBinaryString-expected.txt	2020-10-06 16:45:24 UTC (rev 268054)
@@ -1,3 +1,3 @@
 
-FAIL FileAPI Test: filereader_readAsBinaryString assert_equals: expected 1 but got 2
+PASS FileAPI Test: filereader_readAsBinaryString
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/reading-data-section/filereader_readAsDataURL-expected.txt (268053 => 268054)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/reading-data-section/filereader_readAsDataURL-expected.txt	2020-10-06 16:40:12 UTC (rev 268053)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/reading-data-section/filereader_readAsDataURL-expected.txt	2020-10-06 16:45:24 UTC (rev 268054)
@@ -1,5 +1,5 @@
 
-FAIL FileReader readyState during readAsDataURL assert_equals: expected 1 but got 2
+PASS FileReader readyState during readAsDataURL
 PASS readAsDataURL result for Blob with specified MIME type
 FAIL readAsDataURL result for Blob with unspecified MIME type assert_equals: expected "data:application/octet-stream;base64,VEVTVA==" but got "data:;base64,VEVTVA=="
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/reading-data-section/filereader_readAsText-expected.txt (268053 => 268054)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/reading-data-section/filereader_readAsText-expected.txt	2020-10-06 16:40:12 UTC (rev 268053)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/reading-data-section/filereader_readAsText-expected.txt	2020-10-06 16:45:24 UTC (rev 268054)
@@ -1,4 +1,4 @@
 
-FAIL readAsText should correctly read UTF-8. assert_equals: expected 1 but got 2
+PASS readAsText should correctly read UTF-8.
 PASS readAsText should correctly read UTF-16.
 

Modified: trunk/Source/WebCore/ChangeLog (268053 => 268054)


--- trunk/Source/WebCore/ChangeLog	2020-10-06 16:40:12 UTC (rev 268053)
+++ trunk/Source/WebCore/ChangeLog	2020-10-06 16:45:24 UTC (rev 268054)
@@ -1,3 +1,24 @@
+2020-10-06  Alex Christensen  <achristen...@webkit.org>
+
+        FileReader should transition to readyState DONE after last onprogress event
+        https://bugs.webkit.org/show_bug.cgi?id=217333
+
+        Reviewed by Youenn Fablet.
+
+        I also bring the abort function in alignment with https://w3c.github.io/FileAPI/
+        and the behavior of Chrome and Firefox.
+
+        As suggested by Chris, I added a bool to remember if loading is complete so we can
+        retain existing behavior when abort is called inside the last onprogress event.
+        This is not mentioned by the spec yet, but it matches the existing behavior and the
+        behavior of Chrome, so I added a web platform test for it.
+
+        * fileapi/FileReader.cpp:
+        (WebCore::FileReader::abort):
+        (WebCore::FileReader::didFinishLoading):
+        (WebCore::FileReader::didFail):
+        * fileapi/FileReader.h:
+
 2020-10-06  Simon Fraser  <simon.fra...@apple.com>
 
         Rename scheduleTimedRenderingUpdate() to scheduleRenderingUpdate() everywhere

Modified: trunk/Source/WebCore/fileapi/FileReader.cpp (268053 => 268054)


--- trunk/Source/WebCore/fileapi/FileReader.cpp	2020-10-06 16:40:12 UTC (rev 268053)
+++ trunk/Source/WebCore/fileapi/FileReader.cpp	2020-10-06 16:45:24 UTC (rev 268054)
@@ -152,25 +152,16 @@
 void FileReader::abort()
 {
     LOG(FileAPI, "FileReader: aborting\n");
-
-    if (m_aborting || m_state != LOADING)
+    if (m_state != LOADING || m_finishedLoading)
         return;
-    m_aborting = true;
 
-    // Schedule to have the abort done later since abort() might be called from the event handler and we do not want the resource loading code to be in the stack.
     m_pendingTasks.clear();
-    enqueueTask([this] {
-        ASSERT(m_state != DONE);
+    stop();
+    m_error = DOMException::create(Exception { AbortError });
 
-        stop();
-        m_aborting = false;
-
-        m_error = DOMException::create(Exception { AbortError });
-
-        fireEvent(eventNames().errorEvent);
-        fireEvent(eventNames().abortEvent);
-        fireEvent(eventNames().loadendEvent);
-    });
+    auto protectedThis = makeRef(*this);
+    fireEvent(eventNames().abortEvent);
+    fireEvent(eventNames().loadendEvent);
 }
 
 void FileReader::didStartLoading()
@@ -197,14 +188,14 @@
 
 void FileReader::didFinishLoading()
 {
-    if (m_aborting)
-        return;
-
     enqueueTask([this] {
-        ASSERT(m_state != DONE);
+        if (m_state == DONE)
+            return;
+        m_finishedLoading = true;
+        fireEvent(eventNames().progressEvent);
+        if (m_state == DONE)
+            return;
         m_state = DONE;
-
-        fireEvent(eventNames().progressEvent);
         fireEvent(eventNames().loadEvent);
         fireEvent(eventNames().loadendEvent);
     });
@@ -212,12 +203,9 @@
 
 void FileReader::didFail(ExceptionCode errorCode)
 {
-    // If we're aborting, do not proceed with normal error handling since it is covered in aborting code.
-    if (m_aborting)
-        return;
-
     enqueueTask([this, errorCode] {
-        ASSERT(m_state != DONE);
+        if (m_state == DONE)
+            return;
         m_state = DONE;
 
         m_error = DOMException::create(Exception { errorCode });

Modified: trunk/Source/WebCore/fileapi/FileReader.h (268053 => 268054)


--- trunk/Source/WebCore/fileapi/FileReader.h	2020-10-06 16:40:12 UTC (rev 268053)
+++ trunk/Source/WebCore/fileapi/FileReader.h	2020-10-06 16:45:24 UTC (rev 268054)
@@ -103,7 +103,7 @@
     void fireEvent(const AtomString& type);
 
     ReadyState m_state { EMPTY };
-    bool m_aborting { false };
+    bool m_finishedLoading { false };
     RefPtr<Blob> m_blob;
     FileReaderLoader::ReadType m_readType { FileReaderLoader::ReadAsBinaryString };
     String m_encoding;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to