If RenderBlockFlow::adjustLinePositionForPagination() encounters a line with no root box, the result will be a crash with a null pointer dereference.
<rdar://problem/57073037>
Created attachment 388472 [details] Patch
Comment on attachment 388472 [details] Patch Is everything in that test case needed to trigger the crash (there is a lot of craziness in there)? Seems like it can probably be reduced further?
(In reply to Sam Weinig from comment #3) > Comment on attachment 388472 [details] > Patch > > Is everything in that test case needed to trigger the crash (there is a lot > of craziness in there)? Seems like it can probably be reduced further? Yeah, I tried to reduce the number of "strange" things in the test case that would still trigger the crash, and this was what I came up with. If someone more familiar with the code knows a more direct means to trigger the same state/crash, I can't say how happy I'd be to replace this test case. :)
Created attachment 388565 [details] Test reduction
Comment on attachment 388472 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=388472&action=review > Source/WebCore/rendering/RenderBlockFlow.cpp:1813 > + if (!firstRootBox) > + return; I think what we want here is to ignore the firstRootBox when it is nullptr but still call setPaginationStrut() (not that it matters since this is such an edge case) (and please use the attached test reduction)
(In reply to Doug Kelly from comment #4) > (In reply to Sam Weinig from comment #3) > > Comment on attachment 388472 [details] > > Patch > > > > Is everything in that test case needed to trigger the crash (there is a lot > > of craziness in there)? Seems like it can probably be reduced further? > > Yeah, I tried to reduce the number of "strange" things in the test case that > would still trigger the crash, and this was what I came up with. If someone > more familiar with the code knows a more direct means to trigger the same > state/crash, I can't say how happy I'd be to replace this test case. :) Oh drat, looks like Zalan beat me to it. His is good.
Created attachment 388731 [details] Patch
Thanks Zalan/Sam. :) Incorporated the suggestions, and the much simpler test case.
Comment on attachment 388731 [details] Patch Would be even better if the test covered correct behavior as well as the lack of a crash.
(In reply to Darin Adler from comment #10) > Comment on attachment 388731 [details] > Patch > > Would be even better if the test covered correct behavior as well as the > lack of a crash. I agree — testing the behavior here would be best, but I’ll need help from someone (Sam or Zalan?) to understand what the correct behavior in this case is. Thanks!
Comment on attachment 388731 [details] Patch Clearing flags on attachment: 388731 Committed r255437: <https://trac.webkit.org/changeset/255437>
All reviewed patches have been landed. Closing bug.