798659 - Improve Efficiency of nsImapMailFolder::GetOfflineMsgFolder by not checking "\\All Mail", "\\Trash" or "\\Spam" labels because they are never returned by the server

Status: NEW

(MailNews Core :: Networking: IMAP, defect)

Description

[David Lechner (:dlech)]

We don't need to check for "\\All Mail", "\\Trash" or "\\Spam" labels because they are never returned by the server. It is assumed that all messages are in one of these three folders exclusively.
    
In fact, you could add some additional conditions here...
- If the current folder is the Trash folder or the Spam folder, then skip
  iterating the labels because if a message is in Trash or Spam, it still
  has the labels of other folders, but will not actually appear in any other
  folder
- If the current folder is not Trash or Spam or All Mail, then also check
  the All Mail folder for the offline copy of the message.

Link to related source code:
https://mxr.mozilla.org/comm-central/ident?i=GetOfflineMsgFolder&tree=comm-central&filter=nsImapMailFolder

Comment 1

[David Lechner (:dlech)]

Forgot to mention this is follow up to item 6 in https://bugzilla.mozilla.org/show_bug.cgi?id=721316#c60

Comment 2

[Atul Jangra [:atuljangra]]

Attachment: introductory patch

Have included the two changes suggest by Dlech.
Dlech, can you please have a look on this patch and tell me if it's okay. :-)

Comment 3

[David Lechner (:dlech)]

(In reply to Atul Jangra [:atuljangra] from comment #2)
> Created attachment 695462 [details] [diff] [review]
> introductory patch
> 
> Have included the two changes suggest by Dlech.
> Dlech, can you please have a look on this patch and tell me if it's okay. :-)

I have updated my Gmail Buttons extension to look at the offline storage location. So starting with TB 18.0b1 we can easily see what is going on in real life usage. I will follow up when I have had time to use it to check out your patch.

Comment 4

[David :Bienvenu]

Comment on attachment 695462 [details] [diff] [review]
introductory patch

this

+        if (mFlags & (nsMsgFolderFlags::Junk | nsMsgFolderFlags::Trash))
+          break;

seems to make part of this

+          if (!(mFlags & (nsMsgFolderFlags::Archive | nsMsgFolderFlags::Junk | nsMsgFolderFlags::Trash)))
+          {

redundant.

But I suspect that David meant for you to put those first two lines at the end of the for loop, not the beginning.

Comment 5

[WADA:World Anti-bad-Duping Agency]

Is "check of X-GM-LABEL of each UID in each Gmail IMAP Mbox(==Gmail Label)" mandatory for current mechanism of IMAP access in Tb?

(1) Because "UID of each mail in each Gmail Label(==Gmail IMAP Mbox)" is required, "uid 1:* fetch Flags" or "uid highgest_UID:* flags" is mandatory for current implementation.
Gmail Label for this selected Mbox name is not set in X-GM-LABEL.
Mbox name set in select command == Gmail Label of this selected Mbox.
(2) Once UID is known, "uid fetch X-GM-MSGID" can be used.
(3) Once "X-GM-MSGID for an UID in an Gmail IMAP Mbox" is known, it can be used forever, unless Gmail changes X-GM-MSGID and UID.
(4) Gmail shows an original mail in Gmail to IMAP client via one of
    (a) [Gmail]/All Mail, (b) [Gmail]/Trash, (c) [Gmail]/Spam,
    and mail in (a) is shown as a mail in Gmail Label==Gmail IMAP Mbox.
(5) So, a mail data can be identified like next:
    [Gmail]/All Mail, X-GM-MSGID = m1, UID in [Gmail/All Mail = UID-m1
      Mbox=Gmail Label=Mbox_A, X-GM-MSGID=m1, UID=UID-Mbox_A
                            |                              | 
      Mbox=Gmail Label=Mbox_Z, X-GM-MSGID=m1, UID=UID-Mbox_Z
(6) Currently, unique identifier of a mail in an Mbox is UID in the Mbox.
(7) So, access like following is possible.
    Mbox name(==Gmail Label) + UID in the Mbox => X-GM-MSGID
    => UID in [Gmail]/All Mail => mail data for X-GM-MSGID
(8) If "uid xx fetch header", "uid xx fetch body[]" etc. is executed at [Gmail]/All Mail only, there is no need to check X-GM-LABEL of each UID in each Mbox. And, if "mail data save in offline-store file" is executed for [Gmail]/All Mail only, there is no need to hold multiple copy of originally same mail if Gmail IMAP.

Is fetch of X-GM-LABEL and check of X-GM-LABEL mandatory?
I think that X-GM-LABEL check is mandatory only when "no IMAP access via IMAP MBox(=Gmail Label)", "IMAP access via [Gmail]/All Mail, [Gmail]/Trash, [Gmail]/Spam only", will be implemented.

Comment 6

[Wayne Mery (:wsmwk)]

Size of gmail folder has not impact here, correct?

(not sure I fully understand the import/impact of comment 1)

Comment 7

[David Lechner (:dlech)]

(In reply to Wayne Mery (:wsmwk) from comment #6)
> Size of gmail folder has not impact here, correct?

I suppose it is possible, but hard to tell without doing some kind of profiling. The affected code here is doing a lookup to try to find a matching message id in the header database for each label. I don't know how fast or slow this is. The changes that I suggested would eliminate the lookups for messages in Trash and Spam, but these are not that many messages. If you added looking in All Mail, the it could actually make things worse cpu wise (have to parse All Mail for each message), but better download wise (fewer duplicate emails will be downloaded).

> 
> (not sure I fully understand the import/impact of comment 1)

In the whole scheme of things, it is probably not that important. In fact, I am not sure that I fully understand it either - it's been too long :p

Comment 8

[Wayne Mery (:wsmwk)]

in addiiton to profiling it would be nice to know what % of users have gmail, and a profile of their folder sizes.  If they are anything like my mom, All Mail is a big folder ...

Comment 9

[WADA:World Anti-bad-Duping Agency]

(In reply to David Lechner (:dlech) from comment #0)
> We don't need to check for "\\All Mail", "\\Trash" or "\\Spam" labels because they are never returned by the server.
> It is assumed that all messages are in one of these three folders exclusively.

You are right. So, there is no need to check about GMLBEL named \AllMail, \Trash, \Spam in X-GM-LABEL response which is stored in StringProperty of each msgHdr. 
Further, mapping of "a GMLABEL of \Spam in X-GMLBELS" not always equal to "a mail of the MSGID is held in special folder of folderFlag=Junk".
     "GMLABEL of \Spam in X-GMLBELS of a mail" = Gmail stores copy of the mail of the MSGID in Gmail IMAP Mbox of GMLABEL==\Spam.
     "Gmail IMAP Mbox of GMLABEL==\Spam" == folder which has "kImapSpam bit is on"  in msgFolder.boxFlags.
      "kImapSpam bit  in msgFolder.boxFlags" is set based on \Spam label in XLIST response..
Although Tb's default of Junk folder(folder of Junk bit of msfFolder.flag=On)   is "Mbox of \Spm label in XLIST response", user can change "Junk folder in Tb" by Copies & Folders setting. And, because "Junk bit of msgFolder.flag" is baased on Copies&Folders settig, other ccount's identty can use This Gmail aaccouts any Mbox(==Gmail Label). i.e. Multiple Junk folder(Junk bit of msgFolder.flags=On).

Please clearly isolate "Junk folder in Tb"(Junk bit of msgFolder.flag) and "Mbox of \Spam label in Gmail"(kImapSpam bit of msgFolder.boxFlags).
Please don't confuse "SpecialUSE of msgFolder.flag" and "Gmail's label in XLIST response => a bit in msgFolder.boxFlags".

Because purpose of the code is to check whether the MSID is already. auto-sync'ed by "auto-sync + Offline-Use=On" at other folder, what is needed is;
     Check other folders, which is listed in X-GM-LABELS of a new mail at an Mbox(Gmail Label)
Because any non-\Trash/non-\Spam mail is held in \AllMail folder, and because first detection of a new MSGID in Gmail account is done at Inbox(\Inbox folder), I think "Check \AllMail folder, and fetch/auto-sync at \AllMail folderf  not auto-sync'ed" is best practice.
Please note that no X-GM-LABELS is usually returned to new mail in Inbox, because GMLABEL of this Mbox(\Inbox for Inbox folder) is not returned from Gmail IMAP and \AllMail is never returned from Gmail server.
"X-GM-LABELS of newly arrived multiple mails for single MSGID(not uploaded mail, not copied mail)  occurs only when mail is copied by server side filter.
   1. New mail of MSGID=mmmm arrives at Gmail => put in [Gmail]/All Mail, GMLABE=\Inbox is added
   2. Server side filter adds Gmail Label(call GMLABEL-X)
   3. New mail at Inbox                : X-GM-MSGID mmmm  X-GM-LABELS (GMLABEL-X)
       New mail at [Gmail]/All Mail : X-GM-MSGID mmmm  X-GM-LABELS\(Inbox GMLABEL-X)
       New mail at GMLABEL-X       : X-GM-MSGID mmmm  X-GM-LABELS\(Inbox)
So, check \AllMail folder, then if \Inbox in X-GM-LABELS, check Inbox, then check other GMLABEL in X-GMLABEL, ...like order is better.

Following is actual X-GM-LABELS data of an MSGID in [Gmail]/All Mail which is generated by intentional test.
This can occur by; a new message arrives, server side filter adds all other GMLABELs.
Same X-GM-LABELS is returned to a new mail in an Mbox(Gmail Label fo FolderX) if this mail is copied to FolderX.
> X-GM-LABELS
>  "INBOX/AA ( ) BB" "INBOX/AA BB" "INBOX/AA \" \\ BB" "INBOX/AA\"\\BB" "[Imap]/Drafts"
>  "[Imap]/Drafts/ABC" "[Imap]/Important" "[Imap]/Important/ABC" "[Imap]/Sent Mail" "[Imap]/Sent" "[Imap]/Spam"
>  "[Imap]/Starred" "[Imap]/Trash/ABC" "\\Drafts" "\\Important" "\\Inbox" "\\Sent" "\\Starred"
>  "\\\\Important" INBOX/&ZeVnLIqe-

Note:
   GMLABEL = [Imap]/Spam             => top level Spam in Thunderbird
   GMLABEL = [Imap]                        => top level [Imap] in Thunderbird
   GMLABEL = [Imap] /Spam/ABC    => [Imap] /Spam/ABC  in Thunderbird ([Imap] /Spam is \Noselect mbox for Tb as it's not returned to LIST)
   Top level \Important in Tb can be created > User defined Gmail Label named \Importaant is generated by Gmail.
   So, if user creates top level \Important in Tb, there is no way to know "GMLABEL=\Important of a MSGID" is [Gmail]/Important or \Important.

I don't think each GMLABEL handling upon new mail fetch is needed.
In any case, I think following is sufficient for your purpose.
   check \AllMail folder(never listed in X-GM-LABELS) first, 
   then, if \Inbox is found in X-GM-LABELS, check \Inbox folder(always Mbox named Inbox because of IMAP).
   If \AllMail folder is not Offline-Use=Off and the MSGID is not held in Inbox,
   and if other GMLABELS corresponds to Offline-Use=On folder, search the folder for the MSGID, aand if found, use it.
This process can be used for Offline-use=Off folder if there is Offline-Use=On folder. If a MSGID is auto-sync'ed at an Mbox, same MSGID in Offline-Use=Off folder can use already downloaded data for the MSGID in any Offline-Store file.

So, if table like following is maintained, there is no need of X-GM-LABEL upon new mail fetch.
    MSGID=aaaa
        GMLABELtable[m] = GMLABEL#M(==MboxM in Tb), UID=uu, auto-sync'ed at this Mbox or not
        GMLABELtable[n] = GMLABEL#N(==MboxN in Tb), UID=vv, auto-sync'ed at this Mbox or not
    MSGID=bbbb
        GMLABELtable[p] = GMLABEL#P(==MboxP in Tb), UID=ww, auto-sync'ed at this Mbox or not
        GMLABELtable[q] = GMLABEL#Q(==MboxQ in Tb), UID=xx, auto-sync'ed at this Mbox or not
    If new mail with MSGID=aaaa is detected at MboxX in Tb, and if auto-sync'ed as GMLABEL#N(==MboxN in Tb), UID=vv, use it.
Any Mbox whio can do auto-sync does do auto-sync. Any mail of an MSGID in any Mbox can use already downloaded data for the MSGID in any Mbox.

Comment 10

[WADA:World Anti-bad-Duping Agency]

Comment on attachment 695462 [details] [diff] [review]
introductory patch

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

Some comments on current logic of nsImapMailFolder::GetOfflineMsgFolder() for Gmail IMAP.

(1) "GMLABEL corresponds to Mbox where the mail is contained" is not returned as X-GM-LABELS response to uid fetch "X-GM-LABELS". So, "A GMLABEL  in X-GM-LABELS response" means "already held Mbox when the mail is added to this Mbox" if this is newly detected at this Mox. So, "trying to check offline-store of Mbox correspnds to the GMLABEL" itself is correct action.
However, code like following is not always correct.
> 9671           if (labelNames[i].Equals("\"\\\\All Mail\""))
> 9672              rv = rootFolder->GetFolderWithFlags(nsMsgFolderFlags::Archive,
> 9673                                                  getter_AddRefs(subMsgFolder));
(A) nsMsgFolderFlags::Archive of msgFolder.flag is set by choosing a folder as archive folder at Copies&Folders setting, and nsMsgFolderFlags::Archive of previously selected arcive folder is removed. So, if user chooses other than [Gmail]/All Mail as archive folder, GetFolderWithFlags(nsMsgFolderFlags::Archive) won't return [Gmail]/All Mail.
(B) Any identity including identity of other account can use different Mbox from [Gmail]/All Mail as archive folder. So, multiple Mboxes under this Gmail IMAP account can have nsMsgFolderFlags::Archive in msgFolder.flag. GetFolderWithFlags() perhaps scans folders from top to bottom, left to right. If so, if other than [Gmail]/All Mail is found first by scan, [Gmail]/All Mail won't be returned. Because multiple folders can have a special folder flag, at least GetFoldersWithFlags() should be used except for Inbox and Trash(one folder only is guranteed in Tb, except in special situation like existence of Inbox/Trash).
This is applicable to all Copies&Folders folders and folder set by Junk Settings.

If Mbox corresponds to \AllMail is needed, "msgFolder.boxFlags of all [Gmail]/xxx folders" currently should be scanned.
Because \AllMail is curently never returned from Gmail IMAP to any "uid fetch nn X-GM-LABELS" for any Gmail IMAP folder, code for "\AllMail in X-GM-LABELS" is currently useless. Check of \AllMail is needed for XLIST response only.
"Adding \AllMail(or\Trash,\Spam) to StringProperty value" may be good for coding, because "this folder is [Gmail]/All Mail or [Gmail/Trash or [Gmail]/Spam" becomes irrelevant in GMLABEL prosessing by existence of \AllMail(or\Trash,\Spam).
Following is applicable always.
  If X-GM-LABELS contains \AllMail(or \Trash,  \Spam), this MSGID is held in \AllMail folder(or \Trash folder, \Spam folder).

If "Mbox corresponds to \AllMail first, then try Inbox(lways top level Inbox although case sensitive) second"  is added to top of logic of GetOfflineMsgFolder(), I think current code of "scan all folder corresponds to all GMLABELs in X-GM-LABELS" will not produce performnce issue, because I don't think very many GMLABELS are added to a MSGID in real world.
   To add GMLABEL using Tb, user has to do "copy mail", To add GMLABEL using Gmail Web, user has to do action of
   "Add Label", and usability of Gmail Web UI is not so good aand efficient even though it's not so bad.

Comment 11

[WADA:World Anti-bad-Duping Agency]

FYI.
Image of "Table of MSGIDs in [Gmail]/All Mail(and [Gmail]/Trash, [Gmail/Spam] )by JavaScript Object
Because JavaScript Object is associative array, correction of JavaScript Object == simple Databse.

1. From msgHdr and msgHdr.getStringProperty("X-GM-LABELSl") of all mails in [Gmail]/All Mail, following table(Databse) can be pretty easily generated. Current GetOfflineMsgFolder() can do this kind of job with minimum modification or minimum enhancement.
> GM_MSGID_TBL = { 
>     [1487362127576781862] = { 
>         [MSGID] = 1487362127576781862
>         [THRID] = 1487362127576781862
>         [UID_Label] = \AllMail
>         [UID_Owner] = [Gmail]/All Mail
>         [UID] = 3726
>         [X-GM-LABELS] = { 
>             [\AllMail] = "INBOX/AA ( ) BB" "INBOX/AA BB" "INBOX/AA \" \\ BB" "INBOX/AA\"\\BB" "[Imap]/Drafts" "[Imap]/Drafts/ABC" "[Imap]/Important" "[Imap]/Important/ABC" "[Imap]/Sent Mail" "[Imap]/Sent" "[Imap]/Spam" "[Imap]/Starred" "[Imap]/Trash/ABC" "\\Drafts" "\\Important" "\\Inbox" "\\Sent" "\\Starred" "\\\\Important" INBOX/&ZeVnLIqe-
>         }
>         [LABELS] = { 
>             [\AllMail] = \AllMail
>             [INBOX/&ZeVnLIqe-] = INBOX/&ZeVnLIqe-
>             [INBOX/AA " \ BB] = INBOX/AA " \ BB
>             [INBOX/AA ( ) BB] = INBOX/AA ( ) BB
>             [INBOX/AA BB] = INBOX/AA BB
>             [INBOX/AA"\BB] = INBOX/AA"\BB
>             [[Imap]/Drafts] = [Imap]/Drafts
>             [[Imap]/Drafts/ABC] = [Imap]/Drafts/ABC
>             [[Imap]/Important] = [Imap]/Important
>             [[Imap]/Important/ABC] = [Imap]/Important/ABC
>             [[Imap]/Sent] = [Imap]/Sent
>             [[Imap]/Sent Mail] = [Imap]/Sent Mail
>             [[Imap]/Spam] = [Imap]/Spam
>             [[Imap]/Starred] = [Imap]/Starred
>             [[Imap]/Trash/ABC] = [Imap]/Trash/ABC
>             [\Drafts] = \Drafts
>             [\Important] = \Important
>             [\Inbox] = \Inbox
>             [\Sent] = \Sent
>             [\Starred] = \Starred
>             [\\Important] = \\Important
>         }
>     }
> }

2. Once such table(database) for "MSGID, GMLABEL in X-GM-LABELS of each mail, UID of each mail in [Gmail]/All Mail" is generted,
"re-costructinon of table(database) like next for each GMLABEL"(==Mbox in Gmail IMAP) is also pretty easy.
> GMLABEL_TBL = { 
>     [Drafts] = { 
>         [GMLABEL] = [Imap]/Drafts
>         [MSGID->UID_in_[Gmail]/All Mail] = { 
>             [1487362140221708716] = 3724
>             [1487362127576781862] = 3726
>             [1252049043496444727] = 3730
>         }
>         [UID_in_[Gmail]/All Mail->MSGID] = { 
>             [3724] = 1487362140221708716
>             [3726] = 1487362127576781862   <------+
>             [3730] = 1252049043496444727   <------+
>         }                                                                       |
>     }                                                                           |  same maill.
>     [[Gmail]/Sent Mail] = {                                        |  user copied 2 mails in [Gmail]/Sent Mail 
>         [GMLABEL] = \Sent                                          |    to top level Drafts in Tb 
>         [MSGID->UID_in_[Gmail]/All Mail] = {            |    in order to use as draft mail
>             [1487362127576781862] = 3726   <------ +
>             [1252049043496444727] = 3730  < ------ +
>         }
>         [UID_in_[Gmail]/All Mail->MSGID] = { 
>             [3726] = 1487362127576781862
>             [3730] = 1252049043496444727
>         }
>         [Warning] = { 
>             [TopLevelFolderExists] = Top level folder named \Sent is also defined by user
>         }
>     }
> }

If X-GM-LABELS of each mail in [Gmail]/All Mail is correctly maintained, this is absolutely same as "msgDatabase of an Mbox of Gmail IMAP" == collection of "msgHdr with key=messgeKey==UID, with a GMLABEL in StringProperty(X-GM-LABELS) virtually(Mbox name==associated GMLABEL)".
except UID in this context is "UID of a mail for the MSGID in [Gmail]/All Mail", except for top level Mbox with same name as reserved GMLABEL such as \Spam, \Sent, \Starred, \Important.
Required data is only "msgHdr.messageKey + data of X-GM-LABELS" of all mails in [Gmail]/All Mail.

If this kind of table(database) is created, Thunderbird can store "entire mail data of any MSGID" in "any Offline-Store file for any Mbox in Gmail IMAP", and Thunderbird can use "entire mail data stored in any Offline-Store file for any Mbox in Gmail IMAP" for an MSGID. Needless to say, "only one copy of locally held entire mail data for an MSGID" is always possible by pretty simple database obtained from "msgHdr.messageey + associated X-GM-LABELS in [Gmail]/All Mail" only.

Comment 12

[WADA:World Anti-bad-Duping Agency]

Attachment: An idea about enhancement of concept of nsImapMailFolder::GetOfflineMsgFolder for "reducing duplicated copy in offline-store"

Attached is based on following.

(1) If table like next is created from X-GM-MSGID and X-GM-LABELS at [Gmail]/All Mail, [Gmail]/Trash,  [Gmail]/Spam,
>     GM_MSGID_TBL[2222222222]["MSGID"] = 2222222222
>                             ["Owner"]["Mbox"]  = [Gmail]/All Mail
>                             ["Owner"]["Label"] = \AllMail
>                             ["Owner"]["UID"]   = 2222
>                             ["LABELS"][\AllMail]   = true
>                             ["LABELS"][\Sent]      = true
>                             ["LABELS"][ other GMLABELs ] = true
(2) table like next is pretty easily obtained,
>       GM_LABEL_TBL[ \Sent ]["GMLABEL"]   = \Sent
>                                              ["Mbox"]      = [Gmail]/Sent Mail                   
>                                              ["MSGID->UID"][3333333333] = 3333    (["Owner"]["Mbox"] = [Gmail]/Trash)   
>                                                                         [2222222222] = 2222    (["Owner"]["Mbox"] = [Gmail]/All Mail)
>                                              ["UID->MSGID"][3333] = 3333333333    (["Owner"]["Mbox"] = [Gmail]/Trash)   
>                                                                         [2222] = 2222222222    (["Owner"]["Mbox"] = [Gmail]/All Mail)
>       GM_Mbox_TBL[ [Gmail]/Sent Mail ]["GMLABEL"]    = \Sent
>                                                                ["Mbox"]      = [Gmail]/Sent Mail                   
>                                                                ["MSGID->UID"][3333333333] = 3333    (["Owner"]["Mbox"] = [Gmail]/Trash)   
>                                                                                           [2222222222] = 2222    (["Owner"]["Mbox"] = [Gmail]/All Mail)
>                                                                ["UID->MSGID"][3333] = 3333333333    (["Owner"]["Mbox"] = [Gmail]/Trash)   
>                                                                                           [2222] = 2222222222    (["Owner"]["Mbox"] = [Gmail]/All Mail)
(3) and, above GM_Mbox_TBL is same as set of "msgFolder.URL of a folder" + "list of all msgDBHsr.messageKey of the msgFolder", except differece of "key is MSGID in GM_Mbox_TBL but key is messageKey==UID in another".

Comment 13

[WADA:World Anti-bad-Duping Agency]

Attachment: A idea (v2) about enhancement of concept of nsImapMailFolder::GetOfflineMsgFolder for "reducing duplicated copy in offline-store"

v2. typo is corrected, some parts are re-written.

Comment 14

[WADA:World Anti-bad-Duping Agency]

(In reply to Atul Jangra [:atuljangra] from comment #2)
> introductory patch
>          rv = GetRootFolder(getter_AddRefs(rootFolder));
> +        // If current folder is Spam or Trash, then we don't want to check other labels.
> +        if (mFlags & (nsMsgFolderFlags::Junk | nsMsgFolderFlags::Trash))
> +          break;

If this kind of decision is made, it should be one like next:
>   if(      ( msgFolder_of_currentFolder["boxFlags"] & boxFlagTable[ "kImapXListTrash" ] )
>       ||  ( msgFolder_of_currentFolder["boxFlags"] & boxFlagTable[ "kImapSpam" ] )        )
>       { return null; } // no need to check other folder's Offline-store file when \Trash folder or \Spam folder
>   else if( msgFolder_of_FolderWhoHas_kImapAllMail_flag["flags"] & Components.interfaces.nsMsgFolderFlags["Offline"]  )
>         { 
>              if( msgFolder_of_currentFolder == msgFolder_of_FolderWhoHas_kImapAllMail_flag )
>                 { return null; } // I am \AllMail folder, and Offline-Use=On
>              // I am other than \AllMail, \Trash, \Spam folder && \AllMail folder is Offline-Use=On
>              else { return msgFolder_of_FolderWhoHas_kImapAllMail_flag ; } 
>         } 
>   else // \AllMail folder is not Offline-Use=On
>   {
>        // I am \AllMail folder && I am not Offline-Use=On folder
>        if( msgFolder_of_currentFolder == msgFolder_of_FolderWhoHas_kImapAllMail_flag )
>        { 
>              // Search appropriate Offline-Use=On folder. Check \Inbox folder first because probability of Offline-Use=On is usually high.
>              // Because X-GM-FLAGS is "folder who has copy of this MSGID when this MSGID is added to this folder as mail of new UID",
>              // check "folders in X-GM-LABELS" and "Offline-Use=On/Off of the folder" is best practice.
>              // But "do nothing" is also a good answer for "\AllMail folder", because user intentionally sets Offline-Use=Off for \AllMail folder 
>        }
>        // I am other than \AllMail, \Trash, \Spam folder, but \AllMail folder is not Offline-Use=On
>        { 
>              // Search appropriate Offline-Use=On folder including me. Check \Inbox folder first.
>              // Because X-GM-FLAGS is "folder who has copy of this MSGID when this MSGID is dded to this folder as mail of new UID",
>              // check "folders in X-GM-LABELS" and "Offline-Use=On/Off of the folder" is best practice.
>              // Even if this folder is Offline-use=Off, if other Gmail Label folder is Offline-Use=On,
>              // entire mail data held in other folder's Offline-store file can be used.
>         }
>
> To do abobe job efficiently, following is needed.
>    GM_SpecialLabel_TBL ={}
>    GM_SpecialLabel_TBL[ \AllMail ]["MboxName"] = "[Gmail]/All Mail "  ("All Mail" can be localized)
>                                                        ["OfflineUse"]   = true/false (obtained from msgFolder.flags)
>    GM_SpecialLabel_TBL[ \Trash   ] "MboxName"] = "[Gmail]/Trash"      ("Trash" can be localized)
>                                                        ["OfflineUse"]   = true/false (obtained from msgFolder.flags)
>    GM_SpecialLabel_TBL[ \Spam  ]["MboxName"] = "[Gmail]/Spam"     ("Spam" looks always "Spam")
>                                                        ["OfflineUse"]   = true/false (obtained from msgFolder.flags)
>    GM_OffLineUseOnFolder_TBL ={}
>    GM_OffLineUseOnFolder_TBL[ \Inbox          ]["MboxName"] = "Inbox"                       (may be "INBOX" in LIST response)
>    GM_OffLineUseOnFolder_TBL[ \Starred       ]["MboxName"] = "[Gmail]/Starred "      ("Starredl" can be localized)
>    GM_OffLineUseOnFolder_TBL[ \Important   ]["MboxName"] = "[Gmail]/Important"   ("Starredl" can be localized)
>    GM_OffLineUseOnFolder_TBL[ \Sent            ]["MboxName"] = "[Gmail]/Sent Mail"    ("Sent Mail" can be localized)
>    GM_OffLineUseOnFolder_TBL[ [Imap]/Sent ]["MboxName"] = "Sent"
>    GM_OffLineUseOnFolder_TBL[ [ user defined Gmail Label ]["MboxName"] = "user defined Gmail Label "
If JavaScript, simple code like next can be used to find first candidate folder.
>     if(  (GM_LABEL_forMe=="\Trash" || GM_LABEL_forMe=="\Spam" ) return null;
>     else if(GM_SpecialLabel_TBL["\AllMail"][OfflineUse] ) 
>     { 
>          if(GM_LABEL_forMe=="\AllMail")  return null;
>          else return  GM_SpecialLabel_TBL["\AllMail"]["MboxName"]  ;
>     }
>     else
>     { 
>          if(GM_LABEL_forMe=="\AllMail")  { ... }
>          else { ... }
>     }
> 
>     for(var LABEL in X-GM-LBELS) {
>        if(GM_OffLineUseOnFolder[LABEL]){
>            This is Offline-Use=On folder who holds copy of this MSGID;
>            return  GM_OffLineUseOnFolder[LABEL]["MboxName"]  ;
>        }
>    }

Comment 15

[WADA:World Anti-bad-Duping Agency]

FYI.
Because we already have nsImapMailFolder::GetOfflineMsgFolder, following is possible with minimum changes.
   1. Offline-Use=On folder is [Gmail]/All Mail, [Gmail]/Trash, [Gmail]/Spam only
       => "duplicated copy in Offline-Store file" is not generated by auto-sync
   2. nsImapMailFolder::GetOfflineMsgFolder returns [Gmail]/Trash or [Gmail]/Spam for [Gmail]/Trash or [Gmail]/Spam.
       nsImapMailFolder::GetOfflineMsgFolder returns [Gmail]/All Mail for [Gmail]/All Mail and any Gmail Label.
   3. Regardless of Offline-Use=On/Off of folder for Gmail Label,
       tries to use entire mail data of the MSGID in Offline-store file of [Gmail]/All Mail.
       If the MSGID is not auto-sync'ed yet at [Gmail]/All Mail, request "uid fetch NN BODY.PEEK[]" at [Gmail]/All Mail and wait for fetch.
       If "waiting for fetch completion at [Gmail]/All Mail" is hard, request fetch at folder for Gmail Label as currently done.
       Because Offline-Use=Off folder, data is stored in Disk Cache.

Comment 16

[WADA:World Anti-bad-Duping Agency]

FYI.
Essentilly, nsImapMailFolder::GetOfflineMsgFolder can be used upon fetch header when Gmail IMAP.
   Crrent : 
          new mail detection -> uid fech NN Flags -> uid fetch NN  X-GM-MSGID Flgs BODY.PEEK[HEADER.FIELDS(...)]
          uid fech NextID:* Flags -by Biff -> if UID is returned, uid fetch NN Flgas X-GM-MSGID BODY.PEEK[HEADER.FIELDS(...)]
   ~> Change to "uid fech NN Flags  Flgas X-GM-MSGID X-GMLABELS" when "fetch Flas" is used
         If \Trash/\Spam/\AllMil folder, continue process.
         Of other than \Trash/\Spam/\AllMil folder,  copy msgDBHdr data from rdatafor mil of the MSGID.
         => no need to fetch header at Gmail Label folder, no need to parse header at  Gmail Label folder.

There are 3 options. 
(1) No server access if Gmail Label folder. Always use data of \AllMail folder.  (UID at each Gmail Label folder is not usable)
(2) uid fetch NextID:* Flags X-GM-MSGID X-GM-LABELS by Biff 
      => By calling nsImapMailFolder::GetOfflineMsgFolder, use data of \AllMail folder. If msgDBHdr data, copy except messageKey(==UID).
(3) Use nsImapMailFolder::GetOfflineMsgFolder for getting message body data, as done currently.
I think (1) is perhaps hard, but I believe (2) is possible with minimum change, because we already have nsImapMailFolder::GetOfflineMsgFolder and we can alredy use X-GM-MSGID.

Comment 17

[Wayne Mery (:wsmwk)]

Gene, do you have short take on this, and the patch in comment 10?
In short, is it worth adding (or simplifying) code?

Comment 18

[Wayne Mery (:wsmwk)]

(Comment 9 about server side filters is interesting)

Comment 19

[gene smith]

I've never before looked at how these special gmail flags work, so this is new to me.
On my gmail account I now have only 1 folder that uses offline store called "nf". I copied 3 messages from nf to a folder called "temp" so these 3 messages in temp have labels "nf" and "temp" and some also have label \INBOX since they originated there I assume. When I open one of the messages in temp the code still thinks temp has offline store until I repair temp then it doesn't. Anyhow, when I open one of the messages again in temp, no offline store is found. It should find the messages stored in mbox for nf.

The bug causing it to not find nf offline store is that subMsgFolder needs to be set back to nullptr at the end of the loop before checking the next label for offline. When I do that it works OK and the messages copied to temp from nf now use the nf offline store when opened in temp (temp itself has no offline store).

However, still need to see why the original copy from nf to temp left the offline flags set and have to repair it to clear them.

Back to the subject of this bug -- Since gmail never returns \All Mail, \Spam and \Trash labels, looks like the checks for those could be eliminated. However those checks look fairly quick so not sure it would really affect performance. What I'm seeing is the function that does the check is called hundreds of time when a message is opened and don't know why. That seems like a bigger performance concern.

Comment 20

[Wayne Mery (:wsmwk)]

(In reply to gene smith from comment #19)

...
Back to the subject of this bug -- Since gmail never returns \All Mail, \Spam and \Trash labels, looks like the checks for those could be eliminated. However those checks look fairly quick so not sure it would really affect performance. What I'm seeing is the function that does the check is called hundreds of time when a message is opened and don't know why. That seems like a bigger performance concern.

Do we have a bug report for that "bigger concern"?

Comment 21

[gene smith]

Do we have a bug report for that "bigger concern"?

No other report for that. I did do some work on this back in Sept '23 and put it on "hg shelve" and haven't looked at it since. Don't remember the details at this time, but still have the patch. So I'll leave the NI open and look at this again ASAP.

Comment 22

[gene smith]

Attachment: gmail-efficiency-wip-patch.diff

Here's most of the work I did on this a while back (Sept `23). I still need to work on thisl
Note to self: this is from ~/mozilla2/comm/.hg/shelved/cs-qr-02.patch which may contain more stuff relevant to this.