Skip to content

Commit e92816c

Browse files
fallintoplacemeta-codesync[bot]
authored andcommitted
Keep FileReader in DONE state after abort (#57692)
Summary: Before #57375, reads never entered `LOADING`, so the active abort path was effectively unreachable. Now that reads use `LOADING`, that path calls `_reset()` before and after transitioning to `DONE`, leaving the reader in `EMPTY` after `abort()` returns. This differs from the [File API abort algorithm](https://w3c.github.io/FileAPI/#dfn-abort), which keeps an active reader in `DONE`. The final reset also overwrites a replacement read started by an `abort` handler, and that new read clears the shared `_aborted` flag before the canceled native promise settles. This change: - keeps an aborted active reader in `DONE` with a null result - skips the old `loadend` if the abort handler starts another read - gives each read an ID so a canceled native promise cannot complete over a newer read ## Changelog: [GENERAL] [FIXED] - Keep `FileReader` in the correct state after aborting a read. Pull Request resolved: #57692 Test Plan: - `yarn jest packages/react-native/Libraries/Blob/__tests__/FileReader-test.js --runInBand` - `./node_modules/.bin/prettier --check packages/react-native/Libraries/Blob/FileReader.js packages/react-native/Libraries/Blob/__tests__/FileReader-test.js` - `./node_modules/.bin/eslint --max-warnings 0 packages/react-native/Libraries/Blob/FileReader.js packages/react-native/Libraries/Blob/__tests__/FileReader-test.js` - `yarn flow-check` Reviewed By: christophpurrer Differential Revision: D113802292 Pulled By: Abbondanzo fbshipit-source-id: d5c8beeaf2539f3784e7f61ba500e67da575164c
1 parent 3b90423 commit e92816c

2 files changed

Lines changed: 84 additions & 22 deletions

File tree

packages/react-native/Libraries/Blob/FileReader.js

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ class FileReader extends EventTarget {
4444
_error: ?Error;
4545
_result: ?ReaderResult;
4646
_aborted: boolean = false;
47+
_readId: number = 0;
4748

4849
constructor() {
4950
super();
@@ -56,6 +57,15 @@ class FileReader extends EventTarget {
5657
this._result = null;
5758
}
5859

60+
_startRead(): number {
61+
this._aborted = false;
62+
this._error = null;
63+
this._result = null;
64+
const readId = ++this._readId;
65+
this._setReadyState(LOADING);
66+
return readId;
67+
}
68+
5969
_setReadyState(newState: ReadyState) {
6070
this._readyState = newState;
6171
this.dispatchEvent(new Event('readystatechange'));
@@ -67,24 +77,24 @@ class FileReader extends EventTarget {
6777
} else {
6878
this.dispatchEvent(new Event('load'));
6979
}
70-
this.dispatchEvent(new Event('loadend'));
80+
if (this._readyState !== LOADING) {
81+
this.dispatchEvent(new Event('loadend'));
82+
}
7183
}
7284
}
7385

7486
readAsArrayBuffer(blob: ?Blob): void {
75-
this._aborted = false;
76-
7787
if (blob == null) {
7888
throw new TypeError(
7989
"Failed to execute 'readAsArrayBuffer' on 'FileReader': parameter 1 is not of type 'Blob'",
8090
);
8191
}
8292

83-
this._setReadyState(LOADING);
93+
const readId = this._startRead();
8494

8595
NativeFileReaderModule.readAsDataURL(blob.data).then(
8696
(text: string) => {
87-
if (this._aborted) {
97+
if (readId !== this._readId) {
8898
return;
8999
}
90100

@@ -95,7 +105,7 @@ class FileReader extends EventTarget {
95105
this._setReadyState(DONE);
96106
},
97107
error => {
98-
if (this._aborted) {
108+
if (readId !== this._readId) {
99109
return;
100110
}
101111
this._error = error;
@@ -105,26 +115,24 @@ class FileReader extends EventTarget {
105115
}
106116

107117
readAsDataURL(blob: ?Blob): void {
108-
this._aborted = false;
109-
110118
if (blob == null) {
111119
throw new TypeError(
112120
"Failed to execute 'readAsDataURL' on 'FileReader': parameter 1 is not of type 'Blob'",
113121
);
114122
}
115123

116-
this._setReadyState(LOADING);
124+
const readId = this._startRead();
117125

118126
NativeFileReaderModule.readAsDataURL(blob.data).then(
119127
(text: string) => {
120-
if (this._aborted) {
128+
if (readId !== this._readId) {
121129
return;
122130
}
123131
this._result = text;
124132
this._setReadyState(DONE);
125133
},
126134
error => {
127-
if (this._aborted) {
135+
if (readId !== this._readId) {
128136
return;
129137
}
130138
this._error = error;
@@ -134,26 +142,24 @@ class FileReader extends EventTarget {
134142
}
135143

136144
readAsText(blob: ?Blob, encoding: string = 'UTF-8'): void {
137-
this._aborted = false;
138-
139145
if (blob == null) {
140146
throw new TypeError(
141147
"Failed to execute 'readAsText' on 'FileReader': parameter 1 is not of type 'Blob'",
142148
);
143149
}
144150

145-
this._setReadyState(LOADING);
151+
const readId = this._startRead();
146152

147153
NativeFileReaderModule.readAsText(blob.data, encoding).then(
148154
(text: string) => {
149-
if (this._aborted) {
155+
if (readId !== this._readId) {
150156
return;
151157
}
152158
this._result = text;
153159
this._setReadyState(DONE);
154160
},
155161
error => {
156-
if (this._aborted) {
162+
if (readId !== this._readId) {
157163
return;
158164
}
159165
this._error = error;
@@ -163,14 +169,12 @@ class FileReader extends EventTarget {
163169
}
164170

165171
abort() {
166-
this._aborted = true;
167-
// only call onreadystatechange if there is something to abort, as per spec
168-
if (this._readyState !== EMPTY && this._readyState !== DONE) {
169-
this._reset();
172+
this._result = null;
173+
if (this._readyState === LOADING) {
174+
this._aborted = true;
175+
this._readId++;
170176
this._setReadyState(DONE);
171177
}
172-
// Reset again after, in case modified in handler
173-
this._reset();
174178
}
175179

176180
get readyState(): ReadyState {

packages/react-native/Libraries/Blob/__tests__/FileReader-test.js

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import type Event from '../../../src/private/webapis/dom/events/Event';
1414

1515
const Blob = require('../Blob').default;
1616
const FileReader = require('../FileReader').default;
17+
const NativeFileReaderModule = require('../NativeFileReaderModule').default;
1718

1819
jest.mock('../../BatchedBridge/NativeModules', () => ({
1920
__esModule: true,
@@ -69,6 +70,63 @@ describe('FileReader', function () {
6970
reader.abort();
7071
expect(aborted).toBe(true);
7172
expect(loadended).toBe(true);
73+
expect(reader.readyState).toBe(FileReader.DONE);
74+
expect(reader.result).toBe(null);
75+
});
76+
77+
it('should preserve a read started by an abort handler', async () => {
78+
const reader = new FileReader();
79+
let loadendCount = 0;
80+
const replacementRead = new Promise<void>(resolve => {
81+
reader.onloadend = () => {
82+
loadendCount++;
83+
resolve();
84+
};
85+
});
86+
reader.onabort = () => {
87+
reader.readAsText(new Blob());
88+
};
89+
90+
reader.readAsText(new Blob());
91+
reader.abort();
92+
93+
expect(reader.readyState).toBe(FileReader.LOADING);
94+
expect(loadendCount).toBe(0);
95+
96+
await replacementRead;
97+
expect(reader.readyState).toBe(FileReader.DONE);
98+
expect(reader.result).toBe('');
99+
expect(loadendCount).toBe(1);
100+
});
101+
102+
it('should clear stale result and error when starting a read', async () => {
103+
const reader = new FileReader();
104+
const readAsText = jest.spyOn(NativeFileReaderModule, 'readAsText');
105+
106+
const successfulRead = new Promise<void>(resolve => {
107+
reader.onloadend = () => resolve();
108+
});
109+
reader.readAsText(new Blob());
110+
await successfulRead;
111+
expect(reader.result).toBe('');
112+
113+
const error = new Error('read failed');
114+
readAsText.mockRejectedValueOnce(error);
115+
const failedRead = new Promise<void>(resolve => {
116+
reader.onloadend = () => resolve();
117+
});
118+
reader.readAsText(new Blob());
119+
expect(reader.result).toBe(null);
120+
expect(reader.error).toBe(null);
121+
await failedRead;
122+
expect(reader.error).toBe(error);
123+
124+
readAsText.mockReturnValueOnce(new Promise(() => {}));
125+
reader.readAsText(new Blob());
126+
expect(reader.result).toBe(null);
127+
expect(reader.error).toBe(null);
128+
129+
readAsText.mockRestore();
72130
});
73131

74132
it('should read blob as ArrayBuffer', async () => {

0 commit comments

Comments
 (0)