1412110 - Assertion failure: !item->GetAbove(), at /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp:3151

Status: RESOLVED FIXED

(Core :: Web Painting, defect)

Description

[Jason Kratzer [:jkratzer]]

Attachment: trigger.html

Testcase found while fuzzing mozilla-central rev d734e6acf777.

Comment 1

[Jason Kratzer [:jkratzer]]

Attachment: log_minidump.txt

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: testcase 2 (reduced, interactive)

The first testcase reloads repeatedly, and I think it crashes when I click it a certain way (without needing the reloads).

I reduced it quite a bit to this testcase, removing a lot of the complexity -- this just combines position:sticky, a list bullet, and contenteditable.

Comment 3

[Daniel Holbert [:dholbert]]

The failing assertion is brand-new, as of bug 1405146.  mattwoodrow, can you take a look?

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: log from brief "rr" debugging session with testcase 2

If it helps, here's a log from a brief "rr" debugging session, including a backtrace and a framedump.

As you can see, the "item" in question is a nsDisplayStickyPosition, and its GetAbove() method returns a nsDisplayCaret* (which is non-null, hence the assertion failure).

(I assume the nsDisplayCaret is the blinking insertion-point caret from contenteditable, or something like that.)

Comment 5

[Daniel Holbert [:dholbert]]

(57 is unaffected, per comment 3)

Comment 6

[Matt Woodrow (:mattwoodrow)]

Attachment: Make sure we create a wraplist when we build a caret item

We have code to manually invalidate the frame when we become/stop being the caret frame, so we don't need to worry about the transition from no wrapper -> wrapper.

Comment 7

[Miko Mynttinen]

Comment on attachment 8924475 [details] [diff] [review]
Make sure we create a wraplist when we build a caret item

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

LGTM. It would be nice to have a comment to explain the logic behind canSkipWrapList and aCreatedContainerItem.

::: layout/generic/nsFrame.cpp
@@ +3505,5 @@
>        // return early.
>  
>        aBuilder->AdjustWindowDraggingRegion(child);
>        child->BuildDisplayList(aBuilder, aLists);
> +      if (aBuilder->DisplayCaret(child, aLists.Content())) {

This change seems unnecessary since we return immediately?

::: layout/painting/nsDisplayList.h
@@ +719,5 @@
>    bool IsCompositingCheap() const { return mIsCompositingCheap; }
>    /**
>     * Display the caret if needed.
>     */
> +  bool DisplayCaret(nsIFrame* aFrame, nsDisplayList* aList) {

Since you touch the line anyway, could you move the brace on a new line?

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c87b4385113
Make sure we build a wrap list for the caret frame, since it will have multiple display items. r=miko

Comment 9

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/7c87b4385113