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

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

 /*
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 77 matching lines @@
 
     return dst->installPixels(info, pixels_, row_bytes_);
   }
 
  private:
   SkImageInfo info_;
   void* pixels_;
   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,
                                          bool is_multi_frame,
                                          const ColorBehavior& color_behavior)
     : full_size_(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() {
   ImageDecodingStore::Instance().RemoveCacheIndexedByGenerator(this);
 }
 
 bool ImageFrameGenerator::DecodeAndScale(
     SegmentReader* data,
     bool all_data_received,
     size_t index,
@@ skipping 63 matching lines @@
   decoder->SetImagePlanes(std::move(image_planes));
 
   DCHECK(decoder->CanDecodeToYUV());
 
   if (decoder->DecodeToYUV()) {
     SetHasAlpha(0, false);  // YUV is always opaque
     return true;
   }
 
   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;
 }
 
 SkBitmap ImageFrameGenerator::TryToResumeDecode(
     SegmentReader* data,
     bool all_data_received,
     size_t index,
     const SkISize& scaled_size,
     SkBitmap::Allocator* allocator,
     ImageDecoder::AlphaOption alpha_option) {
@@ skipping 160 matching lines @@
   if (index < has_alpha_.size())
     return has_alpha_[index];
   return true;
 }
 
 bool ImageFrameGenerator::GetYUVComponentSizes(SegmentReader* data,
                                                SkYUVSizeInfo* size_info) {
   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
   // do YUV decoding.
   std::unique_ptr<ImagePlanes> dummy_image_planes =
       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