Closed Bug 1615306 Opened 5 years ago Closed 5 years ago

Crash in [@ nsFrameLoader::DestroyDocShell]

Categories

(Core :: DOM: Navigation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla76
Tracking Status
firefox-esr68 --- unaffected
firefox74 --- disabled
firefox75 --- fixed
firefox76 --- fixed

People

(Reporter: pascalc, Assigned: kmag)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

This bug is for crash report bp-ba14a585-cc45-4f54-a7c7-4a7940200212.

Top 10 frames of crashing thread:

0 xul.dll nsFrameLoader::DestroyDocShell dom/base/nsFrameLoader.cpp:1934
1 xul.dll nsFrameLoaderDestroyRunnable::Run dom/base/nsFrameLoader.cpp:1867
2 xul.dll mozilla::dom::Document::MaybeInitializeFinalizeFrameLoaders dom/base/Document.cpp:8521
3 xul.dll mozilla::detail::RunnableMethodImpl< xpcom/threads/nsThreadUtils.h:1215
4 xul.dll static nsContentUtils::RemoveScriptBlocker dom/base/nsContentUtils.cpp:5376
5 xul.dll nsDocumentViewer::InitInternal layout/base/nsDocumentViewer.cpp:973
6 xul.dll nsDocumentViewer::Init layout/base/nsDocumentViewer.cpp:758
7 xul.dll nsDocShell::SetupNewViewer docshell/base/nsDocShell.cpp:8004
8 xul.dll nsDocShell::Embed docshell/base/nsDocShell.cpp:5777
9 xul.dll nsDocShell::CreateAboutBlankContentViewer docshell/base/nsDocShell.cpp:6617

New signature that started appearing on 74.0b1/b2

A couple of crashes in the last days on Nightly as well.

Component: General → DOM: Core & HTML

Hi :kmag, I saw frame 0 was changed by bug 1582832. It seems that it's unable to read mBrowsingContext->EverAttached(). Could you help to take a look? Thank you.

Component: DOM: Core & HTML → DOM: Navigation
Flags: needinfo?(kmaglione+bmo)

the signature stopped in 74.0b after the early beta phase - in 75.0b it's continuing and looking like it will go to release as well.
submitted urls in crash reports are dominated by mail.google.com, which is also referenced in the two available comments:

  • Clicked to open a gmail message and BLAMMO!
  • Hit refresh on GMail after computer sat in sleep mode over night
Flags: needinfo?(cpeterson)
Keywords: regression
OS: Windows 10 → All
Hardware: Unspecified → All

(In reply to [:philipp] from comment #3)

the signature stopped in 74.0b after the early beta phase - in 75.0b it's continuing and looking like it will go to release as well.

@ Philipp: I see crash reports from 74.0b and 75.0b with about the same volume, but no crash reports from 74.0 release. I wonder why not. Any reason to think the crashes in 75.0b will continue into 75.0 release when they didn't in 74.0?

Peeking at crash report bp92c4a0e1-f42f-4e68-ba60-205620200330, looks like maybe a nsFrameLoaderDestroyRunnable::mFrameLoader is crashing on a null mPendingBrowsingContext?

https://hg.mozilla.org/mozilla-central/annotate/01b9bdb6e5ef2a19e1c3311a47569d62f275db1f/dom/base/nsFrameLoader.cpp#l1931

Flags: needinfo?(cpeterson) → needinfo?(madperson)

during the 74 beta cycle, crashes were last recorded in 74.0b5 and have stopped in b6 and later on:
https://crash-stats.mozilla.org/search/?signature=%3DnsFrameLoader%3A%3ADestroyDocShell&release_channel=beta&date=>%3D2020-01-01&_facets=version#facet-version (sort by version)

first i thought this may have been due to reverting the EARLY_BETA_OR_EARLIER flag, but that happened later in b7.

this is the pushlog of changes happening between 74.0b5 and 74.0b6, so perhaps the backout at https://bugzilla.mozilla.org/show_bug.cgi?id=1582832#c29 took care of the issue for 74:
https://hg.mozilla.org/releases/mozilla-beta/pushloghtml?fromchange=FIREFOX_74_0b5_RELEASE&tochange=23159c7ee39f9cb1ee182a72d0b3d1afdc6ceba4&full=1

Flags: needinfo?(madperson)
Assignee: nobody → kmaglione+bmo
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla76

hi, thanks for the patch. as it looks fairly simple/safe and a regressing issue, could you request uplift to beta? (although it's fairly late in the cycle already)

Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(kmaglione+bmo)

Comment on attachment 9136970 [details]
Bug 1615306: Add missing null check. r=nika

Beta/Release Uplift Approval Request

  • User impact if declined: crashes
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): It just adds a null check. Maybe it won't fix anything, but it shouldn't cause any additional issues.
  • String changes made/needed:
Attachment #9136970 - Flags: approval-mozilla-beta?
Flags: needinfo?(kmaglione+bmo)

Comment on attachment 9136970 [details]
Bug 1615306: Add missing null check. r=nika

nullptr check, approved for 75 rc1

Attachment #9136970 - Flags: approval-mozilla-beta? → approval-mozilla-release+
Flags: qe-verify+

There is nothing QE can do here as per comment 10.
Removing QE+ flag.

Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: