1367707 - Download rest of message not working after fetch headers only or partial download (do not download messages larger than ...) - POP account - Part 2

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Jorg K (CEST = GMT+2)]

+++ This bug was initially created as a clone of Bug #1361020 +++

This was initially reported in bug 1361020 (headers only) and bug 1363274 (partially downloaded/truncated message).

Two bugs contributed to this failure: Bug 968342 and bug 791645.

Since bug 791645 is not in TB 52, the fix to bug 1361020 alone fixes the problem for TB 52. For later version, like TB 54 beta or TB 55 Daily (those are the versions at time of writing) the problem still exists.

In bug 1361020 I fixed the part that was caused by a restricted content policy introduced in bug 968342. With bug 1361020 fixed, users can now click on the links in the messages again "Download the rest of the message", and that somehow triggers the download since in the status bar "Loading Message..." is shown. However, the message never loads completely. That second problem is caused by the proxy changes in bug 791645 as was detected by Alice in bug 1361020 comment #11.

Comment 1

[Jorg K (CEST = GMT+2)]

This problem it due to incorrect refactoring in the proxy bug 791645.

Clicking on the link in the message "Download the rest of the message" triggers a POP load via a click. Looking at some stacks I see:
nsDocShell::OnLinkClickSync() calling nsDocShell::DoURILoad() which calls NS_NewChannelInternal() which ultimately calls nsPop3Service::NewChannel2() which calls protocol->Initialize(aURI) which loads(!!) the URL. The load is delayed due to the async proxy.

Then nsDocShell::DoURILoad() calls DoChannelLoad() which ultimately calls nsMsgProtocol::AsyncOpen() and this calls nsPop3Protocol::LoadUrl() which calls Initialize() again, doh!

A second call to set up a proxy is made and then nsPop3Protocol::OnProxyAvailable() arrives twice in a row for matching the two Initialize() calls. The second one fails and the whole thing hangs. Just suppressing the second call makes the message load properly. So there's the problem.

So there seems to be an architectural problem that wasn't uncovered while things ran synchronously. I guess the URL was loaded twice, maybe the second time around it noticed that it was loaded already.

Comment 2

[Jorg K (CEST = GMT+2)]

Attachment: 1367707-load-rest-of-message.patch

This little reshuffling fixes the problem.

Comment 3

[Jorg K (CEST = GMT+2)]

Attachment: 1367707-load-rest-of-message.patch (v2).

(compile error on Mac due to unused variable.)

Comment 4

[Jorg K (CEST = GMT+2)]

Comment on attachment 8872104 [details] [diff] [review]
1367707-load-rest-of-message.patch (v2).

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=a77fba6de0c4a4bf97498f97759ae75ac362d7f4

This is a follow-up of bug 791645.

r+ as per bug 791645 comment #38 (quote):
===
We've had both jcranmer and now jorgk look this over. I can't really spare the couple of days this would take me anytime soon. I realize this is putting you out on a limb a bit here jorkg, but I think that the best thing is for you to [...] land it with your review, as you are doing, then get some testing feedback after it is landed.
===

and the following Council post:
===
... doing that review means I have bought the problem, opening myself up to additional pressure in the future.
===

Comment 5

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/comm-central/rev/489ff5fa04053595b1505036c27c8177f5a837e5

Comment 6

[Jorg K (CEST = GMT+2)]

Beta (TB 54):
https://hg.mozilla.org/releases/comm-beta/rev/1ca6b6c3bb5f4ecbfcd54082ef1d7f07982eed3f