Closed Bug 573388 Opened 11 years ago Closed 11 years ago
Web Progress and channel callbacks do not point to the same docshell
I noticed this assertion while reading a message. aWebProgress is the content docShell for the IMAP url. However the channel callbacks are the chrome docShell for the mail window. The assertion was introduced by bug 374577.
The code that does the asserting was introduced by bug 374577, and modified a bit by 479906. Is it your belief that this assertion has been triggered all along, or that this is new?
No, it started recently (say about a week ago).
Would I be correct to assume that this is trunk-only?
Yes, this is both c-c trunk and m-c trunk.
I see this happen if I click on imap messages while autosync is downloading other imap messages in the same folder. I'm wondering if this somehow related to the imap disk cache corruptions and/or offline store corruptions.
Actually, I see this whenever we fetch an imap message from the server for display, as opposed to using the disk cache or memory cache or offline store.
Neil, what do you think of this approach? This makes us use the loading docshell when setting up the notification callbacks, if available, which makes the content policy code less unhappy. I checked that the cert exception override dialog is still displayed when clicking on a message, if there's no cert exception.
Assignee: nobody → bienvenu
Status: NEW → ASSIGNED
Attachment #486805 - Flags: review?(neil)
Given your diagnosis, this is an alternative approach to the problem. What do you think?
Attachment #486906 - Flags: review?(bienvenu)
Comment on attachment 486906 [details] [diff] [review] Alternative approach yes, I thought about that alternative in the middle of the night. It's definitely nicer. I think you want to return an error if we can't get the docshell, instead of always returning NS_OK, and unless this comment isn't valid anymore, it should be propagated to the nsIMsgWindow.idl: - // on the message window docshell. Under no circumstances should you be holding on to - // the docshell returned here outside the scope of your routine. I'll give the patch a try.
Comment on attachment 486906 [details] [diff] [review] Alternative approach r=me, modulo prev comments...
Attachment #486906 - Flags: review?(bienvenu) → review+
Seeking sr because of the API addition to nsIMsgWindow.
Comment on attachment 487475 [details] [diff] [review] Addressed review comments So I kinda prefer David's solution on the fact it doesn't rely on the hard-coded "messagepane" element. However, looking at where GetMessageWindowDocShell is used currently, there's a bigger set of issues to resolve to avoid that hard-coding. Therefore I think we'll take this for now and look at those issues later.
Attachment #487475 - Flags: superreview?(bugzilla) → superreview+
Pushed changeset e9aa02dcfc2d to comm-central.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.