Bug 209996

Summary: Additional sanity checks in compareAnimationsByCompositeOrder()
Product: WebKit Reporter: Doug Kelly <dougk>
Component: AnimationsAssignee: Doug Kelly <dougk>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, ggaren, graouts, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Doug Kelly 2020-04-03 18:28:45 PDT
compareAnimationsByCompositeOrder() is used by std::sort which requires strict weak ordering.  Add additional checks to ensure strict weak ordering is maintained.

<rdar://problem/60199826>
Comment 1 Doug Kelly 2020-04-03 18:37:15 PDT
Created attachment 395427 [details]
Patch
Comment 2 Geoffrey Garen 2020-04-04 08:27:31 PDT
Comment on attachment 395427 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=395427&action=review

> Source/WebCore/animation/WebAnimationUtilities.cpp:87
> +        } else
> +            return !rhsIsCSSAnimation;

If you put this code first as an early return, you can eliminate the nested indentation for twenty lines of code above. I think that would read more clearly.
Comment 3 Doug Kelly 2020-04-04 08:35:54 PDT
Comment on attachment 395427 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=395427&action=review

>> Source/WebCore/animation/WebAnimationUtilities.cpp:87
>> +            return !rhsIsCSSAnimation;
> 
> If you put this code first as an early return, you can eliminate the nested indentation for twenty lines of code above. I think that would read more clearly.

I think that makes sense... it can probably apply to the same case above, too. :)
Comment 4 Doug Kelly 2020-04-04 09:13:55 PDT
Created attachment 395450 [details]
Patch
Comment 5 Geoffrey Garen 2020-04-04 15:04:25 PDT
Comment on attachment 395450 [details]
Patch

r=me
Comment 6 EWS 2020-04-04 15:39:23 PDT
Committed r259538: <https://trac.webkit.org/changeset/259538>

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