Even though we're trying to stream messages only locally when compacting offline stores, the stream message code can still cause an imap server message fetch if we think we have the message offline, but the offset into the offline store for the message is incorrect - in that case, we fail the check for the message starting with "From " and the imap protocol code falls back to trying to fetch the message from the imap server. This needs to block rc1. It shouldn't be too hard to fix. It could also cause gloda to try to stream messages from the imap server...

two possible approaches - 1. Make the code that checks if we have a message offline also check that the message looks OK in the offline store before getting into the imap protocol code. 2. Annotate the imap url such that it knows it shouldn't fall back to fetching from the server; in other words, store the aLocalOnly argument to StreamMessage in the imap url, and check that before fetching from the server.

I have #2 approach coded up, but propagating this error back gracefully is proving challenging.

This adds an attribute to imap urls that prevents them from doing an online fetch, and makes StreamMessage set that attribute. The tricky part was propagating this back to nsMsgFolderCompactor - I had to send an OnStopRequest directly, instead of using the nsImapCacheStreamListener, which we don't create in this case. Then, I made the folder compactor clear the offline flag on the header if streaming failed. I made compact continue on in this case because in theory some messages might have the wrong offset into the offline store, but later ones could be correct. However, to test this, I just remove some lines from the second message in the offline store, and restarted and did a compact. I suppose a different test would be to tweak the bytes of the "From " line of an offline store message w/o changing the size of the offline store, so just the one message would seem corrupted.

Comment on attachment 404931 [details] [diff] [review] proposed fix >- nsresult rv = status; >+ nsresult rv; > nsCOMPtr<nsIURI> uri; > nsCOMPtr<nsIMsgDBHdr> msgHdr; > nsCOMPtr<nsIMsgDBHdr> newMsgHdr; > nsCOMPtr <nsIMsgStatusFeedback> statusFeedback; > ReleaseFolderLock(); > >+ // This particular error should allow us to continue, so we check >+ // for it specifically and don't terminate the compaction. >+ if (status != NS_MSG_ERROR_MSG_NOT_OFFLINE) >+ rv = status; > if (NS_FAILED(rv)) goto done; Not sure this is quite what you meant - if the message is not offline then rv will still be uninitialised at this point. Maybe you want to use nsresult rv = status; ... if (NS_FAILED(rv) && rv != NS_MSG_ERROR_MSG_NOT_OFFLINE) goto done;

fix checked in, with suggested change, and a correction to the comment about sending OnStart/StopRequest.