[webkit-reviews] review granted: [Bug 174122] HTMLVideoElement with a broken poster image will take square dimension : [Attachment 378581] A updated patch after discussion with Daniel
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Sep 13 15:33:23 PDT 2019
Daniel Bates <dbates at webkit.org> has granted Peng Liu <peng.liu6 at apple.com>'s
request for review:
Bug 174122: HTMLVideoElement with a broken poster image will take square
dimension
https://bugs.webkit.org/show_bug.cgi?id=174122
Attachment 378581: A updated patch after discussion with Daniel
https://bugs.webkit.org/attachment.cgi?id=378581&action=review
--- Comment #12 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 378581
--> https://bugs.webkit.org/attachment.cgi?id=378581
A updated patch after discussion with Daniel
View in context: https://bugs.webkit.org/attachment.cgi?id=378581&action=review
Your change log message is very descriptive. I like it.
> Source/WebCore/rendering/RenderImage.cpp:463
> + RefPtr<Image> image = brokenImageAndImageScaleFactor.first;
It would be slightly more optimal to use auto& here instead of RefPtr<> (you'll
need to * the right-hand side) because brokenImageAndImageScaleFactor.first is
the broken image, which is guaranteed never be nullptr and exist for the entire
program lifetime. So, no need to ref it.
Optional: This doesn't need to be done in this patch callers of
CachedImage::brokenImage() would benefit if it returned a
std::pair<std::reference_wrapper<const Image>, float> instead of
std::pair<Image*, float>. If std::pair<std::reference_wrapper<const Image>,
float> doesn't work, then std::pair<std::reference_wrapper<Image>, float>.
More information about the webkit-reviews
mailing list