Assertion failure: !mSuppressionActive (Should have un-suppress via StopDrag() first.), at layout/xul/nsSliderFrame.cpp:96 on Google Keep

RESOLVED FIXED in Firefox 47

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: dbaron, Assigned: BenWa)

Tracking

({regression})

Trunk
mozilla48
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox46 unaffected, firefox47 fixed, firefox48 fixed)

Details

()

Attachments

(1 attachment)

Steps to reproduce:
 1. load https://keep.google.com/
 2. create (if you don't have one already) a note that's long enough to have a scrollbar
 3. open that note
 4. drag the scrollbar

Crash due to fatal assertion:
Assertion failure: !mSuppressionActive (Should have un-suppress via StopDrag() first.), at mozilla/layout/xul/nsSliderFrame.cpp:96

This assertion was added recently in bug 1257369.

This appears to be reliably reproducable (I've hit it twice).


Stack is:

Assertion failure: !mSuppressionActive (Should have un-suppress via StopDrag() first.), at /home/dbaron/builds/ssd/mozilla-central/mozilla/layout/xul/nsSliderFrame.cpp:96
#01: nsIPresShell::FreeByObjectID(mozilla::ArenaObjectID, void*) (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/nsIPresShell.h:265)
#02: nsFrameList::DestroyFramesFrom(nsIFrame*) (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/generic/nsFrameList.cpp:56)
#03: nsContainerFrame::DestroyFrom(nsIFrame*) (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/generic/nsContainerFrame.cpp:222)
#04: nsFrameList::DestroyFramesFrom(nsIFrame*) (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/generic/nsFrameList.cpp:56)
#05: nsContainerFrame::DestroyFrom(nsIFrame*) (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/generic/nsContainerFrame.cpp:222)
#06: nsBlockFrame::DoRemoveFrame(nsIFrame*, unsigned int) (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/generic/nsBlockFrame.cpp:5802)
#07: nsBlockFrame::RemoveFrame(mozilla::layout::FrameChildListID, nsIFrame*) (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/generic/nsBlockFrame.cpp:5166)
#08: nsFrameManager::RemoveFrame(mozilla::layout::FrameChildListID, nsIFrame*) (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/nsFrameManager.cpp:508)
#09: nsCSSFrameConstructor::ContentRemoved(nsIContent*, nsIContent*, nsIContent*, nsCSSFrameConstructor::RemoveFlags, bool*, nsIContent**) (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/nsCSSFrameConstructor.cpp:8393)
#10: nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*, bool, nsCSSFrameConstructor::RemoveFlags, nsIContent**) (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/nsCSSFrameConstructor.cpp:9577 (discriminator 4))
#11: mozilla::RestyleManager::ProcessRestyledFrames(nsStyleChangeList&) (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/RestyleManager.cpp:835)
#12: mozilla::RestyleManager::ComputeAndProcessStyleChange(nsIFrame*, nsChangeHint, mozilla::RestyleTracker&, nsRestyleHint, mozilla::RestyleHintData const&) (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/RestyleManager.cpp:
#13: mozilla::RestyleManager::RestyleElement(mozilla::dom::Element*, nsIFrame*, nsChangeHint, mozilla::RestyleTracker&, nsRestyleHint, mozilla::RestyleHintData const&) (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/RestyleMa
#14: mozilla::RestyleTracker::ProcessOneRestyle(mozilla::dom::Element*, nsRestyleHint, nsChangeHint, mozilla::RestyleHintData const&) (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/RestyleTracker.cpp:95)
#15: mozilla::RestyleTracker::DoProcessRestyles() (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/RestyleTracker.cpp:264)
#16: mozilla::RestyleManager::ProcessPendingRestyles() (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/RestyleManager.cpp:1786)
#17: PresShell::FlushPendingNotifications(mozilla::ChangesToFlush) (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/nsPresShell.cpp:4060)
#18: PresShell::WillPaint() (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/nsPresShell.cpp:8854)
#19: nsViewManager::CallWillPaintOnObservers() (/home/dbaron/builds/ssd/mozilla-central/mozilla/view/nsViewManager.cpp:1137)
#20: nsViewManager::ProcessPendingUpdates() (/home/dbaron/builds/ssd/mozilla-central/mozilla/view/nsViewManager.cpp:1104)
#21: nsRefreshDriver::Tick(long, mozilla::TimeStamp) (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/nsRefreshDriver.cpp:1903)
#22: mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/nsRefreshDriver.cpp:246)
#23: mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/nsRefreshDriver.cpp:267)
#24: mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/nsRefreshDriver.cpp:510)
#25: mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/nsRefreshDriver.cpp:429 (discriminator 4))
#26: mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) (/home/dbaron/builds/ssd/mozilla-central/mozilla/layout/ipc/VsyncChild.cpp:67)
#27: mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) (/home/dbaron/builds/ssd/mozilla-central/obj/firefox-debugopt/ipc/ipdl/PVsyncChild.cpp:234)
#28: mozilla::ipc::MessageChannel::AutoSetValue<int>::~AutoSetValue() (/home/dbaron/builds/ssd/mozilla-central/obj/firefox-debugopt/dist/include/mozilla/ipc/MessageChannel.h:542)
#29: mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message const&) (/home/dbaron/builds/ssd/mozilla-central/mozilla/ipc/glue/MessageChannel.cpp:1586)
#30: mozilla::ipc::MessageChannel::OnMaybeDequeueOne() (/home/dbaron/builds/ssd/mozilla-central/mozilla/ipc/glue/MessageChannel.cpp:1553)
#31: MessageLoop::RunTask(Task*) (/home/dbaron/builds/ssd/mozilla-central/mozilla/ipc/chromium/src/base/message_loop.cc:350)
#32: MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) (/home/dbaron/builds/ssd/mozilla-central/mozilla/ipc/chromium/src/base/message_loop.cc:360)
#33: MessageLoop::DoWork() (/home/dbaron/builds/ssd/mozilla-central/mozilla/ipc/chromium/src/base/message_loop.cc:444)
#34: mozilla::ipc::DoWorkRunnable::Run() (/home/dbaron/builds/ssd/mozilla-central/mozilla/ipc/glue/MessagePump.cpp:228)
#35: nsThread::ProcessNextEvent(bool, bool*) (/home/dbaron/builds/ssd/mozilla-central/mozilla/xpcom/threads/nsThread.cpp:994)
#36: NS_ProcessNextEvent(nsIThread*, bool) (/home/dbaron/builds/ssd/mozilla-central/mozilla/xpcom/glue/nsThreadUtils.cpp:297)
#37: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (/home/dbaron/builds/ssd/mozilla-central/mozilla/ipc/glue/MessagePump.cpp:99)
#38: MessageLoop::RunInternal() (/home/dbaron/builds/ssd/mozilla-central/mozilla/ipc/chromium/src/base/message_loop.cc:231)
#39: MessageLoop::AutoRunState::~AutoRunState() (/home/dbaron/builds/ssd/mozilla-central/mozilla/ipc/chromium/src/base/message_loop.cc:505)
#40: nsBaseAppShell::Run() (/home/dbaron/builds/ssd/mozilla-central/mozilla/widget/nsBaseAppShell.cpp:158)
#41: XRE_RunAppShell (/home/dbaron/builds/ssd/mozilla-central/mozilla/toolkit/xre/nsEmbedFunctions.cpp:759)
#42: mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) (/home/dbaron/builds/ssd/mozilla-central/mozilla/ipc/glue/MessagePump.cpp:271)
#43: MessageLoop::RunInternal() (/home/dbaron/builds/ssd/mozilla-central/mozilla/ipc/chromium/src/base/message_loop.cc:231)
#44: MessageLoop::AutoRunState::~AutoRunState() (/home/dbaron/builds/ssd/mozilla-central/mozilla/ipc/chromium/src/base/message_loop.cc:505)
#45: XRE_InitChildProcess (/home/dbaron/builds/ssd/mozilla-central/mozilla/toolkit/xre/nsEmbedFunctions.cpp:641)
#46: content_process_main(int, char**) (/home/dbaron/builds/ssd/mozilla-central/mozilla/ipc/app/../contentproc/plugin-container.cpp:240)
#47: __libc_start_main (/build/glibc-ryFjv0/glibc-2.21/csu/libc-start.c:323)
Flags: needinfo?(bgirard)
(setting flags without testing in those versions, based on where the patch causing it has been landed)
(Note that this should only happen with the apz.drag.enabled pref set to true, which is a non-default configuration)
I do not have that pref enabled.  So your assumption that this can only happen with it enabled does not hold.
Assignee

Comment 4

3 years ago
Right, this comes from bug 1257369. The aim of this assert is to make sure that we don't forget to unsuppress the display-port and leave the user in a state with no displayport.

Thanks for the steps :dbaron. I'll address this tomorrow.
Ah, my mistake. I got confused with the other changes we made to that file.
Blocks: apz-desktop
No longer blocks: async-scrollbar-drag
Keywords: regression
Assignee

Comment 7

3 years ago
Alright my goal with this assert is if we started getting into a state with no displayport and reports if this assert we would know that we were too late to unsuppress the displayport. But given that this hasn't happen I think the assert is just too strict and that unsupressing from ~nsSliderFrame is ok.
Flags: needinfo?(bgirard)
Attachment #8744375 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8744375 [details]
MozReview Request: Bug 1265825 - Remove mSuppressionActive assert. r?kats

https://reviewboard.mozilla.org/r/48505/#review45199

Comment 10

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5554679e1574
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Please request uplift to 47 as well.
Component: XP Toolkit/Widgets: XUL → Panning and Zooming
Assignee: nobody → bgirard
Assignee

Comment 12

3 years ago
Comment on attachment 8744375 [details]
MozReview Request: Bug 1265825 - Remove mSuppressionActive assert. r?kats

Approval Request Comment
[Feature/regressing bug #]: bug 1257369
[User impact if declined]: None, this is #DEBUG only
[Describe test coverage new/current, TreeHerder]: No test coverage change required.
[Risks and why]: Should be none to the user, it will only remove a #DEBUG check
[String/UUID change made/needed]: none
Attachment #8744375 - Flags: approval-mozilla-beta?
Comment on attachment 8744375 [details]
MozReview Request: Bug 1265825 - Remove mSuppressionActive assert. r?kats

Removes an assert, Beta47+
Attachment #8744375 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.