I noticed this while investigating  issue 722072 . I thought GIF was susceptible to the same bug, but it is not, because GIFImageDecoder never calls FindRequiredPreviousFrame(i, true) (i.e. sets frame_rect_is_opaque to true). As a result, GIF frames that follow Keep frames are always marked as depending on the prior frame (at least until decoding; more on that below).

If we knew that a GIF frame was opaque ahead of time (i.e. in InitializeNewFrame), we could potentially mark it as independent (e.g. if it filled the screen). This could be beneficial in the following situation:

- Frame A is Keep and fills the screen
- Frame B is opaque and DisposePrevious
- Frame C depends on A

In the current system, we know that C depends on A (technically, we just assume that it might, since B is DisposePrevious), so we do not TakeBitmapDataIfWritable; instead we copy A into B's ImageFrame. This is unnecessary, though; we could have simply allocated new memory. In addition, if the time to show A had passed and we want to skip it and show B, we currently have to decode A, but we could have skipped it.

After we decode B, we will recognize if it never used the transparent pixel and call ImageDecoder::CorrectAlphaWhenFrameBufferSawNoAlpha. If B fills the screen, we will then mark it as opaque and independent (which may help on a future iteration through the loop [1]), but if it does not, B will remain depending on A and having alpha.

It seems like we *could* know ahead of time that the frame rect is opaque, i.e. if it has no transparent pixel. (SkGifCodec currently checks for that.) But if we trust that, buggy GIF files could still make us wrong:

- If an index is outside the range of the color map, it is treated as transparent. (Only an issue if the color map is smaller than 256, since the indices are 8 bit.) We instead could fill it with an opaque pixel to defeat this. (Not sure how often this happens in practice.)
- If a GIF has a "do-nothing frame" with no rows [2], it is essentially fully transparent. I think we treat this incorrectly, though; since current_buffer_saw_alpha_ will be set to false, if this do-nothing frame "fills the screen" (according to its header), it will be marked as opaque and independent, meaning it may do something strange on the second iteration - if we need to redecode it, it will just be transparent black, marked as opaque.

I'm not aware of any attempts to determine before decoding whether a GIF frame is opaque; it was not considered when setRequiredPreviousFrameIndex was introduced in https://codereview.chromium.org/15350006 or when it was updated to consider an opaque frame (for webp) as in https://chromiumcodereview.appspot.com/23068027 (unless I missed something while looking through the comments), so I do not know whether it was deliberate or an oversight that we do not.

[1] In theory it should also improve draw speed, but IIUC, due to our current deferred decoding implementation, in which the DecodingImageGenerator used to draw the frame was already marked as having alpha, so we still take the slower drawing path.

[2] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp?l=189