Move gif image decoder to SkCodec

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

+// 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) {
+  reader_ = reader;
+  UpdateInternals(position_);
+}
+
+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;

[scroggo_chromium, 2017/04/25 15:31:40]

UpdateInternals also updates the position. Why not make this

  UpdateInternals(position_ + bytes_advanced);

?

[cblume, 2017/04/26 10:09:42]

Done.

+  UpdateInternals(position_);
+
+  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() {
+  UpdateInternals(0);
+  return true;
+}
+
+bool SegmentStream::seek(size_t position) {
+  UpdateInternals(position);
+  return true;
+}
+
+bool SegmentStream::move(long offset) {
+  long absolute_position = position_ + offset;
+
+  // clamp inside the bounds of the buffer size

[scroggo_chromium, 2017/04/25 15:31:40]

This comment looks to be out of date. Might be unnecessary anyway

[cblume, 2017/04/26 10:09:41]

Done.

+  absolute_position = std::max(absolute_position, 0l);
+  UpdateInternals(absolute_position);
+  return true;
+}
+
+void SegmentStream::UpdateInternals(size_t new_position) {

[scroggo_chromium, 2017/04/25 15:31:40]

"UpdateInternals" sounds vague to me. What if we called it something like

SetPosition(Internal)
Seek(Internal)

It is used to set the position (and update the other fields that depend on the
position). The one case where we're not modifying the position is SetReader.

[cblume, 2017/04/26 10:09:42]

I agree that there could be a better name.

I am tempted to name it something like OnPositionChanged() but that sounds a bit
like it is a callback.
I'll go with UpdatePositionState().

[scroggo_chromium, 2017/04/26 19:02:22]

That sounds better. What do you think of SetPositionState?

[cblume, 2017/04/26 19:16:06]

SetPositionState sounds great. I'll go with that.

+  position_ = new_position;
+  if (reader_) {
+    const size_t reader_size = reader_->size();
+    has_read_all_contents_ = position_ >= reader_size;
+    is_cleared_ = position_ > reader_size;
+  } else {
+    has_read_all_contents_ = true;
+    is_cleared_ = true;
+  }
+}
+
+}  // namespace blink