217434 - nsImapMockChannel does not implement Suspend/Resume

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

suspend() and resume() just return a failure code.

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

ok, well, it looks like this is nothing I can implement; -> default owner

Comment 2

[David :Bienvenu]

Is this an actual issue? 

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

Yes, bug 57342 and possibly bug 69938 require this to be implemented for imap
channels.

This is the channel that necko/exthandler/... use for attachments loaded from
mails on an imap server, right?

Comment 4

[Boris Zbarsky [:bzbarsky]]

David, this is an issue if nsImapMockChannel is used for attachments to mails
read via imap when the attachments are double-clicked on.  If that's the case,
things will soon break badly, because the helper app handler will soon require
that the channel it interacts with support suspend/resume.  If the channel does
not support it, that content will not open in helper applications and not be
able to be saved to disk.

Comment 5

[David :Bienvenu]

OK, makes sense. I can do this, I think - can you tell me roughly when you need
it by?

The channel gets suspended from the UI thread, right, not the imap thread?

Here's roughly how it would work - nsImapMockChannel::Suspend would set a flag
on the mock channel - resume would clear it..
nsImapProtocol::CreateNewLineFromSocket would check if the channel was suspended
before reading data from the server. Because of the way the parser is written, I
think we'd have to block in CreateNewLineFromSocket, waiting until the channel
is resumed or cancelled.

the listener is notified of ODA in nsImapProtocol::PostLineDownLoadEvent - if
blocking the imap thread turns out to be a huge problem, we could just continue
to buffer the data from the server that we requested but hold off sending ODA
events until the channel was resumed.

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

>OK, makes sense. I can do this, I think - can you tell me roughly when you need
>it by?

oh, I have no specific deadline, I suppose it would be nice if you could do it
before 1.6a freezes.

>The channel gets suspended from the UI thread, right, not the imap thread?

Correct.

Thanks for implementing this!

Comment 8

[Magnus Melin [:mkmelin]]

Daniel, did you want to have a go at it?

Comment 10

[Wayne Mery (:wsmwk)]

(In reply to Magnus Melin [:mkmelin] from comment #8)

Daniel, did you want to have a go at it?

Comment 11

[Daniel]

Attachment: Bug 217434 - nsImapMockChannel supports Suspend and Resume. r=BenC

Per suggestions at https://bugzilla.mozilla.org/show_bug.cgi?id=217434#c5:

• nsImapMockChannel::Suspend sets mSuspended

• nsImapMockChannel::{Resume,Cancel} unset mSuspended and notify the
  associated nsImapProtocol thread that waits on mSuspendedCV

• nsImapProtocol::CreateNewLineFromSocket is blocked until the
  associated m_mockChannel is resumed (or cancelled)

Assumptions:

• nsIMapMockChannel{Suspend,Resume,Cancel} are only called from the main (UI) thread
• nsImapProtocol::CreateNewLineFromSocket is called from threads other than the main one

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/a8aa32e1bd2c
nsImapMockChannel supports Suspend and Resume. r=BenC DONTBUILD

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/0d5a46fbeae0
followup - apply clang-format. rs=clang-format