1483043 - smtp://foo and ldap:foo get linkified and crash when clicked

Status: RESOLVED FIXED

(MailNews Core :: General, defect)

Description

[Jorg K (CEST = GMT+2)]

smtp://foo and ldap:foo get linkified and crash when clicked.

Crash is in SerializeURI() on MOZ_CRASH("All IPDL URIs must be serializable!").

Valentin, can you enlighten me here? ;-)

Comment 1

[Valentin Gosu [:valentin] (he/him)]

That's because we are serializing a URI type that doesn't implement nsIIPCSerializableURI.
I was under the impression that thunderbird in doesn't use e10s, so I don't know why they would be serialized when clicked.

Comment 2

[Jorg K (CEST = GMT+2)]

I didn't think we were using e10s. How do I investigate this further?

Comment 3

[Valentin Gosu [:valentin] (he/him)]

We should find out why this happens (a stack trace would help).

Comment 4

[Jorg K (CEST = GMT+2)]

Yes, I saw the stack yesterday, not very useful:

xul.dll!mozilla::ipc::SerializeURI(nsIURI * aURI, mozilla::ipc::URIParams & aParams) Line 50	C++
xul.dll!mozilla::places::`anonymous namespace'::NotifyManyVisitsObservers::Run() Line 662	C++
xul.dll!nsThread::ProcessNextEvent(bool aMayWait, bool * aResult) Line 1238	C++
xul.dll!NS_ProcessNextEvent(nsIThread * aThread, bool aMayWait) Line 519	C++
xul.dll!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate * aDelegate) Line 97	C++
xul.dll!MessageLoop::RunHandler() Line 319	C++
xul.dll!MessageLoop::Run() Line 299	C++
xul.dll!nsBaseAppShell::Run() Line 160	C++
xul.dll!nsAppShell::Run() Line 417	C++
xul.dll!nsAppStartup::Run() Line 290	C++
xul.dll!XREMain::XRE_mainRun() Line 4799	C++
xul.dll!XREMain::XRE_main(int argc, char * * argv, const mozilla::BootstrapConfig & aConfig) Line 4944	C++
xul.dll!XRE_main(int argc, char * * argv, const mozilla::BootstrapConfig & aConfig) Line 5036	C++

I did a DumpJSStack() in the debugger and got "there is no JSContext on the stack!" :-(

BTW, you can happily click onto a mailbox://foo (text is linkified) without a crash. So somehow smtp: and mailbox: are different. Where to look for that difference?

Comment 5

[Valentin Gosu [:valentin] (he/him)]

So, the crash seems to be here:
https://searchfox.org/mozilla-central/rev/2466b82b729765fb0a3ab62f812c1a96a7362478/toolkit/components/places/History.cpp#661,671

IMO we shouldn't be serializing the URIs if we are not in a e10s situation.
We should probably add a guard if (mozilla::BrowserTabsRemoteAutostart()) before serializing.

Comment 6

[Jorg K (CEST = GMT+2)]

Valentin, could you do that for us?

Comment 7

[Valentin Gosu [:valentin] (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6687ebc65623564f5dee3c5a305b963cf94defd0

Comment 8

[Jorg K (CEST = GMT+2)]

Thanks, looks like to need to add #include "nsIXULRuntime.h" to make it compile. The patch stops the crash.

It also reveals a difference between mailbox://foo (non-crashing) and smtp://foo (was crashing). When clicking the latter, I get the "Launch Application" panel.

Where do I look for the code/config that attempts to launch an external app for smtp but not mailbox. Hmm, we have
  pref("network.protocol-handler.expose.mailbox", true);
but nothing for smtp or ldap. So that appears to be the difference. I learn something new every day ;-)

Comment 9

[Valentin Gosu [:valentin] (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=093170a7feacc77a9270bf71c61d19227dfda6c4

Comment 10

[Valentin Gosu [:valentin] (he/him)]

(In reply to Jorg K (GMT+2) from comment #8)
> Thanks, looks like to need to add #include "nsIXULRuntime.h" to make it
> compile. The patch stops the crash.

Glad that it fixed the issue. I'll wait for the try run, then submit for review.

Comment 11

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1483043 - Only call SerializeURI if running in e10s r=mak

Some nsIURI types do not implement nsIIPCSerializableURI, and this is causing a crash in SerializeURI when running in thunderbird.
This shouldn't happen, since thunderbird is not running under e10s, so even attempting to serialize the URIs is pointless.

Comment 13

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Comment on attachment 9000181 [details]
Bug 1483043 - Only call SerializeURI if running in e10s r=mak

Marco Bonardo [::mak] has approved the revision.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/92c10215a3f6
Only call SerializeURI if running in e10s r=mak

Comment 15

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/92c10215a3f6

Comment 16

[Jorg K (CEST = GMT+2)]

Comment on attachment 9000181 [details]
Bug 1483043 - Only call SerializeURI if running in e10s r=mak

The crash can be reproduced with TB 60 ESR, so I'll take that patch to our release branch. Of course the bug is in the wrong product, but that lets me attach the flag I need.

Comment 17

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/releases/mozilla-esr60/rev/88613057c07dbcb2b5be036c5b5e76faeb8fd91d (THUNDERBIRD_60_VERBRANCH)

Trick to get this off my query: releases/comm-esr60/rev.