Closed Bug 1265825 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: Panning and Zooming, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox46 --- unaffected
firefox47 --- fixed
firefox48 --- fixed

People

(Reporter: dbaron, Assigned: BenWa)

References

()

Details

(Keywords: regression)

Attachments

(1 file)

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.
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
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+
https://hg.mozilla.org/mozilla-central/rev/5554679e1574
Status: NEW → RESOLVED
Closed: 8 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
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.