[webkit-reviews] review granted: [Bug 201402] REGRESSION (r249367): m_decodingPromises grows indefinitely until ImageLoader destruction : [Attachment 377964] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 201402: REGRESSION (r249367): m_decodingPromises grows indefinitely until
ImageLoader destruction
https://bugs.webkit.org/show_bug.cgi?id=201402

Attachment 377964: Patch

https://bugs.webkit.org/attachment.cgi?id=377964&action=review




--- Comment #3 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 377964
  --> https://bugs.webkit.org/attachment.cgi?id=377964
Patch

Thank you for looking into this so quickly and writing a test! On a
tangentially relate note there might be a better solution:

1. Making the reject*() resolve*() code private member functions since they
only operate on class instance data.
2. .clear() is ever so slightly less efficient compared to WTFMove()ing the
m_decodingPromises into a local since .clear() calls .shrink(0) and that
function has quite a few branches and more inlined function calls to restore
inline capacity, etc etc. And m_decodingPromises has no inline capacity. In
comparison, there is less code emitted (less branches and less inlined function
calls) needed to move a Vector (just a swap()) and construct an empty Vector.
3. The only use for pendingDecodePromisesCount() is for testing. Maybe adding a
ForTesting() suffix to its name would make this clear to people that they
should not use it for non-testing purposes.