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)

Product:
Core
Component:
Panning and Zooming
Version:
43 Branch
Platform:
Unspecified
macOS
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla46
Tracking Flags:
Tracking Status
firefox46 --- verified

People

(Reporter: milan, Assigned: kats)

References

Details

(Keywords: regression, Whiteboard: [gfx-noted])

Attachments

(5 files, 2 obsolete files)

Apple crash report
120.92 KB, text/plain
Details
crashtest.html (reduced testcase)
244 bytes, text/html
Details
fix v3
7.17 KB, patch
mstange
: review+
kats
: checkin+
Details | Diff | Splinter Review
Alternate fix
13.07 KB, patch
Details | Diff | Splinter Review
Fix by changing clip
2.17 KB, 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
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
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]
I imagine APZ related, but I will leave it as graphics general until we know.
I don't get the crash but I'm getting -awful- performance. Even my cursor is unresponsive. We're very likely sizing something incorrectly.
Attached file Apple crash report — 
This one is more useful.  Shows up crashing in swap buffers.
(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...
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.
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]
Depends on: 1214386
Regression range looks to be APZ on mac which isn't very interesting.
Flags: needinfo?(bgirard)
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)
Summary: Crash on the mac while scrolling → Crash on the mac while scrolling due to massive scrollbars
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)
Timothy, since this is mostly needed for zooming, how do you feel about only doing this for root documents?
Blocks: 1076447
Flags: needinfo?(tnikkel)
Keywords: regression
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
No longer depends on: 1214386
I believe dvander is working on this now
Assignee: mstange → dvander
Priority: -- → P1
Attached patch fix (obsolete) — — Splinter Review 
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 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 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.
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.
(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.)
(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.
(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?
(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.
Attached patch fix v2 (obsolete) — — Splinter Review 
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)
Attached patch fix v3 — — Splinter Review 
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 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+
dvander is on PTO, I'll look into the reftest failure.
Assignee: dvander → bugmail.mozilla
Flags: needinfo?(dvander)
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.
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
Attached patch Alternate fix — — Splinter Review 
Try is green, and this fixes it for me locally. What do you think?
Attachment #8700773 - Flags: review?(tnikkel)
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)
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.
(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.
(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.
Depends on: 1234725
(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.
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.
https://hg.mozilla.org/mozilla-central/rev/9e172c20b8f9
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox46: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Attachment #8699309 - Flags: checkin- → checkin+
Flags: qe-verify+
QA Contact: vasilica.mihasca
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)
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)
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
status-firefox46: fixed → verified
Regressions: 1548484
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: