Clarify ImageDecoder::SetMemoryAllocator requirements.

ImageDecoder::SetMemoryAllocator() said it did not work for multi-frame
images. But it did not describe why.

This CL adds a comment explaining the reasoning behind it.

Change-Id: I8f2f0fa5e36f8eed938b5bfe17f5e82995fd2bd2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2236927Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Chris Blume <cblume@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786105}

third_party/blink/renderer/platform/graphics/image_decoder_wrapper.cc

@@ -202,9 +202,13 @@ bool ImageDecoderWrapper::Decode(ImageDecoderFactory* factory,
 bool ImageDecoderWrapper::ShouldDecodeToExternalMemory(
     size_t frame_count,
     bool resume_decoding) const {
-  // Multi-frame images need their decode cached in the decoder to allow using
-  // subsequent frames to be decoded by caching dependent frames.
-  // Also external allocators don't work for multi-frame images right now.
+  // Some multi-frame images need their decode cached in the decoder to allow
+  // future frames to reference previous frames.
+  //
+  // This implies extra requirements on external memory allocators for
+  // multi-frame images. However, there is no enforcement of these extra
+  // requirements. As a result, do not attempt to use external memory
+  // allocators for multi-frame images.
   if (generator_->IsMultiFrame())
     return false;
 

third_party/blink/renderer/platform/image-decoders/image_decoder.h

@@ -390,7 +390,14 @@ class PLATFORM_EXPORT ImageDecoder {
   virtual bool HotSpot(IntPoint&) const { return false; }
 
   virtual void SetMemoryAllocator(SkBitmap::Allocator* allocator) {
-    // FIXME: this doesn't work for images with multiple frames.
+    // This currently doesn't work for images with multiple frames.
+    // Some animated image formats require extra guarantees:
+    // 1. The memory is cheaply readable, which isn't true for GPU memory, and
+    // 2. The memory's lifetime will persist long enough to allow reading past
+    //   frames, which isn't true for discardable memory.
+    // Not all animated image formats share these requirements. Blocking
+    // all animated formats is overly aggressive. If a need arises for an
+    // external memory allocator for animated images, this should be changed.
     if (frame_buffer_cache_.IsEmpty()) {
       // Ensure that InitializeNewFrame is called, after parsing if
       // necessary.