Closed Bug 1489770 Opened 6 years ago Closed 6 years ago

crash at null in [@ nsCSSFrameConstructor::CreateContinuingFrame]

Categories

(Core :: Layout, defect, P3)

Product:
Core
Component:
Layout
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla64
Tracking Flags:
Tracking Status
firefox-esr60 --- wontfix
firefox62 --- wontfix
firefox63 --- fixed
firefox64 --- fixed

People

(Reporter: tsmith, Assigned: MatsPalmgren_bugz)

References

(Blocks 1 open bug)

Details

(Keywords: crash, testcase)

Attachments

(2 files, 1 obsolete file)

testcase.html
434 bytes, text/html
Details
Don't convert InlineBreak::Before reflow status to Incomplete unless we know the child frame is splittable.
5.51 KB, patch
dholbert
: review+
pascalc
: approval-mozilla-beta+
Details | Diff | Splinter Review
Attached file testcase.html — 
==5947==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fdc9727d291 bp 0x7ffdb1b1d030 sp 0x7ffdb1b1cec0 T0)
==5947==The signal is caused by a WRITE memory access.
==5947==Hint: address points to the zero page.
    #0 0x7fdc9727d290 in nsCSSFrameConstructor::CreateContinuingFrame(nsPresContext*, nsIFrame*, nsContainerFrame*, bool) src/layout/base/nsCSSFrameConstructor.cpp
    #1 0x7fdc975a876d in nsGridContainerFrame::ReflowRowsInFragmentainer(nsGridContainerFrame::GridReflowInput&, mozilla::LogicalRect const&, mozilla::ReflowOutput&, nsReflowStatus&, nsGridContainerFrame::Fragmentainer&, nsSize const&, nsTArray<nsGridContainerFrame::GridItemInfo const*> const&, unsigned int, unsigned int, int, int) src/layout/generic/nsGridContainerFrame.cpp:5601:26
    #2 0x7fdc975a5956 in nsGridContainerFrame::ReflowInFragmentainer(nsGridContainerFrame::GridReflowInput&, mozilla::LogicalRect const&, mozilla::ReflowOutput&, nsReflowStatus&, nsGridContainerFrame::Fragmentainer&, nsSize const&) src/layout/generic/nsGridContainerFrame.cpp:5399:10
    #3 0x7fdc975aaf68 in nsGridContainerFrame::ReflowChildren(nsGridContainerFrame::GridReflowInput&, mozilla::LogicalRect const&, mozilla::ReflowOutput&, nsReflowStatus&) src/layout/generic/nsGridContainerFrame.cpp:5715:13
    #4 0x7fdc975ada77 in nsGridContainerFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsGridContainerFrame.cpp:6038:11
    #5 0x7fdc9740caeb in nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowInput&) src/layout/generic/nsBlockReflowContext.cpp:309:11
    #6 0x7fdc9740032f in nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3571:11
    #7 0x7fdc973fdce4 in nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2921:5
    #8 0x7fdc973f2b4a in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) src/layout/generic/nsBlockFrame.cpp:2458:7
    #9 0x7fdc973e9ea7 in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsBlockFrame.cpp:1292:3
    #10 0x7fdc9744f3a6 in 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
    #11 0x7fdc97453fc2 in nsColumnSetFrame::ReflowChildren(mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&, nsColumnSetFrame::ReflowConfig const&, bool, nsCollapsingMargin*, nsColumnSetFrame::ColumnBalanceData&) src/layout/generic/nsColumnSetFrame.cpp:783:7
    #12 0x7fdc97458ecb in ReflowColumns src/layout/generic/nsColumnSetFrame.cpp:473:19
    #13 0x7fdc97458ecb in nsColumnSetFrame::FindBestBalanceBSize(mozilla::ReflowInput const&, nsPresContext*, nsColumnSetFrame::ReflowConfig&, nsColumnSetFrame::ColumnBalanceData&, mozilla::ReflowOutput&, nsCollapsingMargin&, bool&, bool&, nsReflowStatus&) src/layout/generic/nsColumnSetFrame.cpp:1124
    #14 0x7fdc97459f35 in nsColumnSetFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsColumnSetFrame.cpp:1231:5
    #15 0x7fdc9740caeb in nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowInput&) src/layout/generic/nsBlockReflowContext.cpp:309:11
    #16 0x7fdc9740032f in nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3571:11
    #17 0x7fdc973fdce4 in nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2921:5
    #18 0x7fdc973f2b4a in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) src/layout/generic/nsBlockFrame.cpp:2458:7
    #19 0x7fdc973e9ea7 in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsBlockFrame.cpp:1292:3
    #20 0x7fdc9744f3a6 in 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
    #21 0x7fdc9744d387 in nsCanvasFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsCanvasFrame.cpp:803:5
    #22 0x7fdc9744f3a6 in 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
    #23 0x7fdc9753fe05 in nsHTMLScrollFrame::ReflowScrolledFrame(mozilla::ScrollReflowInput*, bool, bool, mozilla::ReflowOutput*, bool) src/layout/generic/nsGfxScrollFrame.cpp:606:3
    #24 0x7fdc97541344 in nsHTMLScrollFrame::ReflowContents(mozilla::ScrollReflowInput*, mozilla::ReflowOutput const&) src/layout/generic/nsGfxScrollFrame.cpp:730:3
    #25 0x7fdc9754543f in nsHTMLScrollFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsGfxScrollFrame.cpp:1120:3
    #26 0x7fdc973cc66e in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, int, int, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:995:14
    #27 0x7fdc973cb254 in mozilla::ViewportFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/ViewportFrame.cpp:338:7
    #28 0x7fdc971af302 in mozilla::PresShell::DoReflow(nsIFrame*, bool) src/layout/base/PresShell.cpp:9026:11
    #29 0x7fdc971c4de0 in mozilla::PresShell::ProcessReflowCommands(bool) src/layout/base/PresShell.cpp:9199:24
    #30 0x7fdc971c31f9 in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) src/layout/base/PresShell.cpp:4351:11
    #31 0x7fdc94e0298c in FlushPendingNotifications src/obj-firefox/dist/include/nsIPresShell.h:568:5
    #32 0x7fdc94e0298c in FlushPendingEvents src/dom/events/EventStateManager.cpp:5483
    #33 0x7fdc94e0298c in mozilla::EventStateManager::PreHandleEvent(nsPresContext*, mozilla::WidgetEvent*, nsIFrame*, nsIContent*, nsEventStatus*, nsIContent*) src/dom/events/EventStateManager.cpp:690
    #34 0x7fdc971ebd2a in mozilla::PresShell::HandleEventInternal(mozilla::WidgetEvent*, nsEventStatus*, bool, nsIContent*) src/layout/base/PresShell.cpp:7652:19
    #35 0x7fdc971e7c16 in mozilla::PresShell::HandleEvent(nsIFrame*, mozilla::WidgetGUIEvent*, bool, nsEventStatus*) src/layout/base/PresShell.cpp:7297:17
    #36 0x7fdc96b58881 in nsViewManager::DispatchEvent(mozilla::WidgetGUIEvent*, nsView*, nsEventStatus*) src/view/nsViewManager.cpp:812:14
    #37 0x7fdc96b58056 in nsView::HandleEvent(mozilla::WidgetGUIEvent*, bool) src/view/nsView.cpp:1141:9
    #38 0x7fdc96bc24d5 in mozilla::widget::PuppetWidget::DispatchEvent(mozilla::WidgetGUIEvent*, nsEventStatus&) src/widget/PuppetWidget.cpp:409:35
    #39 0x7fdc915a77c0 in mozilla::layers::APZCCallbackHelper::DispatchWidgetEvent(mozilla::WidgetGUIEvent&) src/gfx/layers/apz/util/APZCCallbackHelper.cpp:537:21
    #40 0x7fdc96446108 in DispatchWidgetEventViaAPZ src/dom/ipc/TabChild.cpp:1805:10
    #41 0x7fdc96446108 in mozilla::dom::TabChild::HandleRealMouseButtonEvent(mozilla::WidgetMouseEvent const&, mozilla::layers::ScrollableLayerGuid const&, unsigned long const&) src/dom/ipc/TabChild.cpp:1736
    #42 0x7fdc9644721e in mozilla::dom::TabChild::RecvRealMouseButtonEvent(mozilla::WidgetMouseEvent const&, mozilla::layers::ScrollableLayerGuid const&, unsigned long const&) src/dom/ipc/TabChild.cpp:1708:3
    #43 0x7fdc964473f4 in RecvSynthMouseMoveEvent src/dom/ipc/TabChild.cpp:1669:8
    #44 0x7fdc964473f4 in non-virtual thunk to mozilla::dom::TabChild::RecvSynthMouseMoveEvent(mozilla::WidgetMouseEvent const&, mozilla::layers::ScrollableLayerGuid const&, unsigned long const&) src/dom/ipc/TabChild.cpp
    #45 0x7fdc90573832 in mozilla::dom::PBrowserChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PBrowserChild.cpp:3513:20
    #46 0x7fdc8ff73c58 in mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PContentChild.cpp:5563:28
    #47 0x7fdc8fdf648e in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) src/ipc/glue/MessageChannel.cpp:2239:25
    #48 0x7fdc8fdf33a4 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) src/ipc/glue/MessageChannel.cpp:2166:17
    #49 0x7fdc8fdf4bfc in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:2012:5
    #50 0x7fdc8fdf5258 in mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:2045:15
    #51 0x7fdc8ee8df9e in mozilla::SchedulerGroup::Runnable::Run() src/xpcom/threads/SchedulerGroup.cpp:337:32
    #52 0x7fdc8eebbe0f in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1167:14
    #53 0x7fdc8eec2fa8 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:519:10
    #54 0x7fdc8fdfdf3a in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:97:21
    #55 0x7fdc8fd51b2c in RunInternal src/ipc/chromium/src/base/message_loop.cc:325:10
    #56 0x7fdc8fd51b2c in RunHandler src/ipc/chromium/src/base/message_loop.cc:318
    #57 0x7fdc8fd51b2c in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:298
    #58 0x7fdc96bececa in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:158:27
    #59 0x7fdc9a87ce1f in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:944:22
    #60 0x7fdc8fd51b2c in RunInternal src/ipc/chromium/src/base/message_loop.cc:325:10
    #61 0x7fdc8fd51b2c in RunHandler src/ipc/chromium/src/base/message_loop.cc:318
    #62 0x7fdc8fd51b2c in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:298
    #63 0x7fdc9a87c6e9 in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:770:34
    #64 0x4f2304 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:50:30
    #65 0x4f2304 in main src/browser/app/nsBrowserApp.cpp:287
    #66 0x7fdcae39882f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
    #67 0x421728 in _start (firefox+0x421728)
Flags: in-testsuite?
MOZ_CRASH("unexpected frame type") when trying to create a continuation
for a nsComboboxControlFrame.

nsComboboxControlFrame::Reflow returns aStatus with:
  mInlineBreak = nsReflowStatus::InlineBreak::Before, 
  mCompletion = nsReflowStatus::Completion::FullyComplete, 
which is OK.

nsGridContainerFrame::ReflowInFlowChild just propagates that.

In nsGridContainerFrame::ReflowRowsInFragmentainer we set it
to incomplete here:
https://searchfox.org/mozilla-central/rev/37663bb87004167184de6f2afa6b05875eb0528e/layout/generic/nsGridContainerFrame.cpp#5551
Assignee: nobody → mats
OS: Unspecified → All
Priority: -- → P3
Hardware: Unspecified → All
Comment on attachment 9007665 [details] [diff] [review]
Don't convert InlineBreak::Before reflow status to Incomplete unless we know the child frame is splittable.

Review of attachment 9007665 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, one nit:

::: layout/generic/nsGridContainerFrame.cpp
@@ +5478,5 @@
>      }
>  
>      // aFragmentainer.mIsTopOfPage is propagated to the child reflow state.
> +    // When it's false the child may request InlineBreak::Before.  We set it
> +    // it to false when the row is growable (as determined in CSS Grid

s/set it it/set it/

("it" is repeated across linebreak)
Attachment #9007665 - Flags: review?(dholbert) → review+
My bad, childStatus.SetIncomplete() doesn't reset the BreakBefore
state as I assumed here, so the testcase in bug 1490032 triggers
the new assertion I added (!childStatus.IsInlineBreakBefore()).

(I tend to think Set[Overflow]Incomplete() should reset it since
the completion state is invalid if there's a BreakBefore but
let's deal with that separately.)

So the interdiff here is:
-        if (!child->GetNextInFlow()) {
-          childStatus.Reset();  // report that it's complete
-        } else {
+        childStatus.Reset();
+        if (child->GetNextInFlow()) {
           // The child already has a fragment, so we know it's splittable.
           childStatus.SetIncomplete();
-        }
+        } // else, report that it's complete
Attachment #9007956 - Flags: review?(dholbert)
Attachment #9007665 - Attachment is obsolete: true
Attachment #9007956 - Flags: review?(dholbert) → review+
Pushed by mpalmgren@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/21fc8a773b28
Don't convert InlineBreak::Before reflow status to Incomplete unless we know the child frame is splittable.  r=dholbert
(Filed bug 1490422 about catching missing Reset(), or doing it automatically)
Severity: normal → critical
Flags: in-testsuite? → in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/21fc8a773b28
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox64: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
mats, this bug is marked as affecting 63, should we consider uplifting the patch to beta while we are early in the beta cycle or can it ride the trains? Thanks
Flags: needinfo?(mats)
Sure, this seems like a low-risk change to me.
Flags: needinfo?(mats)
Please request Beta approval on this patch then :)
Flags: needinfo?(mats)
Comment on attachment 9007956 [details] [diff] [review]
Don't convert InlineBreak::Before reflow status to Incomplete unless we know the child frame is splittable.

Approval Request Comment
[Feature/Bug causing the regression]:grid fragmentation feature
[User impact if declined]:crash
[Is this code covered by automated tests?]:yes
[Has the fix been verified in Nightly?]:yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]:no
[Why is the change risky/not risky?]:trivial fix, only affects grid fragmentation
[String changes made/needed]:none
Flags: needinfo?(mats)
Attachment #9007956 - Flags: approval-mozilla-beta?
Comment on attachment 9007956 [details] [diff] [review]
Don't convert InlineBreak::Before reflow status to Incomplete unless we know the child frame is splittable.

Approved for 63 beta 7, thanks.
Attachment #9007956 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: