Move gif image decoder to SkCodec

third_party/WebKit/Source/platform/image-decoders/SegmentStream.cpp

Index: third_party/WebKit/Source/platform/image-decoders/SegmentStream.cpp
diff --git a/third_party/WebKit/Source/platform/image-decoders/SegmentStream.cpp b/third_party/WebKit/Source/platform/image-decoders/SegmentStream.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..c5cf2e7dcb6a46f642eb0878f7c210286e8464fc
--- /dev/null
+++ b/third_party/WebKit/Source/platform/image-decoders/SegmentStream.cpp
@@ -0,0 +1,107 @@
+// Copyright (c) 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "platform/image-decoders/SegmentStream.h"
+
+namespace blink {
+
+SegmentStream::SegmentStream()
+    : reader_(),
+      position_(0),
+      has_read_all_contents_(true),
+      is_cleared_(true) {}
+
+void SegmentStream::SetReader(SegmentReader* reader,
+                              bool all_contents_received) {

[scroggo_chromium, 2017/04/17 20:04:56]

It looks like this parameter is unused.

[cblume, 2017/04/20 03:42:26]

Done.

[scroggo_chromium, 2017/04/21 20:09:31]

You've commented out the parameter, but why not remove it entirely if it's not
used?

[cblume, 2017/04/21 23:47:38]

Oops. Thought it was part of an interface we were adhering to. Fixed.

+  reader_ = reader;
+  if (reader) {
+    has_read_all_contents_ = reader->size() == position_;

[scroggo_chromium, 2017/04/17 20:04:56]

If the position > reader->size(), should has_read_all_contents_ not be true?

[cblume, 2017/04/20 03:42:26]

Done.

+    is_cleared_ = position_ > reader_->size();
+  } else {
+    has_read_all_contents_ = true;
+    is_cleared_ = true;
+  }
+}
+
+size_t SegmentStream::read(void* buffer, size_t size) {
+  if (is_cleared_)
+    return 0;
+
+  size = std::min(size, reader_->size() - position_);
+
+  size_t bytes_advanced = 0;
+  if (!buffer) {  // skipping, not reading
+    bytes_advanced = size;
+  } else {
+    bytes_advanced = peek(buffer, size);
+  }
+
+  position_ += bytes_advanced;
+  has_read_all_contents_ = position_ == reader_->size();
+
+  return bytes_advanced;
+}
+
+size_t SegmentStream::peek(void* buffer, size_t size) const {
+  if (is_cleared_)
+    return 0;
+
+  size = std::min(size, reader_->size() - position_);
+
+  size_t peek_position = position_;
+  size_t total_bytes_peeked = 0;
+  char* buffer_as_char_ptr = reinterpret_cast<char*>(buffer);
+  while (size) {
+    const char* segment = nullptr;
+    size_t bytes_peeked = reader_->GetSomeData(segment, peek_position);
+    if (!bytes_peeked)
+      break;
+    if (bytes_peeked > size)
+      bytes_peeked = size;
+
+    memcpy(buffer_as_char_ptr, segment, bytes_peeked);
+    buffer_as_char_ptr += bytes_peeked;
+    size -= bytes_peeked;
+    total_bytes_peeked += bytes_peeked;
+    peek_position += bytes_peeked;
+  }
+
+  return total_bytes_peeked;
+}
+
+bool SegmentStream::rewind() {
+  position_ = 0;
+  has_read_all_contents_ = true;

[scroggo_chromium, 2017/04/17 20:04:56]

Why is this set to true? Should this be in an "else" to the "if (reader_)"?

[cblume, 2017/04/20 03:42:26]

Done.

+  if (reader_)
+    has_read_all_contents_ = position_ == reader_->size();
+
+  return true;
+}
+
+bool SegmentStream::seek(size_t position) {
+  has_read_all_contents_ = true;

[scroggo_chromium, 2017/04/17 20:04:56]

Similarly, why did this get set to true?

[cblume, 2017/04/20 03:42:26]

Done.

+  if (reader_) {
+    position = std::min(position, reader_->size());
+    has_read_all_contents_ = position == reader_->size();

[scroggo_chromium, 2017/04/17 20:04:56]

This gets repeated a lot. Maybe add an inlined method?

[cblume, 2017/04/20 03:42:26]

Done.

+  }
+  position_ = position;

[scroggo_chromium, 2017/04/17 20:04:56]

It seems odd to me that we correct position if there is a reader_, but otherwise
we blindly use the input. Maybe that's okay though.

[cblume, 2017/04/20 03:42:26]

I agree.
We cannot correct without a reader. So my only other option would be to ignore
the input. Given those two options I feel like blindly not correcting is the
better move.

[scroggo_chromium, 2017/04/21 20:09:31]

Maybe add a comment?

[cblume, 2017/04/21 23:47:38]

Done.

+
+  return true;
+}
+
+bool SegmentStream::move(long offset) {
+  long absolute_position = position_ + offset;
+
+  // clamp inside the bounds of the buffer size
+  absolute_position = std::max(absolute_position, 0l);
+  absolute_position =
+      std::min(static_cast<size_t>(absolute_position), reader_->size());
+
+  position_ = absolute_position;
+  has_read_all_contents_ = position_ == reader_->size();
+
+  return true;
+}
+
+}  // namespace blink