| Summary: | List of extents should be bounds-checked when iterating display list items | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Wenson Hsieh <wenson_hsieh> | ||||
| Component: | WebKit2 | Assignee: | Wenson Hsieh <wenson_hsieh> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | kkinnunen, sabouhallawa, simon.fraser, thorton, webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Wenson Hsieh
2021-03-31 14:22:16 PDT
Created attachment 424828 [details]
Patch
Comment on attachment 424828 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424828&action=review > Tools/TestWebKitAPI/Tests/WebCore/cg/DisplayListTestsCG.cpp:135 > - EXPECT_EQ(result.reasonForStopping, StopReplayReason::InvalidItem); > + EXPECT_EQ(result.reasonForStopping, StopReplayReason::InvalidItemOrExtent); Neither of these tests the extent validation, right? Is it possible to write a test? (In reply to Tim Horton from comment #2) > Comment on attachment 424828 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=424828&action=review > > > Tools/TestWebKitAPI/Tests/WebCore/cg/DisplayListTestsCG.cpp:135 > > - EXPECT_EQ(result.reasonForStopping, StopReplayReason::InvalidItem); > > + EXPECT_EQ(result.reasonForStopping, StopReplayReason::InvalidItemOrExtent); > > Neither of these tests the extent validation, right? Is it possible to write > a test? Thanks for the review! As it turns out, I tried to write a test but unfortunately I don't think it's possible unless I expose `DisplayList::addDrawingItemExtent` as a public method (but that test would be a bit contrived anyways :P). Using just the public interfaces of DisplayList, when appending a drawing item to a display list, we'll always keep the drawing item extents in sync. I also thought of perhaps enabling drawing extent tracking halfway through appending display list items, but then recalled that doing so would simply trip the release assertion that I adjusted slightly in this patch. Committed r275334: <https://commits.webkit.org/r275334> All reviewed patches have been landed. Closing bug and clearing flags on attachment 424828 [details]. |