914891 - "ASSERTION: Need a scrolling container" and crash (with position:sticky and fixed root)

Status: RESOLVED FIXED

(Core :: Layout: Positioned, defect)

Description

[Jesse Ruderman]

Attachment: testcase (crashes Firefox when loaded)

With:
  user_pref("layout.css.sticky.enabled", true);

###!!! ASSERTION: Need a scrolling container: 'scrollFrame', file layout/generic/StickyScrollContainer.cpp, line 50

Crash [@ mozilla::StickyScrollContainer::StickyScrollContainerForFrame]

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Corey Ford [:coyotebush]]

Ah, we thought we might find some situation without a scroll container. That one makes sense.

Probably I should just bail more gently on anything sticky-related if there isn't a scroll container?

Comment 3

[Corey Ford [:coyotebush]]

That would be easy enough.

Comment 4

[Corey Ford [:coyotebush]]

Although this particular case will be a bit more involved; it's been nice to assume that StickyScrollContainerForFrame was infallible.

Comment 5

[Corey Ford [:coyotebush]]

Huh, looks like that test case *doesn't* crash as a reftest...

Comment 6

[Corey Ford [:coyotebush]]

Attachment: Bail gracefully on sticky positioning with no scroll container.

(The nsComputedDOMStyle.cpp code for sticky already handles this okay.)

Comment 7

[Daniel Holbert [:dholbert]]

Comment on attachment 803293 [details] [diff] [review]
Bail gracefully on sticky positioning with no scroll container.

>diff --git a/layout/generic/StickyScrollContainer.h b/layout/generic/StickyScrollContainer.h
>--- a/layout/generic/StickyScrollContainer.h
>+++ b/layout/generic/StickyScrollContainer.h
>@@ -24,17 +24,17 @@ namespace mozilla {
> 
> class StickyScrollContainer MOZ_FINAL : public nsIScrollPositionListener
> {
> public:
>   /**
>    * Find the StickyScrollContainer associated with the scroll container of
>    * the given frame, creating it if necessary.
>    */
>-  static StickyScrollContainer* StickyScrollContainerForFrame(nsIFrame* aFrame);
>+  static StickyScrollContainer* GetStickyScrollContainerForFrame(nsIFrame* aFrame);


Update the documentation here, to indicate that this can return null & why.

r=me with that.

Comment 8

[Corey Ford [:coyotebush]]

Attachment: Bail gracefully on sticky positioning with no scroll container.

Comment 9

[Corey Ford [:coyotebush]]

Let's double-check. https://tbpl.mozilla.org/?tree=Try&rev=5cb6567a32a5

Comment 10

[Daniel Holbert [:dholbert]]

Try run looks good.

Landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/82d94735ba7f

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/82d94735ba7f