Bug 224019

Summary: List of extents should be bounds-checked when iterating display list items
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: WebKit2Assignee: 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 Flags
Patch none

Description Wenson Hsieh 2021-03-31 14:22:16 PDT
<rdar://problem/71851600>
Comment 1 Wenson Hsieh 2021-03-31 15:17:31 PDT
Created attachment 424828 [details]
Patch
Comment 2 Tim Horton 2021-03-31 15:21:55 PDT
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?
Comment 3 Wenson Hsieh 2021-03-31 15:27:11 PDT
(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.
Comment 4 EWS 2021-03-31 18:39:41 PDT
Committed r275334: <https://commits.webkit.org/r275334>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424828 [details].