Crash in nsWebShell::DoLoadURL



19 years ago
11 years ago


(Reporter: phil, Assigned: mscott)



Firefox Tracking Flags

(Not tracked)



(1 attachment)



19 years ago
1. Run mail
2. Load a folder and a message
3. Click away from the thread pane, causing the thread pane to lose focus.
4. Click back on the selected message in the thread pane
5. Boom.

I've seen this a couple times, but I can't make it happen at will. Scott says
he's seen it, though, so I'm filing it.

Comment 1

19 years ago
Also, the crash happens at the end of a C++ block scope, so I assume the
destructor of some automatic object (like a COM ptr) is forcing the crash.


19 years ago
Target Milestone: M13

Comment 2

19 years ago
I finally figured out what was going on here. Marking as M13 as this is a random
crasher when displaying imap messages that's causing problems.
 And the fix is trivial. I'll attach the patch here. Travis, can you give me a
code review on this since Rick Potts is out of town?

DoLoadUrl had code that looked like the following:

nsCOMPtr<nsIChannel> dummyChannel;
rv = NS_OpenURI(getter_AddRefs(dummyChannel), aUri, nsnull, interfaceRequestor);
if (NS_FAILED(rv)) return rv;
if (SOME CONDITION testing if the url has a ref field)
	use the dummy channel and return
else if (SOME OTHER CONDITION to test if we are loading from session history)
   use the dummy channel and return

If neither of these conditions were true, then doLoadURL went ahead and kicked
off a regular url load by calling into the uri loader.

The problem was that for a certain case for imap, we were creating a dummy
channel under some bogus conditions such that the imap protocol handler failed
to create a channel and it returned NS_OK back through NS_OpenURI. So the first
fix is to perculate the error back up. Which I did.

However, that's not good enough because of the if (NS_FAILED(rv)) return rv
statement after creating the dummy channel which causes us to never load the url
in this case. It happens to be for imap that neither of the if conditions in my
example above are ever true. We always want to continue forward with the load
even if we failed to create a dummy channel. Because we are never going to use
the dummy channel.

So my fix for webshell::doloadURl was to stall instantiation of the dummy
channel until we are sure we are going to need it. This meant making the call to
NS_OpenURI inside of each if clause.

if (some condition)
  create and use dummy channel returning if failure.
else if (some condition)
  create and use dummy channel, returning if failure.

This fixes the problem because now we don't even try to create a dummy channel
if we really aren't going to use it. And the fix is completely safe because we
aren't altering the flow of control at all.

Comment 3

19 years ago
Created attachment 4365 [details] [diff] [review]
proposed fix


19 years ago
Whiteboard: have fix; waiting for review


19 years ago
Last Resolved: 19 years ago
Resolution: --- → FIXED
Whiteboard: have fix; waiting for review

Comment 4

19 years ago
I checked in the fix....

Comment 5

19 years ago
Win32  (2000-04-21-08 M16)
Using the same scenario, I do not see the problem.
QA Contact: lchiang → fenella
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.