ImageDecodeBench packeted runs should resume decoding where it left off

third_party/WebKit/Source/platform/testing/ImageDecodeBench.cpp

Index: third_party/WebKit/Source/platform/testing/ImageDecodeBench.cpp
diff --git a/third_party/WebKit/Source/platform/testing/ImageDecodeBench.cpp b/third_party/WebKit/Source/platform/testing/ImageDecodeBench.cpp
index 4ef02ef74e1dbe60c1868871e3dfae3e2aab0fa7..14d42f1cdf17b6aac12ae9cb9e3d9dfe7d71af6e 100644
--- a/third_party/WebKit/Source/platform/testing/ImageDecodeBench.cpp
+++ b/third_party/WebKit/Source/platform/testing/ImageDecodeBench.cpp
@@ -226,6 +226,7 @@ bool DecodeImageData(SharedBuffer* data,
 
   RefPtr<SharedBuffer> packet_data = SharedBuffer::Create();
   size_t position = 0;
+  size_t next_frame_to_decode = 0;
   while (true) {
     const char* packet;
     size_t length = data->GetSomeData(packet, position);
@@ -238,9 +239,11 @@ bool DecodeImageData(SharedBuffer* data,
     decoder->SetData(packet_data.Get(), all_data_received);
 
     int frame_count = decoder->FrameCount();
-    for (int i = 0; i < frame_count; ++i) {
-      if (!decoder->FrameBufferAtIndex(i))
+    for (int i = next_frame_to_decode; i < frame_count; ++i) {

[scroggo, 2017/05/11 13:56:23]

FWIW, starting from frame 0 doesn't re-decode those first three frames (one
exception - when purging aggressively, they *will* need to be re-decoded). We
don't clear the cache, so they should still be there and complete, so
FrameBufferAtIndex will just return the complete frames.

But you're right, no need to call FrameBufferAtIndex for those frames
(especially for the exception of purging aggressively).

I don't think this change goes far enough, though. FrameCount() should only
return frames that we at least have a piece of, and FrameBufferAtIndex only
returns null if i >= FrameCount(). So I don't think we'll ever hit this case
(meaning that next_frame_to_decode will remain 0). Instead, I think you want
something like the following:

  for (; next_frame_to_decode < frame_count; ++next_frame_to_decode) {
    auto status =
decoder->FrameBufferAtIndex(next_frame_to_decode)->GetStatus();
    if (status != ImageFrame::FrameComplete) {
      break;
    }

[cblume, 2017/05/11 15:45:31]

Ah. I agree.

Since I believe this test is aimed at replicating real-world behavior, I might
want to add the purging as well. I don't want to time it (once I fix the
timing). But the act of purging could impact instruction and data caches. What
do you think?

Also Leon, do you prefer I add your google address, chromium address, or both? I
added your chromium address because you've given LGTMs from it before. But maybe
you want both?

+      if (!decoder->FrameBufferAtIndex(i)) {
+        next_frame_to_decode = i;
         break;
+      }
     }
 
     if (all_data_received || decoder->Failed())