[webkit-reviews] review granted: [Bug 219332] [MSE] Move track buffer management from SourceBuffer to SourceBufferPrivate : [Attachment 415187] rebase the patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Peng Liu <peng.liu6 at apple.com>'s
request for review:
Bug 219332: [MSE] Move track buffer management from SourceBuffer to
SourceBufferPrivate
https://bugs.webkit.org/show_bug.cgi?id=219332

Attachment 415187: rebase the patch

https://bugs.webkit.org/attachment.cgi?id=415187&action=review




--- Comment #21 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 415187
  --> https://bugs.webkit.org/attachment.cgi?id=415187
rebase the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=415187&action=review

> Source/WebCore/Modules/mediasource/SourceBuffer.cpp:106
> +    m_private->setIsAttached(false);

Consider flipping the order of these statements to be inverse of constructor
order to be idiomatic: construct A, B, destroy B, A.

> Source/WebCore/platform/graphics/SourceBufferPrivate.cpp:181
> +	   return Vector<String>();

Ok as is. Could return {}

> Source/WebCore/platform/graphics/SourceBufferPrivate.cpp:183
> +    TrackBuffer& trackBuffer = it->value;

Ok as-is. Could use auto.

> Source/WebCore/platform/graphics/SourceBufferPrivate.h:84
> +    TimeRanges* buffered() const { return m_buffered.get(); }

Consider making non-const if possible. General observation: encapsulation not
violated only if either const function returns const data or by value.
Returning non const data from const method still allows caller to mutate data
out from under this object.