Upload progress incorrect in Google Chrome

[mkszepp]

Sometimes the browser doesn't has the correct file size onloadstart. like described here: #1002 (comment)

During debugging i have found out, that this bug is present in Chrome. In Chrome there is often a total wrong total size (see comment) and in Firefox there is sometime a little difference between the total size onloadstart and onprogress.

We should set file.size like in earlier versions also in onprogress event and check in progress event, if the upload is already complete.

#1013

Progress log on Chrome with this fix (3 files in queue):

0
0
1
1
3
3
5
5
6
6
8
8
10
10
11
11
13
13
15
15
16
16
18
18
20
20
21
21
23
23
24
24
24
24
24
24
24
24 // first file uploaded
26
26
27
27
29
29
31
31
33
33
34
34
36
36
38
38
39
39
41
41
43
43
44
44
46
46
48
48
49
49
51
51
53
53
55
55
56
56
58
58
60
60
61
61
63
63
65
65
66
66
68
68
70
70
71
71
73
73
75
75
76
76
78
78
80
80
82
82
83
83
85
85
87
87
88
88
88
88
88
88
88 // 2nd file uploaded
88
89
89
91
91
93
93
95
95
96
96
98
98
100
100
100
100 // all files uploaded

Progress log in Firefox after fix (3 files in queue):

3
6
9
12
15
19
22
24
24 // first file uploaded
27
31
34
37
40
43
47
50
53
56
59
62
66
69
72
75
78
82
85
86
86 // 2nd file uploaded
89
92
95
98
100 // all files uploaded

[RobbieTheWagner]

@mkszepp would it be possible to add some tests for checking progress values? We could then run those tests in both Chrome and Firefox to catch regressions here.

[jelhan]

We could then run those tests in both Chrome and Firefox to catch regressions here.

I'm not sure if we can test the difference at this level. If I understand correctly, both Chrome and Firefox may have a wrong file size in onloadstart event. I fear we are mocking that layer in the tests:

ember-file-upload/ember-file-upload/src/mirage/upload-handler.ts

Lines 55 to 61 in c3af01e

	request.upload.onloadstart(
	new ProgressEvent('loadstart', {
	lengthComputable: true,
	total,
	loaded: Math.min(loaded, total),
	}),
	);

[jelhan]

I had a look in specification: https://xhr.spec.whatwg.org/#the-send()-method

If I understand correctly, the total of ProgressEvent fired at loadstart should be either 0 or the actual length of the body. See step 11. But if I understand correctly, we see different behavior of the browsers in reality, correct?

[mkszepp]

@jelhan exact, we have specially on Chrome the behavior, that loadstart brings a the wrong total. I think this problem occurs with large files, small files are not the problems (because there was 1 of 3 files with this bug)...
In Firefox i have got also yesterday with one file a little difference (but not always).

I have tested this fix today with a 30 MB image in "Fast 3G" mode and it works.

Edit:
I will try to make some tests in the next days

[RobbieTheWagner]

@mkszepp great, thanks! Please keep me posted on writing tests. I do think we should probably merge this either way, but would love to have regression coverage.

[gilest]

Thank you @mkszepp – this looks like an improvement. I'll try to stand up tests

I'm not sure if we can test the difference at this level. If I understand correctly, both Chrome and Firefox may have a wrong file size in onloadstart event. I fear we are mocking that layer in the tests

Yes, that's why I am requesting (in #1013) that we record some upload event data from different browsers and endpoints so that we can re-arrange some internals and replay those events in unit/integration tests

Testing this with the existing mirage handler is useless because the browser and endpoint behaviour is mocked

[gilest]

Pretty sure I've validated with unit tests that this fix is correct – at least for this specific issue (progress straight to 100% in Google Chrome). I'll open a proof PR shortly

[gilest]

Thanks again.

Fix released in 8.3.1

[mkszepp]

Thanks! i have tested in my app and works!

@RobbieTheWagner maybe you can give us also a short feedback, if the bug in you app is also resolved with this fix.