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

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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)

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(setting flags without testing in those versions, based on where the patch causing it has been landed)

Comment 2

[Kartikaya Gupta (email:[e-mail])]

(Note that this should only happen with the apz.drag.enabled pref set to true, which is a non-default configuration)

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I do not have that pref enabled.  So your assumption that this can only happen with it enabled does not hold.

Comment 4

[Benoit Girard (:BenWa)]

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.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Ah, my mistake. I got confused with the other changes we made to that file.

Comment 6

[Benoit Girard (:BenWa)]

Attachment: MozReview Request: Bug 1265825 - Remove mSuppressionActive assert. r?kats

Review commit: https://reviewboard.mozilla.org/r/48505/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48505/

Comment 7

[Benoit Girard (:BenWa)]

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.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

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

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

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5554679e1574

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5554679e1574

Comment 11

[Kartikaya Gupta (email:[e-mail])]

Please request uplift to 47 as well.

Comment 12

[Benoit Girard (:BenWa)]

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

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

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

Removes an assert, Beta47+

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/bd61104c52b8