[webkit-reviews] review granted: [Bug 128717] Fix TimeRanges layering violations : [Attachment 224694] Updated patch

[bugzilla-daemon at webkit.org]

Jer Noble <jer.noble at apple.com> has granted Eric Carlson
<eric.carlson at apple.com>'s request for review:
Bug 128717: Fix TimeRanges layering violations
https://bugs.webkit.org/show_bug.cgi?id=128717

Attachment 224694: Updated patch
https://bugs.webkit.org/attachment.cgi?id=224694&action=review

------- Additional Comments from Jer Noble <jer.noble at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=224694&action=review


r=me, with nits.

> Source/WebCore/Modules/mediasource/MediaSource.cpp:180
> -    return intersectionRanges.release();
> +    auto result = PlatformTimeRanges::create();
> +    unsigned size = intersectionRanges->length();
> +    for (unsigned i = 0; i < size; i++)
> +	   result->add(intersectionRanges->start(i, ASSERT_NO_EXCEPTION),
intersectionRanges->end(i, ASSERT_NO_EXCEPTION));
> +
> +    return result;

This is weird.	Can't we just do:

return PlatformTimeRanges::create(intersectionRanges->ranges()) ?

> Source/WebCore/html/HTMLMediaElement.cpp:3079
> +    auto timeRanges = m_player->buffered();

I don't believe we're supposed to use 'auto' in this way; only when the type is
implied by the method called.

> Source/WebCore/html/HTMLMediaElement.cpp:3082
> -	   double start = timeRanges->start(i, IGNORE_EXCEPTION);
> -	   double end = timeRanges->end(i, IGNORE_EXCEPTION);
> +	   double start = timeRanges->start(i, ignored);
> +	   double end = timeRanges->end(i, ignored);

Why are we removing IGNORE_EXCEPTION?

> Source/WebCore/html/TimeRanges.h:67
> +    const PlatformTimeRanges* ranges() const { return &m_ranges; }

Lets make this public, and have it return a 'const PlatformTimeRanges&'.