1464113 - ImageShapeInfo with shape-margin can't handle shapes offset left or above their margin rects

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Reproduced with m-c:
BuildID=20180523220103
SourceStamp=47e81ea1ef10189ef210867934bf36e14cf223dc

Assertion failure: offsetPoint.x >= 0 && offsetPoint.y >= 0 (aContentRect should be within aMarginRect, which we need for our math to make sense.), at src/layout/generic/nsFloatManager.cpp:1935

#0 nsFloatManager::ImageShapeInfo::ImageShapeInfo(unsigned char*, int, mozilla::gfx::IntSizeTyped<mozilla::LayoutDevicePixel> const&, int, float, int, nsRect const&, nsRect const&, mozilla::WritingMode, nsSize const&) src/layout/generic/nsFloatManager.cpp:1843:1
#1 mozilla::detail::UniqueSelector<nsFloatManager::ImageShapeInfo>::SingleObject mozilla::MakeUnique<nsFloatManager::ImageShapeInfo, unsigned char*&, int&, mozilla::gfx::IntSizeTyped<mozilla::LayoutDevicePixel>&, int&, float&, int&, nsRect&, nsRect&, mozilla::WritingMode&, nsSize const&>(unsigned char*&, int&, mozilla::gfx::IntSizeTyped<mozilla::LayoutDevicePixel>&, int&, float&, int&, nsRect&, nsRect&, mozilla::WritingMode&, nsSize const&) src/obj-firefox/dist/include/mozilla/UniquePtr.h:680:27
#2 nsFloatManager::ShapeInfo::CreateImageShape(mozilla::UniquePtr<nsStyleImage, mozilla::DefaultDelete<nsStyleImage> > const&, float, int, nsIFrame*, mozilla::LogicalRect const&, mozilla::WritingMode, nsSize const&) src/layout/generic/nsFloatManager.cpp:2845:10
#3 nsFloatManager::FloatInfo::FloatInfo(nsIFrame*, int, int, mozilla::LogicalRect const&, mozilla::WritingMode, nsSize const&) src/layout/generic/nsFloatManager.cpp:2374:20
#4 nsFloatManager::AddFloat(nsIFrame*, mozilla::LogicalRect const&, mozilla::WritingMode, nsSize const&) src/layout/generic/nsFloatManager.cpp:260:13
#5 mozilla::BlockReflowInput::FlowAndPlaceFloat(nsIFrame*) src/layout/generic/BlockReflowInput.cpp:994:19
#6 mozilla::BlockReflowInput::AddFloat(nsLineLayout*, nsIFrame*, int) src/layout/generic/BlockReflowInput.cpp:627:14
#7 nsLineLayout::ReflowFrame(nsIFrame*, nsReflowStatus&, mozilla::ReflowOutput*, bool&) src/layout/generic/nsLineLayout.cpp:966:11
#8 nsBlockFrame::ReflowInlineFrame(mozilla::BlockReflowInput&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) src/layout/generic/nsBlockFrame.cpp:4158:15
#9 nsBlockFrame::DoReflowInlineFrames(mozilla::BlockReflowInput&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) src/layout/generic/nsBlockFrame.cpp:3958:5
#10 nsBlockFrame::ReflowInlineFrames(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3832:9
#11 nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2816:5
#12 nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) src/layout/generic/nsBlockFrame.cpp:2352:7
#13 nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsBlockFrame.cpp:1225:3
#14 nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowInput&) src/layout/generic/nsBlockReflowContext.cpp:306:11
#15 nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3463:11
#16 nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2813:5
#17 nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) src/layout/generic/nsBlockFrame.cpp:2352:7
#18 nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsBlockFrame.cpp:1225:3
#19 nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:951:14
#20 nsCanvasFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsCanvasFrame.cpp:713:5
#21 nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:951:14
#22 nsHTMLScrollFrame::ReflowScrolledFrame(mozilla::ScrollReflowInput*, bool, bool, mozilla::ReflowOutput*, bool) src/layout/generic/nsGfxScrollFrame.cpp:555:3
#23 nsHTMLScrollFrame::ReflowContents(mozilla::ScrollReflowInput*, mozilla::ReflowOutput const&) src/layout/generic/nsGfxScrollFrame.cpp:678:3
#24 nsHTMLScrollFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsGfxScrollFrame.cpp:1055:3
#25 nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, int, int, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:995:14
#26 mozilla::ViewportFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/ViewportFrame.cpp:335:7
#27 mozilla::PresShell::DoReflow(nsIFrame*, bool) src/layout/base/PresShell.cpp:8942:11
#28 mozilla::PresShell::ProcessReflowCommands(bool) src/layout/base/PresShell.cpp:9115:24
#29 mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) src/layout/base/PresShell.cpp:4332:11
#30 nsRefreshDriver::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:1923:16
#31 mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:301:7
#32 mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:320:5
#33 mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:760:5
#34 mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:673:35
#35 mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:574:9
#36 mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) src/layout/ipc/VsyncChild.cpp:68:16
#37 mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PVsyncChild.cpp:167:20
#38 mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:1988:28
#39 mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) src/ipc/glue/MessageChannel.cpp:2136:25
#40 mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) src/ipc/glue/MessageChannel.cpp:2066:17
#41 mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:1912:5
#42 mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:1945:15
#43 nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1090:14
#44 NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:519:10
#45 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:97:21
#46 MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:326:10
#47 MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299:3
#48 nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:157:27
#49 XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:893:22
#50 mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:269:9
#51 MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:326:10
#52 MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299:3
#53 XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:719:34
#54 content_process_main(mozilla::Bootstrap*, int, char**) src/browser/app/../../ipc/contentproc/plugin-container.cpp:50:30
#55 main src/browser/app/nsBrowserApp.cpp:282:18
#56 __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
#57 _start (firefox+0x423434)

Comment 1

[Cameron McCormack (:heycam)]

Brad, looks like this one is shape-outside-related.

Comment 2

[Brad Werth [:bradwerth] | Out until Feb 2025]

Looks like in the process of preventing the ImageShapeInfo distance field code from addressing locations outside the distance field, I made it too cautious about handling shapes that are shifted left or above their margin rects. The assert can go away and some of the other math has to be casted differently. I'll also add tests.

Comment 3

[Brad Werth [:bradwerth] | Out until Feb 2025]

Attachment: Bug 1464113 Part 1: Make ImageShapeInfo tolerate shapes that are shifted left or above their margin rects.

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

Comment 4

[Brad Werth [:bradwerth] | Out until Feb 2025]

Attachment: Bug 1464113 Part 2: Add a WPT reftest of a shape-outside: image with a negative left offset relative to its containing block.

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

Comment 5

[Brad Werth [:bradwerth] | Out until Feb 2025]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=86160534ffcb0b999dfb863f10c4486d4826b563

Comment 6

[Daniel Holbert [:dholbert]]

Comment on attachment 8980811 [details]
Bug 1464113 Part 1: Make ImageShapeInfo tolerate shapes that are shifted left or above their margin rects.

https://reviewboard.mozilla.org/r/246984/#review254174

Comment 7

[Daniel Holbert [:dholbert]]

Comment on attachment 8980812 [details]
Bug 1464113 Part 2: Add a WPT reftest of a shape-outside: image with a negative left offset relative to its containing block.

https://reviewboard.mozilla.org/r/246986/#review254248

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5c8b1c2a4200
Part 1: Make ImageShapeInfo tolerate shapes that are shifted left or above their margin rects. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/494c6daf83df
Part 2: Add a WPT reftest of a shape-outside: image with a negative left offset relative to its containing block. r=dholbert

Comment 9

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/11262 for changes under testing/web-platform/tests

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 11

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/5c8b1c2a4200
https://hg.mozilla.org/mozilla-central/rev/494c6daf83df

Comment 12

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Does this need an uplift request or can it ride the trains?

Comment 14

[Brad Werth [:bradwerth] | Out until Feb 2025]

Since it's just an assertion, and the feature is only enabled by default in Nightly 61, this can ride the trains to 62.