1244076 - crash in nsDocShell::IssueWarning regressing since Firefox 44

Status: VERIFIED FIXED

(Core :: DOM: Navigation, defect)

Description

[[:philipp]]

[Tracking Requested - why for this release]:
requesting tracking, since it is a regression.

This bug was filed from the Socorro interface and is 
report bp-e9a1d6f1-9dbc-4483-b436-413582160128.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	xul.dll 	nsDocShell::IssueWarning(unsigned int, bool) 	docshell/base/nsDocShell.cpp
1 	xul.dll 	mozilla::net::nsHttpChannel::MaybeWarnAboutAppCache() 	netwerk/protocol/http/nsHttpChannel.cpp
2 	xul.dll 	mozilla::net::nsHttpChannel::OnOfflineCacheEntryAvailable(nsICacheEntry*, bool, nsIApplicationCache*, nsresult) 	netwerk/protocol/http/nsHttpChannel.cpp
3 	xul.dll 	mozilla::net::nsHttpChannel::OnCacheEntryAvailableInternal(nsICacheEntry*, bool, nsIApplicationCache*, nsresult) 	netwerk/protocol/http/nsHttpChannel.cpp
4 	xul.dll 	mozilla::net::nsHttpChannel::OnCacheEntryAvailable(nsICacheEntry*, bool, nsIApplicationCache*, nsresult) 	netwerk/protocol/http/nsHttpChannel.cpp
5 	xul.dll 	mozilla::net::_OldCacheLoad::Run() 	netwerk/cache2/OldWrappers.cpp
6 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp
7 	xul.dll 	mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp
8 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc
9 	xul.dll 	nsThreadManager::nsThreadManager() 	xpcom/threads/nsThreadManager.h
10 	xul.dll 	nsBaseAppShell::Run() 	widget/nsBaseAppShell.cpp
11 	xul.dll 	nsAppShell::Run() 	widget/windows/nsAppShell.cpp
12 	xul.dll 	nsAppStartup::Run() 	toolkit/components/startup/nsAppStartup.cpp
13 	xul.dll 	XREMain::XRE_mainRun() 	toolkit/xre/nsAppRunner.cpp
14 	xul.dll 	XREMain::XRE_main(int, char** const, nsXREAppData const*) 	toolkit/xre/nsAppRunner.cpp
15 	xul.dll 	XRE_main 	toolkit/xre/nsAppRunner.cpp

this signature is regressing since firefox 44 builds, currently it is at #10 on the crash score board for 44.0 due to happening during startup 60% of the time. 
the cause of this crash may well be bug 1204581, so i've cc'ed ehsan here as well.

Comment 1

[(no longer active)]

Ugh, we need an mContentViewer null check.

Comment 2

[(no longer active)]

Attachment: Patch (v1)

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8714495 [details] [diff] [review]
Patch (v1)

r=me

Comment 4

[(no longer active)]

Comment on attachment 8714495 [details] [diff] [review]
Patch (v1)

Approval Request Comment
[Feature/regressing bug #]: Bug 1204581
[User impact if declined]: Crash
[Describe test coverage new/current, TreeHerder]: Simple null check, no tests
[Risks and why]: Not risky
[String/UUID change made/needed]: none

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/53da63c1fb0e

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/53da63c1fb0e

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8714495 [details] [diff] [review]
Patch (v1)

Fix a crash, taking it.
Should be in 45 beta 3!

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/fae2fa0bf463

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/458a80d87ead

Comment 10

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Ehsan, since this was filed as a #10 top crash scorer for Fx44, do you think this is a good ride-along to take in 44.0.1? I would have preferred to take a fix that is verified (is that the case here?) Thanks!

Comment 11

[Robert Kaiser]

(In reply to Ritu Kothari (:ritu) from comment #10)
> Ehsan, since this was filed as a #10 top crash scorer for Fx44

It was that high in early data but has since slipped pretty low. That's probably because it's a startup crash and we stopped delivering updates so almost nobody get it on starting a fresh update right now.

Comment 12

[Robert Kaiser]

Just if it doesn't make it clear, I wanted to say that despite this not being high up right now, I'd like to see this in 44.0.1 so people will not crash on/near startup once they actually get updates.

Comment 13

[(no longer active)]

I think we should take this for 44.0.1.  While the fix is not verified yet, it's pretty clear to me from analyzing the crash report that this is the right fix.  (And even if the patch doesn't end up fixing the crash for some reason, it's just a null check so it will definitely not hurt anything.)

Comment 14

[(no longer active)]

Comment on attachment 8714495 [details] [diff] [review]
Patch (v1)

Approval Request Comment
See comment 4.

Note that the patch is virtually risk-free!

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8714495 [details] [diff] [review]
Patch (v1)

"Virtually risk-free" :) startup crash fix which was #10 when Fx44 went live. This is another good candidate to be considered for 44.0.1

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-release/rev/eca0ebc1f485

Comment 17

[Cornel Ionce [:noni] [Hubs QA]]

We were unable to reproduce this issue and thus, we can't confirm this fix.
We'll keep an eye on Socorro the following days to make sure this crash signature is gone.
Needinfo'ing myself as a reminder.

Comment 18

[Sylvestre Ledru [:Sylvestre]]

Added to the release notes as "Fix a top crash in cache networking (1244076)" as wording

Comment 19

[Paul Silaghi, QA [:pauly]]

Looking in Socorro, latest crashes are:
46.0a2	20160204004009
45.0b2	20160201143558
44.0	20160126223146
47.0a1	20160130030240
Based on the dates the fix landed to branches, marking this as verified fixed.