:Gavin Sharp [email: gavin@gavinsharp.com] Reporter
	Description • 10 years ago

Not sure whether this is a wider problem or something very specific, but I just tried to download an attachment from Gmail in a current Nightly (built from m-c revision 818f353b7aa6) with e10s enabled, and I got a corrupted file - the file is too small and its contents are, as far as I can tell, garbage.

The download manager reports the "wrong" size (matching the resulting file on disk), and suggests the download was successful, which suggests to me the problem is "upstream" from that code.

Opening a non-e10s window and downloading the same file works fine.

	:Gavin Sharp [email: gavin@gavinsharp.com] Reporter
	Comment 1 • 10 years ago

Hmm, I can reproduce in a clean profile (after enabling e10s), but it only happens with this specific file (a TCX file describing http://www.strava.com/activities/200255108).

	:Gavin Sharp [email: gavin@gavinsharp.com] Reporter
	Comment 2 • 10 years ago

(The other files I tried are a 4 byte file and a 512KB text file consisting of just "1" characters.)

	:Gavin Sharp [email: gavin@gavinsharp.com] Reporter
	Comment 3 • 10 years ago

STR:
1) email attachment 8496479 [details] to a gmail account as an attachment
2 [review]) start e10s build with clean profile, log in to the gmail account
3) open message, download the attachment

Expected: Download manager shows download size as 413KB, file is XML
Actual: Download manager shows download size as 40.8KB, file is corrupted on disk

	:Gavin Sharp [email: gavin@gavinsharp.com] Reporter
	Comment 4 • 10 years ago

Hrm, something weird is going on. I just tried the download in a Firefox 32.0.3 release build, and it completed successfully (file was 413KB, not corrupt), but the download size reported by the download manager was also 40.8KB (which matches the size of the "corrupt" file in the e10s case).

	Kevin Brosnan
	Comment 5 • 10 years ago

Works for me using - Mozilla/5.0 (Windows NT 6.3; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0 Today's build.

I tried a couple older pdfs that were in my gmail inbox and those downloaded fine as well.

	:Gavin Sharp [email: gavin@gavinsharp.com] Reporter
	Comment 7 • 10 years ago

FWIW that bug manifests itself pretty differently from the comment there, so it might be worth testing separately.

	:Gavin Sharp [email: gavin@gavinsharp.com] Reporter
	Comment 8 • 10 years ago

I'm still seeing this.

	Jim Mathies [:jimm]
	Comment 9 • 10 years ago

(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #8)
> I'm still seeing this.

What kind of attachment? Any specific str you can add?

	Jim Mathies [:jimm]
	Comment 10 • 10 years ago

Not having much luck reproducing with various attachments, including the tcx file in comment 3 on mac and windows.

	Dave Townsend [:mossop]
	Comment 11 • 10 years ago

I can't reproduce this either.

	:Gavin Sharp [email: gavin@gavinsharp.com] Reporter
	Comment 12 • 10 years ago

Beyond comment 3, I've got nothing. This might be Google-account specific and related to cookie issues, as I theorized on IRC, so this could be tricky to debug...

	Brad Lassey [:blassey] (use needinfo?)
	Comment 14 • 10 years ago

better STR in https://bugzilla.mozilla.org/show_bug.cgi?id=1082134#c0

	cmtalbert
	Comment 15 • 10 years ago

Just to copy status from bug 1082134, I can still repro but only on the synergy download site behind the paywall (this is with today's nighlty build).

It's pretty cheap to get behind said paywall, so I recommend getting a copy and expensing it. Downloading the 1.5.x msi files seems to fail every time. Interestingly the synergy project nightly build page works every time (though it didn't a few weeks ago, so this bug's behavior is changing).

	Dragana Damjanovic [:dragana] Assignee
	Comment 16 • 10 years ago

There is a patch that landed a day after your comments that should fix you problem.

But looking at it, I was trying https://synergy-project.org/nightly/ I found another bug related to this one. Thanks.

Patch is coming.

	Dragana Damjanovic [:dragana] Assignee
	Comment 17 • 10 years ago

Hi Steve,

I am giving the review to you because you were working on it recently and I need a to ask you about a comment in you patch.

it is about proper encoding when we divert channel from parent back to child.
The problem with you patch is:
it diverts mParentListener in HttpChannelParent to a new listener. 
then DoApplyContentConversions is call and we have a new mConverterListener. So if RecvDivertOnDataAvailable are received the mConverterListener's OnDataAvailable is called and data are properly encoded.
After the channel is resume and HttpChannelParentListener's OnDataAvailable is called from the nsHttpChannel, HttpChannelParentListener has this new listener but it does not have the converters, so data are partially converted and partially not.

I was just wandering about the comment in http://hg.mozilla.org/mozilla-central/annotate/72ae882fce1a/netwerk/protocol/http/HttpChannelParent.cpp#l1009 .. 1016.
I am not sure how the first option is reflected in your code, I do not see it at all. Can you explain it to me, thanks.

	Steve Workman [:sworkman] (INACTIVE)
	Comment 18 • 10 years ago

Comment on attachment 8517589 [details] [diff] [review]
fix v1

Review of attachment 8517589 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Dragana Damjanovic from comment #17)
> ...
> After the channel is resume and HttpChannelParentListener's OnDataAvailable
> is called from the nsHttpChannel, HttpChannelParentListener has this new
> listener but it does not have the converters, so data are partially
> converted and partially not.

Good catch - yes, my previous patch did not add the decoder listener chain to HttpChannelParentListener for ResumeAfterDiversion.

> I was just wandering about the comment in
> http://hg.mozilla.org/mozilla-central/annotate/72ae882fce1a/netwerk/protocol/
> http/HttpChannelParent.cpp#l1009 .. 1016.
> I am not sure how the first option is reflected in your code, I do not see
> it at all. Can you explain it to me, thanks.

It's not ;) Can you update the comments to make sure they reflect your changes, please? r=me with comment updates.

	Dragana Damjanovic [:dragana] Assignee
	Comment 19 • 10 years ago

Only comment updated.

	Dragana Damjanovic [:dragana] Assignee
	Comment 21 • 10 years ago

https://tbpl.mozilla.org/?tree=Try&rev=259f4e9a4530

	Carsten Book [:Tomcat]
	Comment 22 • 10 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/a33eb2f32a32

	Carsten Book [:Tomcat]
	Comment 23 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/a33eb2f32a32

	[SV Manager] Florin Mezei, QA (:FlorinMezei)
	Comment 24 • 9 years ago

There don't seem to have been any more reports of this issue after the fix.