Optimize yuv decoding fail check and remove the usage of unwanted variable.

third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.cpp

Index: third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.cpp b/third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.cpp
index 5ed0c32cb845f5c8a4052e7b3207a22688b238b1..1c71aca1776a9b45fe0ae41be9faa1cf507d509e 100644
--- a/third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.cpp
+++ b/third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.cpp
@@ -95,19 +95,14 @@ class ExternalMemoryAllocator final : public SkBitmap::Allocator {
   size_t row_bytes_;
 };
 
-static bool UpdateYUVComponentSizes(ImageDecoder* decoder,
+static void UpdateYUVComponentSizes(ImageDecoder* decoder,
                                     SkISize component_sizes[3],
                                     size_t component_width_bytes[3]) {
-  if (!decoder->CanDecodeToYUV())
-    return false;
-
   for (int yuv_index = 0; yuv_index < 3; ++yuv_index) {
     IntSize size = decoder->DecodedYUVSize(yuv_index);
     component_sizes[yuv_index].set(size.Width(), size.Height());
     component_width_bytes[yuv_index] = decoder->DecodedYUVWidthBytes(yuv_index);
   }
-
-  return true;
 }
 
 ImageFrameGenerator::ImageFrameGenerator(const SkISize& full_size,
@@ -117,7 +112,6 @@ ImageFrameGenerator::ImageFrameGenerator(const SkISize& full_size,
       decoder_color_behavior_(color_behavior),
       is_multi_frame_(is_multi_frame),
       decode_failed_(false),
-      yuv_decoding_failed_(false),
       frame_count_(0) {}
 
 ImageFrameGenerator::~ImageFrameGenerator() {
@@ -201,7 +195,6 @@ bool ImageFrameGenerator::DecodeToYUV(SegmentReader* data,
   }
 
   DCHECK(decoder->Failed());
-  yuv_decoding_failed_ = true;

[scroggo_chromium, 2017/06/15 16:50:38]

It sounds like you're saying that yuv_decoding_failed_ is never set to true, so
we don't need to check its value? We set it to true right here, if there was a
failure.

[naga, 2017/06/15 17:49:02]

yuv_decoding_failed _ is set to true once DecodeToYUV() is failed.
https://cs.chromium.org/chromium/src/third_party/skia/src/gpu/GrYUVProvider.c...

In our current source code, yuv_decoding_failed_ check present before calling
the decoder. This function is used to fetch yuv plane sizes for memory
allocation
https://cs.chromium.org/chromium/src/third_party/skia/src/gpu/GrYUVProvider.c...

[scroggo_chromium, 2017/06/15 20:40:00]

Isn't that what I just said? This method is DecodeToYUV().
I'm not sure what you're pointing out to me with the links.

I agree that the first time this method is called, yuv_decoding_failed_ is
guaranteed to be false. Are you telling me that we will never call it a second
time?

   return false;
 }
 
@@ -382,12 +375,9 @@ bool ImageFrameGenerator::GetYUVComponentSizes(SegmentReader* data,
   TRACE_EVENT2("blink", "ImageFrameGenerator::getYUVComponentSizes", "width",
                full_size_.width(), "height", full_size_.height());
 
-  if (yuv_decoding_failed_)
-    return false;
-
   std::unique_ptr<ImageDecoder> decoder = ImageDecoder::Create(
       data, true, ImageDecoder::kAlphaPremultiplied, decoder_color_behavior_);
-  if (!decoder)
+  if (!decoder || !decoder->CanDecodeToYUV())

[scroggo_chromium, 2017/06/15 14:45:22]

This change (moving caller of CanDecodeToYUV from UpdateYUVComponentSizes to
here) seems fine to me. This saves us the creation of dummy image planes if
CanDecodeToYUV returns false, and CanDecodeToYUV does not take into
consideration whether image planes are set.

It seems unrelated to the other change though, and maybe belongs in its own CL.

[scroggo_chromium, 2017/07/06 21:10:18]

It looks like you've partially done that in crrev.com/2953023002, but you
haven't removed it from here. This CL should contain just the one change.

     return false;
 
   // Setting a dummy ImagePlanes object signals to the decoder that we want to
@@ -396,8 +386,10 @@ bool ImageFrameGenerator::GetYUVComponentSizes(SegmentReader* data,
       WTF::WrapUnique(new ImagePlanes);
   decoder->SetImagePlanes(std::move(dummy_image_planes));
 
-  return UpdateYUVComponentSizes(decoder.get(), size_info->fSizes,
-                                 size_info->fWidthBytes);
+  UpdateYUVComponentSizes(decoder.get(), size_info->fSizes,
+                          size_info->fWidthBytes);
+
+  return true;
 }
 
 }  // namespace blink