crash in nsDocShell::IssueWarning regressing since Firefox 44

VERIFIED FIXED in Firefox 44

Status

()

Core
Document Navigation
--
critical
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: philipp, Assigned: Ehsan)

Tracking

({crash, regression, topcrash})

44 Branch
mozilla47
x86
Windows NT
crash, regression, topcrash
Points:
---

Firefox Tracking Flags

(firefox44+ verified, firefox45 verified, firefox46 verified, firefox47 verified, relnote-firefox 44+)

Details

(crash signature)

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
[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.
Flags: needinfo?(honzab.moz)
(Assignee)

Comment 1

3 years ago
Ugh, we need an mContentViewer null check.
Assignee: nobody → ehsan
Component: Networking: HTTP → Document Navigation
Flags: needinfo?(honzab.moz)
(Assignee)

Comment 2

3 years ago
Created attachment 8714495 [details] [diff] [review]
Patch (v1)
Attachment #8714495 - Flags: review?(bzbarsky)
Comment on attachment 8714495 [details] [diff] [review]
Patch (v1)

r=me
Attachment #8714495 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 4

3 years ago
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?

Comment 6

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/53da63c1fb0e
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox47: affected → fixed
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+
Keywords: topcrash

Comment 8

3 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/fae2fa0bf463
status-firefox46: affected → fixed

Comment 9

3 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/458a80d87ead
status-firefox45: affected → fixed
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)

Comment 11

3 years ago
(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

3 years ago
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.
(Assignee)

Comment 13

3 years ago
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)
(Assignee)

Comment 14

3 years ago
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+

Comment 16

3 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-release/rev/eca0ebc1f485
status-firefox44: affected → fixed
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
relnote-firefox: --- → 44+

Updated

3 years ago
tracking-firefox44: ? → +
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.
Status: RESOLVED → VERIFIED
status-firefox44: fixed → verified
status-firefox45: fixed → verified
status-firefox46: fixed → verified
status-firefox47: fixed → verified
Flags: needinfo?(cornel.ionce)
You need to log in before you can comment on or make changes to this bug.