TimeRanges is defined in WebCore/html, but it is used by many of the media engine related classes in WebCore/platform.
Created attachment 224681 [details] Patch for the bots to chew on.
Attachment 224681 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:773: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] ERROR: Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:774: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/platform/graphics/PlatformTimeRanges.cpp:28: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/WebCore/platform/graphics/PlatformTimeRanges.cpp:144: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/platform/graphics/PlatformTimeRanges.cpp:156: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/graphics/PlatformTimeRanges.cpp:171: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 6 in 41 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 224694 [details] Updated patch
Attachment 224694 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:772: Non-label code inside switch statements should be indented. [whitespace/indent] [4] ERROR: Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:774: More than one command on the same line [whitespace/newline] [4] Total errors found: 2 in 43 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 224694 [details] Updated patch 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&'.
Comment on attachment 224694 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=224694&action=review >> Source/WebCore/Modules/mediasource/MediaSource.cpp:180 >> + return result; > > This is weird. Can't we just do: > > return PlatformTimeRanges::create(intersectionRanges->ranges()) ? Good idea, I will try that. >> 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. OK >> Source/WebCore/html/HTMLMediaElement.cpp:3082 >> + double end = timeRanges->end(i, ignored); > > Why are we removing IGNORE_EXCEPTION? Exception is an HTML concept, so PlatformTimeRanges returns a bool to indicate success/failure.
Committed r164498: https://trac.webkit.org/r164498