Closed
Bug 1373604
Opened 7 years ago
Closed 7 years ago
Crash in nsContentUtils::DispatchFocusChromeEvent
Categories
(Core :: DOM: Navigation, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox54 | --- | unaffected |
firefox55 | + | fixed |
firefox56 | --- | fixed |
People
(Reporter: philipp, Assigned: freesamael)
References
Details
(Keywords: crash, regression)
Crash Data
Attachments
(2 files)
2.10 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
1.98 KB,
patch
|
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is report bp-e5bae163-2159-4b05-8300-60d2e0170616. ============================================================= Crashing Thread (0) Frame Module Signature Source 0 xul.dll nsContentUtils::DispatchFocusChromeEvent(nsPIDOMWindowOuter*) dom/base/nsContentUtils.cpp:4415 1 xul.dll nsDocShell::InternalLoad(nsIURI*, nsIURI*, bool, nsIURI*, unsigned int, nsIPrincipal*, nsIPrincipal*, unsigned int, nsAString const&, char const*, nsAString const&, nsIInputStream*, nsIInputStream*, unsigned int, nsISHEntry*, bool, nsAString const&, nsIDocShell*, nsIURI*, bool, nsIDocShell**, nsIRequest**) docshell/base/nsDocShell.cpp:10166 2 xul.dll nsDocShell::OnLinkClickSync(nsIContent*, nsIURI*, char16_t const*, nsAString const&, nsIInputStream*, nsIInputStream*, bool, nsIDocShell**, nsIRequest**, nsIPrincipal*) docshell/base/nsDocShell.cpp:14212 3 xul.dll OnLinkClickEvent::Run() docshell/base/nsDocShell.cpp:13966 4 xul.dll nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:1406 5 xul.dll mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:96 6 xul.dll MessageLoop::RunHandler() ipc/chromium/src/base/message_loop.cc:231 7 xul.dll MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:211 8 xul.dll nsBaseAppShell::Run() widget/nsBaseAppShell.cpp:156 9 xul.dll nsAppShell::Run() widget/windows/nsAppShell.cpp:271 10 xul.dll nsAppStartup::Run() toolkit/components/startup/nsAppStartup.cpp:283 11 xul.dll XREMain::XRE_mainRun() toolkit/xre/nsAppRunner.cpp:4569 12 xul.dll XREMain::XRE_main(int, char** const, mozilla::BootstrapConfig const&) toolkit/xre/nsAppRunner.cpp:4749 13 xul.dll XRE_main(int, char** const, mozilla::BootstrapConfig const&) toolkit/xre/nsAppRunner.cpp:4844 14 xul.dll mozilla::BootstrapImpl::XRE_main(int, char** const, mozilla::BootstrapConfig const&) toolkit/xre/Bootstrap.cpp:45 15 firefox.exe wmain toolkit/xre/nsWindowsWMain.cpp:115 16 firefox.exe __scrt_common_main_seh f:/dd/vctools/crt/vcstartup/src/startup/exe_common.inl:253 17 kernel32.dll BaseThreadInitThunk 18 ntdll.dll __RtlUserThreadStart 19 ntdll.dll _RtlUserThreadStart this is a crash signature that's apparently regressing in 55.0b judging by early crash data from the staged rollout. it's happening in a codepath last touched by bug 1303838, so i'm tentatively setting it as blocking that bug.
Comment 2•7 years ago
|
||
This is top6 crash on 55.0b browser type in the past 7 days.
Priority: -- → P1
Assignee | ||
Comment 3•7 years ago
|
||
The reports all look like nullptr+offset case, so likely aWindow is nullptr at [1]. Maybe targetDocShell is being destroyed at [2] so the window becomes null? Need to take a look. [1] https://hg.mozilla.org/releases/mozilla-beta/annotate/6872377277a6/dom/base/nsContentUtils.cpp#l4415 [2] https://hg.mozilla.org/releases/mozilla-beta/annotate/6872377277a6/docshell/base/nsDocShell.cpp#l10166
Assignee: nobody → sawang
Flags: needinfo?(sawang)
Assignee | ||
Comment 4•7 years ago
|
||
MozReview-Commit-ID: H1TsRcbagg4
Assignee | ||
Comment 5•7 years ago
|
||
Comment on attachment 8879837 [details] [diff] [review] Only try to switch focus to targetDocShell if the load succeeds Hi Olli, I think the reason is that I didn't check if targetDocShell->InternalLoad() succeeds, so that if somehow the load fails, targetDocShell->GetWindow() may become nullptr. Would you take a look?
Attachment #8879837 -
Flags: review?(bugs)
Updated•7 years ago
|
Attachment #8879837 -
Flags: review?(bugs) → review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f1118323902a Only try to switch focus to targetDocShell if the load succeeds. r=smaug
Keywords: checkin-needed
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f1118323902a
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Comment 8•7 years ago
|
||
[Tracking Requested - why for this release]: Top crasher on Beta (currently #6).
tracking-firefox55:
--- → ?
Assignee | ||
Comment 11•7 years ago
|
||
rebase for beta
Assignee | ||
Comment 12•7 years ago
|
||
Comment on attachment 8881445 [details] [diff] [review] Only try to switch focus to targetDocShell if the load succeeds Approval Request Comment [Feature/Bug causing the regression]: bug 1303838 [User impact if declined]: application crash [Is this code covered by automated tests?]: not for the crash case [Has the fix been verified in Nightly?]: yes, for few days [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: it essentially only adds a simple condition test [String changes made/needed]: none
Flags: needinfo?(sawang)
Attachment #8881445 -
Flags: approval-mozilla-beta?
Comment 13•7 years ago
|
||
Comment on attachment 8881445 [details] [diff] [review] Only try to switch focus to targetDocShell if the load succeeds Fix for topcrash in beta, let's take this for beta 6.
Attachment #8881445 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 14•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/67c173e36b9e
You need to log in
before you can comment on or make changes to this bug.
Description
•