Move gif image decoder to SkCodec

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

+// 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.
+
+#ifndef SegmentStream_h
+#define SegmentStream_h
+
+#include <algorithm>
+#include "platform/image-decoders/SegmentReader.h"
+#include "platform/wtf/RefPtr.h"
+#include "third_party/skia/include/core/SkStream.h"
+
+namespace blink {
+
+class PLATFORM_EXPORT SegmentStream : public SkStream {
+ public:
+  SegmentStream();
+  SegmentStream(SegmentStream&& rhs);
+  virtual ~SegmentStream() {}

[scroggo_chromium, 2017/08/08 19:39:25]

Should this instead be:
  ~SegmentStream() override
since it is overriding SkStream's destructor?

Also, Chromium generally recommends not inlining destructors [1]. If you move
this definition into the implementation file (along with that of getLength), can
you change SegmentStream from an include to a forward declare?

[1]
https://www.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-Stop-i...

[cblume, 2017/08/09 17:29:29]

Yes.
 
On 2017/08/08 19:39:25, scroggo_chromium wrote:
I strongly agree with this. *Especially* because it is virtual.
Granted, now-a-days compilers do de-virtualization when they can. But still.

I was following SkStream's style, from which this class derives:
https://cs.chromium.org/chromium/src/third_party/skia/include/core/SkStream.h...
But maybe Skia follows a different style guide or this is older code that wasn't
updated since random style updates are not the best use of time.

Anyway, fixed. :)

[scroggo_chromium, 2017/08/09 17:50:48]

Yeah, that code is pretty old. Skia's official style guide is at
https://skia.org/dev/contrib/style, but it is far from exhaustive.

+
+  SegmentStream& operator=(SegmentStream&& rhs);
+
+  void SetReader(WTF::PassRefPtr<SegmentReader>);
+  // If a buffer has shrunk beyond the point we have read, it has been cleared.
+  // This allows clients to be aware of when data suddenly disappears.
+  bool IsCleared() const { return is_cleared_; }
+
+  // From SkStream:
+  size_t read(void* buffer, size_t) override;
+  size_t peek(void* buffer, size_t) const override;
+  bool isAtEnd() const override { return has_read_all_contents_; }
+  bool rewind() override;
+  bool hasPosition() const override { return true; }
+  size_t getPosition() const override { return position_; }
+  bool seek(size_t position) override;
+  bool move(long offset) override;
+  bool hasLength() const override { return true; }
+  size_t getLength() const override {
+    if (reader_)
+      return reader_->size();
+
+    return 0;
+  }
+
+ private:
+  void SetPositionState(size_t new_position);
+
+  WTF::RefPtr<SegmentReader> reader_;
+  size_t position_ = 0;
+  // |has_read_all_contents_| indicates if we have read all the contents of the
+  // buffer.
+  bool has_read_all_contents_ = true;
+  // |is_cleared_| tracks if we have shrunk the buffer beyond the point we have
+  // read. Clients of this class rely on data not disappearing, so we need to
+  // handle the case when it does.
+  bool is_cleared_ = true;
+};
+
+}  // namespace blink
+
+#endif