471695 - Select Offline -> Download / Sync Now doesn't go offline (not downloading all emails)

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Select Offline -> Download / Sync Now doesn't go offline on Mac latest nightly, Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20081231 Lightning/1.0pre Shredder/3.0b2pre

It downloads all files but the icon on the bottom left doesn't turn to an offline one.

Comment 1

[Wayne Mery (:wsmwk)]

also doesn't go offline via download /sync in windows 20081230. i can't tell what, if anything is downloaded - progress indicator just spins and status line cites accessing first non-special folder.

but "work offline" works straight off.

Comment 2

[Mark Banner (:standard8)]

Just to clarify:

- if I download all files then it won't go offline.
- if I don't download all files, it goes offline ok.

Comment 3

[Wayne Mery (:wsmwk)]

happens at least as far back as 20081209, which I see on my vista box that hasn't had anything newer installed than 20081209.

in contrast to comment 2, for me offline>work offline works regardless

Comment 4

[David Ascher (:davida)]

If i uncheck the mail/news/send messages checkbox but do check the 'go offline when done', then it _does_ go offline.

if i check the mail checkbox, then it seems to never end downloading stuff, but I'm not sure if that's just because there's a lot to do.

Comment 5

[David Ascher (:davida)]

Figuring this out seems blocking.  We probably want to wait until the current offline/osx bug is fixed in gecko and we flip the switch back there, to avoid confusing things further.

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Putting in mailnews core, as this seems likely to be backend related.  Since Standard8 has been keeping up with the various OS X bugs, reassigning to him.

Comment 7

[Karsten Düsterloh]

Erm, guys, this is NOT a bug, this is how it is supposed to work for years!
I'm using "Offline -> Download / Sync Now" to sync my (locally stored) newsgroups with the server and I'd pretty pissed if MailNews would put me into offline state after that! I still need to be online to continue getting mail/rss, etc. pp...

Comment 8

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to comment #7)
> Erm, guys, this is NOT a bug, this is how it is supposed to work for years!
> I'm using "Offline -> Download / Sync Now" to sync my (locally stored)
> newsgroups with the server and I'd pretty pissed if MailNews would put me into
> offline state after that! I still need to be online to continue getting
> mail/rss, etc. pp...

I'm pretty sure I got confused by this if that's the case - perhaps a naming thing?

Comment 9

[Karsten Düsterloh]

> I'm pretty sure I got confused by this if that's the case - perhaps a naming
> thing?

Probably. 
"Offline -> Work Offline" goes offline, but Download/Sync and Get Flagged/Starred/Selected shouldn't.
After all, Work Offline will trigger Download/Sync anyway, if you have configured your account's offline settings that way.

Comment 10

[Mark Banner (:standard8)]

(In reply to comment #9)
> > I'm pretty sure I got confused by this if that's the case - perhaps a naming
> > thing?
> 
> Probably. 

Ok, here's what I think this bug is really about:

1) "Offline -> Work Offline" and choose not to download
--> goes offline correctly.
2) "Offline -> Work Offline" and choose to download
--> downloads but never goes into offline mode.
3) "Offline -> Download/Sync Now" get a dialog, unselect "Work Offline once download is completed"
--> goes offline corectly.
4) "Offline -> Download/Sync Now" get a dialog, select "Work Offline once download is completed"
--> downloads but never goes into ofline mode.

I know for sure that 2 is definitely an issue, I expect through association and this bug that 4 is an issue as well.

Comment 11

[Jens Müller (:tessarakt)]

Umm, 2) shows a progress indicator saying it downloads, but for me doesn't seem to do anything else.

Comment 12

[Karsten Düsterloh]

> 3) "Offline -> Download/Sync Now" get a dialog, unselect "Work Offline once
> download is completed"
> --> goes offline corectly.

"Goes offline correctly" in the sense that it does NOT go offline, because you unchecked the box?

Comment 13

[Mark Banner (:standard8)]

(In reply to comment #11)
> Umm, 2) shows a progress indicator saying it downloads, but for me doesn't seem
> to do anything else.

Do you mean doesn't download messages, or doesn't go offline? If you mean doesn't go offline, that's what I said.

(In reply to comment #12)
> > 3) "Offline -> Download/Sync Now" get a dialog, unselect "Work Offline once
> > download is completed"
> > --> goes offline corectly.
> 
> "Goes offline correctly" in the sense that it does NOT go offline, because you
> unchecked the box?

Opps, yeah, I wouldn't expect that to go offline.

Comment 14

[Jens Müller (:tessarakt)]

(In reply to comment #13)
> (In reply to comment #11)
> > Umm, 2) shows a progress indicator saying it downloads, but for me doesn't seem
> > to do anything else.
> 
> Do you mean doesn't download messages, or doesn't go offline? If you mean
> doesn't go offline, that's what I said.

Well, it's hard to get a situation where there are messages left to download. There is no download counter or something like this. It just shows the moving activity indicator indefinitely.

Comment 15

[David :Bienvenu]

Historically, the failure to go offline after the download is caused by an error somewhere in the download process - the download doesn't complete, so we error out, and don't get to the code that takes us offline. It's generally profile-specific, e.g., some account we don't like that makes us error out in the iteration process.

Comment 16

[Mark Banner (:standard8)]

(In reply to comment #15)
> Historically, the failure to go offline after the download is caused by an
> error somewhere in the download process

AFAICT history doesn't apply here. What I've got so far:

1) Select Offline -> Work Offline
2) Say yes to download messages for offline use.
3) Offline service does its Get New Mail cycle.
4) Offline service starts the first of the sync on the folders:

-1337147392[1cfd7910]: e19c00:imap.plus.net:S-INBOX:ProcessCurrentURL: entering
-1337147392[1cfd7910]: e19c00:imap.plus.net:S-INBOX:ProcessCurrentURL:imap://user@hostbox/select%3E.INBOX:  = currentUrl
-1337147392[1cfd7910]: e19c00:imap.plus.net:S-INBOX:SendData: 16 noop
-1337147392[1cfd7910]: ReadNextLine [stream=1cfd81cc nb=22 needmore=0]
-1337147392[1cfd7910]: e19c00:imap.plus.net:S-INBOX:CreateNewLineFromSocket: 16 OK NOOP completed
-1337147392[1cfd7910]: e19c00:imap.plus.net:S-INBOX:SendData: 17 getquotaroot "INBOX"
-1337147392[1cfd7910]: ReadNextLine [stream=1cfd81cc nb=28 needmore=0]
-1337147392[1cfd7910]: e19c00:imap.plus.net:S-INBOX:CreateNewLineFromSocket: * QUOTAROOT "INBOX" "ROOT"
-1337147392[1cfd7910]: ReadNextLine [stream=1cfd81cc nb=16 needmore=0]
-1337147392[1cfd7910]: e19c00:imap.plus.net:S-INBOX:CreateNewLineFromSocket: * QUOTA "ROOT"
-1337147392[1cfd7910]: ReadNextLine [stream=1cfd81cc nb=24 needmore=0]
-1337147392[1cfd7910]: e19c00:imap.plus.net:S-INBOX:CreateNewLineFromSocket: 17 OK GETQUOTAROOT Ok.
-1337147392[1cfd7910]: e19c00:imap.plus.net:S-INBOX:SendData: 18 UID fetch 405:* (FLAGS)
-1337147392[1cfd7910]: ReadNextLine [stream=1cfd81cc nb=24 needmore=0]
-1337147392[1cfd7910]: e19c00:imap.plus.net:S-INBOX:CreateNewLineFromSocket: 18 OK FETCH completed.

and stops.

A 2.x build continues with: -1608014048[1109dc0]: offline imap url succeeded :imap://[e-mail]:143/select>.INBOX

As far as I can tell, the OnStopRunningURL callback isn't making it back to the offline service for so it doesn't continue.

As this is now into imap territory I think David will probably spot the error faster than me, so reassigning to him, hope that's ok.

Comment 17

[David :Bienvenu]

(In reply to comment #16)
> (In reply to comment #15)
> > Historically, the failure to go offline after the download is caused by an
> > error somewhere in the download process
> 
> AFAICT history doesn't apply here. What I've got so far:

Not getting the on stop running url is an error in the download process :-) that's definitely been an issue in the past, and I should have called it out specifically.

> > 
> As this is now into imap territory I think David will probably spot the error
> faster than me, so reassigning to him, hope that's ok.

sure, np.

Comment 19

[David :Bienvenu]

Attachment: correctness fix

I'm still working on reproducing this, but looking at the code, we may not be sending the last notification that would take us offline.

Comment 20

[David :Bienvenu]

Attachment: proposed fix

the key change is in nsImapMailFolder.cpp - we weren't notifying the imap protocol code about bodies to download if there were no bodies to download and we were downloading for offline use. This hung the imap thread so that we never got to the stop running url call. I'll look at augmenting the offline download test to catch this.

I also added pause and resume to the autosyncmanager so that it wouldn't kick in while offline download was going on. I also made the autosync manager pay attention to the offline/online notifications, and pause/resume auto sync appropriately.

And I made a couple correctness changes to nsImapOfflineSync that I noticed by code inspection, to make sure that we always send a final notification when done.

Comment 21

[David :Bienvenu]

Attachment: augment test case to detect this

this patch makes the test fail w/o the main patch.

Comment 22

[Mark Banner (:standard8)]

Comment on attachment 375096 [details] [diff] [review]
proposed fix

>+      observerService->RemoveObserver(this, NS_IOSERVICE_OFFLINE_STATUS_TOPIC);
>+      observerService->RemoveObserver(this, NS_IOSERVICE_GOING_OFFLINE_TOPIC);
>+      
>     }

nit: no need for the blank line.

Comment 23

[David :Bienvenu]

fix checked in, along with augmented test case.