1337696 - stylo: layout/base/crashtests/468645-3.xhtml non-fatally asserts with "unknown out of flow frame type: 'disp->mDisplay == StyleDisplay::MozPopup'"

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1337696. Fix change hint computation for table-outer frames to be more correct.

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

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8842896 [details]
Bug 1337696.  Fix change hint computation for table-outer frames to be more correct.

https://reviewboard.mozilla.org/r/116630/#review118382

r=me, with that question answered. Nice to see that code go away :)

::: layout/base/ServoRestyleManager.cpp
(Diff revision 1)
> -  MOZ_ASSERT(aTableWrapper->StyleContext()->GetPseudo() ==
> -             nsCSSAnonBoxes::tableWrapper);
> -  RefPtr<nsStyleContext> newContext =
> -    aStyleSet->ResolveAnonymousBoxStyle(nsCSSAnonBoxes::tableWrapper,
> -                                        aTableStyleContext);
> -  aTableWrapper->SetStyleContext(newContext);

So this didn't handle continuations before. Under which circumstances can a table wrapper frame be splitted? Was this code buggy, or we can just assert that there are no continuations below?

::: layout/tables/nsTableFrame.h:605
(Diff revision 1)
>                                     bool aIsFirstReflow);
>  
>    virtual bool ComputeCustomOverflow(nsOverflowAreas& aOverflowAreas) override;
>  
> +  // Update the style of our table wrapper frame.
> +  virtual void UpdateStyleOfOwnedAnonBoxes(mozilla::ServoStyleSet* aStyleSet,

I think virtual isn't supposed to be there per the style guide, but given it's also in pretty much all the file, up to you to change it.

Comment 3

[Bobby Holley (:bholley)]

Comment on attachment 8842896 [details]
Bug 1337696.  Fix change hint computation for table-outer frames to be more correct.

This is about change hints, not restyle hints right? Seems like we should update the name of the patch.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> Under which circumstances can a table wrapper frame be splitted?

Printing.  Then again, we generally don't restyle when printing.  If we ever fix tables in columns to work, though, that will split things too, and it's better to not leave a landmine for it.

> I think virtual isn't supposed to be there per the style guide

Yeah... it's complicated wrt existing code style.  I matched it for now; if we ever decide to mass-change it, we can.

> This is about change hints, not restyle hints right?

Yes.  Will resummarize.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8842896 [details]
Bug 1337696.  Fix change hint computation for table-outer frames to be more correct.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116630/diff/1-2/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/53a53f671871
Fix change hint computation for table-outer frames to be more correct.  r=emilio

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8842896 [details]
Bug 1337696.  Fix change hint computation for table-outer frames to be more correct.

https://reviewboard.mozilla.org/r/116630/#review118382

> So this didn't handle continuations before. Under which circumstances can a table wrapper frame be splitted? Was this code buggy, or we can just assert that there are no continuations below?

Printing.  And columnsets once we actually make it work.

The old code was buggy.

> I think virtual isn't supposed to be there per the style guide, but given it's also in pretty much all the file, up to you to change it.

Right, following file style...

Comment 8

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s dc18a9ca923e -d 53a53f671871: rebasing 379464:dc18a9ca923e "Bug 1337696.  Fix change hint computation for table-outer frames to be more correct.  r=emilio"
merging layout/base/ServoRestyleManager.cpp
merging layout/base/crashtests/crashtests.list
merging layout/tables/nsTableFrame.cpp
merging layout/tables/nsTableFrame.h
note: rebase of 379464:dc18a9ca923e created no changes to commit
rebasing 379465:680ff8eeaadc "Bug 1343771.  Fix stylo to properly update styles on the anonymous block inside a table cell.  r=emilio" (tip)
merging layout/generic/nsFrame.cpp
merging layout/generic/nsFrame.h
merging layout/tables/nsTableCellFrame.cpp
merging layout/tables/nsTableCellFrame.h
warning: conflicts while merging layout/generic/nsFrame.h! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/53a53f671871