Closed Bug 1244076 Opened 4 years ago Closed 4 years ago

crash in nsDocShell::IssueWarning regressing since Firefox 44


(Core :: DOM: Navigation, defect, critical)

44 Branch
Windows NT
Not set



Tracking Status
firefox44 + verified
firefox45 --- verified
firefox46 --- verified
firefox47 --- verified
relnote-firefox --- 44+


(Reporter: philipp, Assigned: ehsan)



(Keywords: crash, regression, topcrash)

Crash Data


(1 file)

[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/
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.
Flags: needinfo?(honzab.moz)
Ugh, we need an mContentViewer null check.
Assignee: nobody → ehsan
Component: Networking: HTTP → Document Navigation
Flags: needinfo?(honzab.moz)
Attached patch Patch (v1)Splinter Review
Attachment #8714495 - Flags: review?(bzbarsky)
Comment on attachment 8714495 [details] [diff] [review]
Patch (v1)

Attachment #8714495 - Flags: review?(bzbarsky) → review+
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
Attachment #8714495 - Flags: approval-mozilla-beta?
Attachment #8714495 - Flags: approval-mozilla-aurora?
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Comment on attachment 8714495 [details] [diff] [review]
Patch (v1)

Fix a crash, taking it.
Should be in 45 beta 3!
Attachment #8714495 - Flags: approval-mozilla-beta?
Attachment #8714495 - Flags: approval-mozilla-beta+
Attachment #8714495 - Flags: approval-mozilla-aurora?
Attachment #8714495 - Flags: approval-mozilla-aurora+
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!
Flags: needinfo?(ehsan)
(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.
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.
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.)
Flags: needinfo?(ehsan)
Comment on attachment 8714495 [details] [diff] [review]
Patch (v1)

Approval Request Comment
See comment 4.

Note that the patch is virtually risk-free!
Attachment #8714495 - Flags: approval-mozilla-release?
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
Attachment #8714495 - Flags: approval-mozilla-release? → approval-mozilla-release+
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.
Flags: needinfo?(cornel.ionce)
Added to the release notes as "Fix a top crash in cache networking (1244076)" as wording
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.
Flags: needinfo?(cornel.ionce)
You need to log in before you can comment on or make changes to this bug.