Closed
Bug 1214261
Opened 9 years ago
Closed 9 years ago
Crash on the mac while scrolling due to massive scrollbars
Categories
(Core :: Panning and Zooming, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla46
Tracking | Status | |
---|---|---|
firefox46 | --- | verified |
People
(Reporter: milan, Assigned: kats)
References
Details
(Keywords: regression, Whiteboard: [gfx-noted])
Attachments
(5 files, 2 obsolete files)
(deleted),
text/plain
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
patch
|
mstange
:
review+
kats
:
checkin+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
Scroll this - http://www.sjsu.edu/openuniversity/schedule/ and crash a few seconds into it.
The stack in crash report is not the best: https://crash-stats.mozilla.com/report/index/340f9956-fd69-4715-85a3-619522151013
but this is what it looked like:
0 libsystem_kernel.dylib 0x00007fff859b7716 __psynch_cvwait + 10
1 libsystem_pthread.dylib 0x00007fff8aacbc3b _pthread_cond_wait + 727
2 libnss3.dylib 0x00000001012031f9 PR_WaitCondVar + 105
3 XUL 0x0000000101781aa5 mozilla::ipc::MessageChannel::WaitForSyncNotify() + 69
4 XUL 0x0000000101781855 mozilla::ipc::MessageChannel::Send(IPC::Message*, IPC::Message*) + 853
5 XUL 0x0000000101b0fed0 mozilla::layers::PCompositorChild::SendFlushRendering() + 288
6 XUL 0x0000000101ffe5d4 mozilla::layers::CompositorChild::SendFlushRendering() + 20
7 XUL 0x000000010321f39a nsViewManager::Refresh(nsView*, nsIntRegion const&) + 234
8 XUL 0x00000001032203ae nsViewManager::PaintWindow(nsIWidget*, nsIntRegion) + 78
9 XUL 0x000000010321e786 nsView::PaintWindow(nsIWidget*, nsIntRegion) + 70
10 XUL 0x00000001032586c1 nsChildView::PaintWindow(nsIntRegion) + 145
11 XUL 0x0000000103262441 -[ChildView drawUsingOpenGL] + 321
12 XUL 0x0000000103261a3a -[ChildView drawRect:inContext:] + 122
13 XUL 0x0000000103261958 -[ChildView drawRect:] + 104
14 com.apple.AppKit 0x00007fff8e87304f -[NSView _drawRect:clip:] + 3748
15 com.apple.AppKit 0x00007fff8e8718c4 -[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] + 1799
16 com.apple.AppKit 0x00007fff8e871ca0 -[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] + 2787
17 com.apple.AppKit 0x00007fff8e871ca0 -[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] + 2787
18 com.apple.AppKit 0x00007fff8e86f706 -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 841
19 com.apple.AppKit 0x00007fff8e86eeb1 -[NSThemeFrame _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 314
20 com.apple.AppKit 0x00007fff8e86be9f -[NSView _displayRectIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:] + 2828
21 com.apple.AppKit 0x00007fff8e84b2da -[NSView displayIfNeeded] + 1680
22 com.apple.AppKit 0x00007fff8e8b074e _handleWindowNeedsDisplayOrLayoutOrUpdateConstraints + 884
23 com.apple.AppKit 0x00007fff8ee86061 __83-[NSWindow _postWindowNeedsDisplayOrLayoutOrUpdateConstraintsUnlessPostingDisabled]_block_invoke1331 + 46
24 com.apple.CoreFoundation 0x00007fff85b4dd67 __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 23
25 com.apple.CoreFoundation 0x00007fff85b4dcd7 __CFRunLoopDoObservers + 391
26 com.apple.CoreFoundation 0x00007fff85b3f3b8 __CFRunLoopRun + 776
27 com.apple.CoreFoundation 0x00007fff85b3ee75 CFRunLoopRunSpecific + 309
28 com.apple.HIToolbox 0x00007fff87e0ca0d RunCurrentEventLoopInMode + 226
29 com.apple.HIToolbox 0x00007fff87e0c685 ReceiveNextEventCommon + 173
30 com.apple.HIToolbox 0x00007fff87e0c5bc _BlockUntilNextEventMatchingListInModeWithFilter + 65
31 com.apple.AppKit 0x00007fff8e71424e _DPSNextEvent + 1434
32 com.apple.AppKit 0x00007fff8e71389b -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 122
33 XUL 0x000000010329a146 -[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 86
34 com.apple.AppKit 0x00007fff8e70799c -[NSApplication run] + 553
35 XUL 0x000000010329b2fc nsAppShell::Run() + 236
36 XUL 0x0000000103b8e789 nsAppStartup::Run() + 41
37 XUL 0x0000000103bd935d XREMain::XRE_mainRun() + 3325
38 XUL 0x0000000103bd96d8 XREMain::XRE_main(int, char**, nsXREAppData const*) + 600
39 XUL 0x0000000103bd9ad6 XRE_main + 246
40 org.mozilla.nightly 0x0000000100001b7d main + 1773
41 org.mozilla.nightly 0x0000000100001134 start + 52
Reporter | ||
Comment 1•9 years ago
|
||
A conversation on the topic:
12:28 dougt: it looks like we're flushing during paint and deadlocking.
12:28 dougt: a timeout causes us to unwind, it looks like
12:33 dougt: like why are we capping the image sizes at 32k
Reporter | ||
Comment 2•9 years ago
|
||
Seeing a lot of these:
[GFX2-]: Failed to Init() DrawTargetCG because of bad size.
[GFX1-]: Failed to create DrawTarget, Type: 2 Size: Size(42,482010)
...
BenWa, display port sizing?
Flags: needinfo?(bgirard)
Whiteboard: [gfx-noted]
Reporter | ||
Comment 3•9 years ago
|
||
I imagine APZ related, but I will leave it as graphics general until we know.
Comment 4•9 years ago
|
||
I don't get the crash but I'm getting -awful- performance. Even my cursor is unresponsive. We're very likely sizing something incorrectly.
Reporter | ||
Comment 5•9 years ago
|
||
This one is more useful. Shows up crashing in swap buffers.
Reporter | ||
Comment 6•9 years ago
|
||
(In reply to Benoit Girard (:BenWa) from comment #4)
> ... We're very likely sizing something incorrectly.
I'd say so, as we're asking for a 42 x 482010 draw target...
Comment 7•9 years ago
|
||
Here's the relevant part of the layer tree:
ContainerLayerComposite (0x1204e3e80) [shadow-visible=< (x=0, y=-121099, w=1648, h=241989); >] [visible=< (x=0, y=-121099, w=1648, h=241989); >]
PaintedLayerComposite (0x11133b300) [not visible] { hitregion=< (x=0, y=0, w=1648, h=1156); >} [opaqueContent]
PaintedLayerComposite (0x120791980) [shadow-clip=(x=0, y=0, w=1633, h=1156)] [shadow-transform=[ 1 0; 0 1; 0 -121099; ]] [shadow-visible=< (x=0, y=119654, w=1633, h=1445); (x=0, y=121099, w=417, h=1156); (x=1279, y=121099, w=354, h=1156); (x=0, y=122255, w=1633, h=1445); >] [transform=[ 1 0; 0 1; 0 -121099; ]] [bounds=(x=0, y=0, w=1633, h=241989)] [visible=< (x=0, y=119654, w=1633, h=4046); >] { hitregion=< (x=0, y=0, w=1633, h=241989); >} [opaqueContent] [metrics0={ [cb=(x=0.000000, y=0.000000, w=1633.000000, h=1156.000000)] [sr=(x=0.000000, y=0.000000, w=1633.000000, h=241988.796875)] [s=(0,121099)] [dp=(x=0.000000, y=-1445.000000, w=1633.000000, h=4046.000000)] [cdp=(x=0.000000, y=0.000000, w=0.000000, h=0.000000)] [color=rgba(230, 230, 224, 1.000000)] [scrollId=2] [rcd] [clip=(x=0, y=0, w=1633, h=1156)] [z=1] }] [valid=< (x=0, y=119654, w=1633, h=4046); >]
It looks like we get a reasonable visible region on the PaintedLayer [visible=< (x=0, y=119654, w=1633, h=4046); >], but the bounds are extreme bounds=(x=0, y=0, w=1633, h=241989).
Botond mentioned something recently about changing how ContaierLayerComposite compute the intermediate surface bounds so it may be related.
Lets see what the regression window turns up first.
Reporter | ||
Comment 8•9 years ago
|
||
The stack for the "give us a target that is huge":
#05: mozilla::gfx::DrawTargetCG::Init(mozilla::gfx::BackendType, unsigned char*, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, int, mozilla::gfx::SurfaceFormat)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x170ecca]
#06: mozilla::gfx::DrawTargetCG::Init(mozilla::gfx::BackendType, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::SurfaceFormat&)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1708b66]
#07: mozilla::gfx::Factory::CreateDrawTarget(mozilla::gfx::BackendType, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::SurfaceFormat)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1721c51]
#08: gfxQuartzNativeDrawing::BeginNativeDrawing()[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1a42bb3]
#09: nsNativeThemeCocoa::DrawWidgetBackground(nsRenderingContext*, nsIFrame*, unsigned char, nsRect const&, nsRect const&)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x41a643c]
#10: non-virtual thunk to nsNativeThemeCocoa::DrawWidgetBackground(nsRenderingContext*, nsIFrame*, unsigned char, nsRect const&, nsRect const&)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x41aabf2]
#11: nsDisplayThemedBackground::PaintInternal(nsDisplayListBuilder*, nsRenderingContext*, nsRect const&, nsRect*)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x466f9da]
#12: nsDisplayThemedBackground::Paint(nsDisplayListBuilder*, nsRenderingContext*)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x466f876]
#13: mozilla::FrameLayerBuilder::PaintItems(nsTArray<mozilla::FrameLayerBuilder::ClippedDisplayItem>&, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, gfxContext*, nsRenderingContext*, nsDisplayListBuilder*, nsPresContext*, mozilla::gfx::IntPoin[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x45ada7c]
#14: mozilla::FrameLayerBuilder::DrawPaintedLayer(mozilla::layers::PaintedLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x45aeb97]
#15: mozilla::layers::ClientMultiTiledLayerBuffer::Update(nsIntRegion const&, nsIntRegion const&, nsIntRegion const&)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x192deef]
#16: mozilla::layers::ClientMultiTiledLayerBuffer::PaintThebes(nsIntRegion const&, nsIntRegion const&, nsIntRegion const&, void (*)(mozilla::layers::PaintedLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegio[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x192d67d]
#17: mozilla::layers::ClientTiledPaintedLayer::RenderHighPrecision(nsIntRegion&, nsIntRegion const&, void (*)(mozilla::layers::PaintedLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x191b993]
#18: mozilla::layers::ClientTiledPaintedLayer::RenderLayer()[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x191c7fc]
#19: non-virtual thunk to mozilla::layers::ClientTiledPaintedLayer::RenderLayer()[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x191cb4c]
#20: mozilla::layers::ClientLayer::RenderLayerWithReadback(mozilla::layers::ReadbackProcessor*)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1937685]
#21: mozilla::layers::ClientContainerLayer::RenderLayer()[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x194fd80]
#22: non-virtual thunk to mozilla::layers::ClientContainerLayer::RenderLayer()[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x194feec]
#23: mozilla::layers::ClientLayer::RenderLayerWithReadback(mozilla::layers::ReadbackProcessor*)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1937685]
#24: mozilla::layers::ClientContainerLayer::RenderLayer()[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x194fd80]
#25: non-virtual thunk to mozilla::layers::ClientContainerLayer::RenderLayer()[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x194feec]
#26: mozilla::layers::ClientLayer::RenderLayerWithReadback(mozilla::layers::ReadbackProcessor*)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1937685]
#27: mozilla::layers::ClientContainerLayer::RenderLayer()[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x194fd80]
#28: non-virtual thunk to mozilla::layers::ClientContainerLayer::RenderLayer()[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x194feec]
#29: mozilla::layers::ClientLayerManager::EndTransactionInternal(void (*)(mozilla::layers::PaintedLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*, mozilla::layers::LayerManager::En[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1917271]
#30: mozilla::layers::ClientLayerManager::EndTransaction(void (*)(mozilla::layers::PaintedLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*, mozilla::layers::LayerManager::EndTransac[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x191752b]
#31: nsDisplayList::PaintRoot(nsDisplayListBuilder*, nsRenderingContext*, unsigned int)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4667d25]
#32: nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion const&, unsigned int, unsigned int)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x46a2547]
#33: PresShell::Paint(nsView*, nsRegion const&, unsigned int)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x46d2b7f]
#34: nsViewManager::ProcessPendingUpdatesPaint(nsIWidget*)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4100f43]
#35: nsViewManager::ProcessPendingUpdatesForView(nsView*, bool)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4100b2c]
#36: nsViewManager::ProcessPendingUpdates()[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4101bd4]
#37: nsRefreshDriver::Tick(long long, mozilla::TimeStamp)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x457ee59]
#38: mozilla::RefreshDriverTimer::TickDriver(nsRefreshDriver*, long long, mozilla::TimeStamp)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4587ab6]
#39: mozilla::RefreshDriverTimer::Tick(long long, mozilla::TimeStamp)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x45879a5]
#40: mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(mozilla::TimeStamp)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x458785d]
#41: mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x45877d4]
#42: mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x458752c]
#43: mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x49ff7f1]
#44: mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0xdcf118]
#45: mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x9812a5]
#46: mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x8e3192]
#47: mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message const&)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x8e248f]
#48: mozilla::ipc::MessageChannel::OnMaybeDequeueOne()[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x8de06b]
#49: void DispatchToMethod<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)()>(mozilla::ipc::MessageChannel*, bool (mozilla::ipc::MessageChannel::*)(), Tuple0 const&)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x900223]
#50: RunnableMethod<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)(), Tuple0>::Run()[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x90011e]
#51: mozilla::ipc::MessageChannel::RefCountedTask::Run()[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x903958]
#52: mozilla::ipc::MessageChannel::DequeueTask::Run()[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x903924]
#53: MessageLoop::RunTask(Task*)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x811920]
#54: MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x811e9f]
#55: MessageLoop::DoWork()[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x8120c4]
#56: mozilla::ipc::DoWorkRunnable::Run()[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x8e64ee]
#57: nsThread::ProcessNextEvent(bool, bool*)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1aa58f]
#58: NS_ProcessPendingEvents(nsIThread*, unsigned int)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x234dba]
#59: nsBaseAppShell::NativeEventCallback()[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x413c4e9]
#60: nsAppShell::ProcessGeckoEvents(void*)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x41d3abd]
Comment 9•9 years ago
|
||
Regression range looks to be APZ on mac which isn't very interesting.
Flags: needinfo?(bgirard)
Comment 10•9 years ago
|
||
Comment 11•9 years ago
|
||
Looks like the problem is caused by a large scrollbar. We just try to render it all. If I disable tiling then the crash is very easy to reproduce because we tried to allocate a single rotated buffer surface.
ni?mstange since I think you're familiar with the APZ mac scrollbar.
Component: Graphics → Panning and Zooming
Flags: needinfo?(mstange)
Updated•9 years ago
|
Summary: Crash on the mac while scrolling → Crash on the mac while scrolling due to massive scrollbars
Comment 12•9 years ago
|
||
Found the suspect:
2685 // The display port doesn't necessarily include the scrollbars, so just
2686 // include all of the scrollbars if we have a display port.
2687 nsRect dirty = aUsingDisplayPort ?
2688 scrollParts[i]->GetVisualOverflowRectRelativeToParent() : aDirtyRect;
https://dxr.mozilla.org/mozilla-central/source/layout/generic/nsGfxScrollFrame.cpp#2685
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Flags: needinfo?(mstange)
Comment 13•9 years ago
|
||
Timothy, since this is mostly needed for zooming, how do you feel about only doing this for root documents?
Assignee | ||
Comment 14•9 years ago
|
||
FWIW I think it should be fine only doing it for root documents as long as it doesn't regress bug 1076447.
Blocks: all-aboard-apz
Blocks: 1220091
Assignee | ||
Comment 15•9 years ago
|
||
I believe dvander is working on this now
Assignee: mstange → dvander
Updated•9 years ago
|
Priority: -- → P1
tn, could you take a look and let me know if this is on the right track?
The approach is basically the one we discussed in IRC. We expand the original dirty rect to include min(scrollport, displayport), then clamp it to the same rect. We then use that to clamp the vertical axis of the vertical scrollbar, the horizontal axis of the horizontal scrollbar, and both axes of the corner box.
Flags: needinfo?(tnikkel)
Attachment #8694020 -
Flags: feedback?(tnikkel)
Comment 17•9 years ago
|
||
Comment on attachment 8694020 [details] [diff] [review]
fix
>+ scrollbarClamp = aDirtyRect.Union(visible);
>+ scrollbarClamp = scrollbarClamp.Intersect(visible);
This will always make scrollbarClamp == visible.
This looks good.
I was trying to wrap my head around the resolution scaling we do of scrollbars in ScrollFrameHelper::LayoutScrollbars, I think we might have to take that into account unfortunately. Hopefully I can figure what we need to do exactly.
Attachment #8694020 -
Flags: feedback?(tnikkel) → feedback+
Comment 18•9 years ago
|
||
Comment on attachment 8694020 [details] [diff] [review]
fix
>+ if (aDisplayPort) {
>+ nsRect visible = aDisplayPort.value();
For the root scroll frame we want to multiply by the resolution of the presshell, because the displayport already has the resolution that the scrolled content should be drawn at factored in. However the resolution doesn't affect the scrollbars.
>+ // If the displayport is larger than the scrollport along either axis,
>+ // then choose the scrollport's axis as visible instead.
>+ if (visible.height > mScrollPort.height) {
>+ visible = nsRect(visible.x, mScrollPort.y, visible.width, mScrollPort.height);
>+ }
Instead of mScrollPort we want to use nsLayoutUtils::CalculateCompositionSizeForFrame here.
>+ if (aDisplayPort) {
>+ // If we have a displayport, our scrollbars aren't necessarily included
>+ // in the displayport. We compute the overflow rect of the scrollpart
>+ // and clamp its dimensions to the visible region we computed earlier.
>+ dirty = scrollParts[i]->GetVisualOverflowRectRelativeToParent();
>+
>+ // These checks include the corner box as well: Since its position is
>+ // dependent on the sizes of both axes, it must be clamped in both
>+ // directions to avoid creating an oversized layer.
>+ if (scrollParts[i] != mHScrollbarBox) {
>+ // Clamp the vertical axis.
>+ dirty.y = std::max(dirty.y, scrollbarClamp.y);
>+ dirty.height = scrollbarClamp.y + scrollbarClamp.height - dirty.y;
>+ }
In ScrollFrameHelper::LayoutScrollbars, when we determine the actual layout rect for the scrollbars, we multiply the height of scrollbars by the resolution if overlayScrollBarsWithZoom is true. So I think we also need to multiply dirty.height here by the resolution if overlayScrollBarsWithZoom (factor out a helper to compute it) is true.
I haven't checked that this produces the correct results, but seems like the theoretically correct thing to do.
Comment 19•9 years ago
|
||
I don't understand the approach that you agreed on.
Why are we changing behavior for the scrollbars of the root scroll frame? Their size is always bounded.
Why are we enlarging the dirty rect for scrollbars of subframes? With subframes, the scrollbars are outside the display port of that scroll frame, so the scroll frame's display port shouldn't be of any relevance to the visibility of its scrollbars.
Comment 20•9 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #19)
> I don't understand the approach that you agreed on.
Basically to clamp the height of vertical scrollbars to the minimum of the displayport height and the scrollport (composition size) height.
> Why are we changing behavior for the scrollbars of the root scroll frame?
> Their size is always bounded.
Good point. For root scroll frames that are in the root document in the process their size is bounded by the widget size. The intent of the approach is to still draw the full scrollbar.
> Why are we enlarging the dirty rect for scrollbars of subframes? With
> subframes, the scrollbars are outside the display port of that scroll frame,
> so the scroll frame's display port shouldn't be of any relevance to the
> visibility of its scrollbars.
If we have a very tall iframe (so that its displayport is smaller than its scrollport) and then we scroll the parent scrollable (not the tall iframe) to bring into view more content then I think we want to avoid the situation where we have content painted inside the tall iframe, but not its scrollbar beside the scrolled content. (We can't draw the entire scrollbar because it would be too big.)
Comment 21•9 years ago
|
||
(In reply to Timothy Nikkel (:tn) from comment #20)
> If we have a very tall iframe (so that its displayport is smaller than its
> scrollport) and then we scroll the parent scrollable (not the tall iframe)
> to bring into view more content then I think we want to avoid the situation
> where we have content painted inside the tall iframe, but not its scrollbar
> beside the scrolled content.
Hmm. Ok, I can kind of see the merit of that, but it doesn't seem like a very important problem to fix. With overlay scrollbars, in the case you're describing the scrollbars are usually hidden. And with classic scrollbars, the scrollbars will be drawn in the PaintedLayer of the parent scroll box contents, and they will be the only part of that layer that sticks that far out. That honestly sounds like it would look worse.
Comment 22•9 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #21)
> (In reply to Timothy Nikkel (:tn) from comment #20)
> > If we have a very tall iframe (so that its displayport is smaller than its
> > scrollport) and then we scroll the parent scrollable (not the tall iframe)
> > to bring into view more content then I think we want to avoid the situation
> > where we have content painted inside the tall iframe, but not its scrollbar
> > beside the scrolled content.
>
> Hmm. Ok, I can kind of see the merit of that, but it doesn't seem like a
> very important problem to fix. With overlay scrollbars, in the case you're
> describing the scrollbars are usually hidden. And with classic scrollbars,
> the scrollbars will be drawn in the PaintedLayer of the parent scroll box
> contents, and they will be the only part of that layer that sticks that far
> out. That honestly sounds like it would look worse.
The scrolled contents of the tall iframe would be drawn to the same point as the scrollbars, no? (They'd be in different layers, but they line up in the final composited image.)
I agree that it's not the most important problem, but since we're fixing this I figured why not fix it properly?
Comment 23•9 years ago
|
||
(In reply to Timothy Nikkel (:tn) from comment #22)
> The scrolled contents of the tall iframe would be drawn to the same point as
> the scrollbars, no? (They'd be in different layers, but they line up in the
> final composited image.)
Yeah ok. But if the iframe has a border, for example, that border will be shorter than the scrollbars.
I'm trying to think of other problems that this might cause, but it's probably fine. (For example, first I thought we might not draw the checkerboarding background color if the visible region bounds of a PaintedLayer cover the scrollport, but that was wrong - we draw the checkerboarding background color based on the display port, not based on the layer's visible region.)
> I agree that it's not the most important problem, but since we're fixing
> this I figured why not fix it properly?
Sure. I wasn't convinced that doing this was closer to "properly" than not doing it, but I think I am now.
Markus, can you take a look at this since tn is out this week?
The original suggestion to use the overflow rect, and then clamp it to the displayport, doesn't really work with classic scrollbars. The problem is that displayports are not enough to figure out where the scrollbars are.
The overflow rect solved this, but clamping it is not enough. Say we have a really tall iframe, with the outer frame scrolled to the top. We clamp the vertical scrollbar's height and this works. But if we clamp the horizontal scrollbar's width, we're ignoring the fact that it's way outside any displayport, and this will cause the same texture-overflow problem. The same is true for the vertical scrollbar if the iframe is extremely wide, and the outer frame is scrolled to the left.
So we need to factor in the outer dirty rect somehow.
This is a much simpler patch that just takes the union of the outer dirty rect and the displayport, but I think it's possible that we could still create an oversize texture. I'm also not sure if we still need to factor in resolution here, since the original code did not.
Attachment #8694020 -
Attachment is obsolete: true
Attachment #8698708 -
Flags: review?(mstange)
This much simpler fix addresses the crash and should not regress the original zooming bug. Looks green on try.
Attachment #8698708 -
Attachment is obsolete: true
Attachment #8698708 -
Flags: review?(mstange)
Attachment #8699309 -
Flags: review?(mstange)
Comment 26•9 years ago
|
||
Comment on attachment 8699309 [details] [diff] [review]
fix v3
Review of attachment 8699309 [details] [diff] [review]:
-----------------------------------------------------------------
::: layout/generic/nsGfxScrollFrame.cpp
@@ +2715,5 @@
> appendToTopFlags |= APPEND_SCROLLBAR_CONTAINER;
> }
>
> // The display port doesn't necessarily include the scrollbars, so just
> + // include all of the scrollbars if we have a display port. We only do
You decided to do this even if we don't have a display port, please fix the comment.
@@ +2719,5 @@
> + // include all of the scrollbars if we have a display port. We only do
> + // this for the root scrollframe of the root content document, which is
> + // zoomable, and where the scrollbar sizes are bounded by the widget.
> + nsRect dirty = mIsRoot && mOuter->PresContext()->IsRootContentDocument()
> + ? scrollParts[i]->GetVisualOverflowRectRelativeToParent()
end-of-line whitespace
Attachment #8699309 -
Flags: review?(mstange) → review+
Comment 27•9 years ago
|
||
I had to back this out for causing reftest bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=18737672&repo=mozilla-inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/6dfa4c059395
Flags: needinfo?(dvander)
Assignee | ||
Comment 29•9 years ago
|
||
dvander is on PTO, I'll look into the reftest failure.
Assignee: dvander → bugmail.mozilla
Flags: needinfo?(dvander)
Assignee | ||
Comment 30•9 years ago
|
||
So the failing reftest has a missing scrollbar inside an <iframe>. Since the iframe isn't a root content document, we just use the aDirtyRect as |dirty| with this patch. Unfortunately aDirtyRect doesn't seem to include the horizontal scrollbar area, probably because the iframe is only scrollable horizontally and so the displayport's height doesn't extend beyond the scrollport height.
I'll try a few things here to get a better idea of what the right fix is.
Assignee | ||
Comment 31•9 years ago
|
||
So fundamentally the problem exposed by that test case is that the scrollbar area may not get included into any of {the displayport, the aDirtyRect, the composited size} in some circumstances. In this case there is a div that is scrollable horizontally but not vertically, and so the displayport height is restricted to the content height which stops just above the horizontal scrollbar.
After thinking about it I came up with a fix which seems to work and pass the test. Try push at [1]. In a nutshell what it does is use the displayport as the bounding rect for the scrollpart dirty rect, but it expands the displayport to include the scrollbar margins if the edge of the displayport lines up with the edge of the scrollport. In a sense it's not very nice because it's discontinuous (the bounding rect expands suddenly once it hits the edge) but it seemed like a relatively straightforward way of accounting for the scrollbar area.
[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=7f8327c7bbfd
Assignee | ||
Comment 32•9 years ago
|
||
Try is green, and this fixes it for me locally. What do you think?
Attachment #8700773 -
Flags: review?(tnikkel)
Assignee | ||
Comment 33•9 years ago
|
||
Comment on attachment 8700773 [details] [diff] [review]
Alternate fix
I was thinking about this more last night and I don't think this fix makes conceptual sense. Instead I feel like there should be a clipstate somewhere that's inherited from the parent and that we should use to clip the scrollParts[i]->GetVisualOverflowRectRelativeToParent
Attachment #8700773 -
Flags: review?(tnikkel)
Assignee | ||
Updated•9 years ago
|
Attachment #8699309 -
Flags: checkin-
Assignee | ||
Comment 34•9 years ago
|
||
Another fix that works, but tn says that changing the clip is too heavyweight and can cause unnecessary invalidation/redrawing. He'll look into fixing up the v2 patch.
Comment 35•9 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #31)
> So fundamentally the problem exposed by that test case is that the scrollbar
> area may not get included into any of {the displayport, the aDirtyRect, the
> composited size} in some circumstances.
The display port is for the scrolled content, so there's no reason it should include the scrollbars.
The composition size by definition excludes the scrollbars.
The dirty rect should include the scroll bars. The reason it doesn't is because in nsLayoutUtils::PaintFrame/nsSubDocumentFrame::BuildDisplayList we change the dirtyrect to the displayport, so the dirty rect that the root scroll frame sees is already the displayport. However, I don't think we need that behaviour, and I'd like to change it. I'll do that in another bug.
Comment 36•9 years ago
|
||
(In reply to Wes Kocher (:KWierso) from comment #28)
> I had to back this out for causing reftest bustage like
> https://treeherder.mozilla.org/logviewer.html#?job_id=18737672&repo=mozilla-
> inbound
The reason this fails is that it essentially backed out http://hg.mozilla.org/mozilla-central/rev/9d88161338f3 for non root content documents. The dirtyrect is the displayport for root scroll frames, and the displayport doesn't have to include the scrollbars. As kats mentioned. I'll change this.
Comment 37•9 years ago
|
||
(In reply to Timothy Nikkel (:tn) from comment #35)
> The dirty rect should include the scroll bars. The reason it doesn't is
> because in nsLayoutUtils::PaintFrame/nsSubDocumentFrame::BuildDisplayList we
> change the dirtyrect to the displayport, so the dirty rect that the root
> scroll frame sees is already the displayport. However, I don't think we need
> that behaviour, and I'd like to change it. I'll do that in another bug.
That bug is bug 1234725.
Assignee | ||
Updated•9 years ago
|
status-firefox46:
--- → affected
Comment 38•9 years ago
|
||
Comment 39•9 years ago
|
||
I pushed this again since bug 1234725 is now fixed. I had a try push where the two patches were green, but I don't have the link handy anymore.
Comment 40•9 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Assignee | ||
Updated•9 years ago
|
Attachment #8699309 -
Flags: checkin- → checkin+
Updated•9 years ago
|
Flags: qe-verify+
QA Contact: vasilica.mihasca
Comment 41•9 years ago
|
||
Using a macbook pro-retina OS X 10.9.5 with Nightly 2015-10-13, I couldn't reproduce the crash, but I encountered freezes.
With this patch, the scroll is smoother, with some side effects:
- a black vertical line covers the right side of the table or the elements are not aligned: http://i.imgur.com/T07W4Qk.png and http://i.imgur.com/FuYNVcA.png
- the border from the attached testcase flicks sometimes: http://i.imgur.com/xucg705.png
- background is visible while scrolling fast using the trackpad: http://i.imgur.com/lteQQip.png
- white areas, rarely: http://i.imgur.com/mIQyv25.png
Kats, could you please let me know which of the above should be filled as follow-ups for this bug? Thank you!
Flags: qe-verify+ → needinfo?(bugmail.mozilla)
Assignee | ||
Comment 42•9 years ago
|
||
Can you reproduce either of these two issues on a recent Nightly?
(In reply to Petruta Rasa [QA] [:petruta] from comment #41)
> - a black vertical line covers the right side of the table or the elements
> are not aligned: http://i.imgur.com/T07W4Qk.png and
> http://i.imgur.com/FuYNVcA.png
> - the border from the attached testcase flicks sometimes:
> http://i.imgur.com/xucg705.png
If you can reproduce these on a recent Nightly please file bugs for them. The other two sound like regular checkerboarding and it's not necessary to file any issue. Thanks!
Flags: needinfo?(bugmail.mozilla)
Comment 43•9 years ago
|
||
Thanks!
I filled bug 1258711 and bug 1258746 (dupe of bug 1256677) for mentioned issues and I'm marking this one as verified.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•