Bug 125245 – [MSE] Refactor MediaSourceBase back into MediaSource

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

[MSE] Refactor MediaSourceBase back into MediaSource

[Jer Noble]

Created attachment 218436 [details]
Patch

[Jer Noble]

The style-bot is going to choke on this patch, but it's literally just moving functions to different files.

[WebKit Commit Bot]

Attachment 218436 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/Modules/mediasource/DOMURLMediaSource.cpp', u'Source/WebCore/Modules/mediasource/DOMURLMediaSource.h', u'Source/WebCore/Modules/mediasource/MediaSource.cpp', u'Source/WebCore/Modules/mediasource/MediaSource.h', u'Source/WebCore/Modules/mediasource/MediaSourceBase.cpp', u'Source/WebCore/Modules/mediasource/MediaSourceBase.h', u'Source/WebCore/Modules/mediasource/MediaSourceRegistry.cpp', u'Source/WebCore/Modules/mediasource/MediaSourceRegistry.h', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', '--commit-queue']" exit_code: 1
ERROR: Source/WebCore/Modules/mediasource/MediaSource.cpp:122:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebCore/Modules/mediasource/MediaSource.cpp:125:  Wrong number of spaces before statement. (expected: 8)  [whitespace/indent] [4]
ERROR: Source/WebCore/Modules/mediasource/MediaSource.cpp:171:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebCore/Modules/mediasource/MediaSource.cpp:182:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebCore/Modules/mediasource/MediaSource.cpp:199:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 5 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eric Carlson]

Comment on attachment 218436 [details]
Patch

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

> Source/WebCore/Modules/mediasource/MediaSource.cpp:86
> +    DEFINE_STATIC_LOCAL(const AtomicString, open, ("open", AtomicString::ConstructFromLiteral));

I have seen more NeverDestroyed<> used more often than DEFINE_STATIC_LOCAL() recently. Do you know if there is a reason for using one or the other?

> Source/WebCore/Modules/mediasource/MediaSource.cpp:639
> +    for (unsigned long i = 0; i < m_sourceBuffers->length(); ++i)

Nit: m_sourceBuffers won't change length during the loop so you can cache length() in a variable during initialization.

> Source/WebCore/Modules/mediasource/MediaSource.cpp:649
> +    for (size_t i = 0; i < m_activeSourceBuffers->length(); ++i)

Ditto.

[kov's GTK+ EWS bot]

Comment on attachment 218436 [details]
Patch

Attachment 218436 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/43348042

[Jer Noble]

Created attachment 218455 [details]
Patch for landing.

[Jer Noble]

(In reply to comment #4)
> (From update of attachment 218436 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=218436&action=review
> 
> > Source/WebCore/Modules/mediasource/MediaSource.cpp:86
> > +    DEFINE_STATIC_LOCAL(const AtomicString, open, ("open", AtomicString::ConstructFromLiteral));
> 
> I have seen more NeverDestroyed<> used more often than DEFINE_STATIC_LOCAL() recently. Do you know if there is a reason for using one or the other?

I don't.  But we can change these in a future patch.

> > Source/WebCore/Modules/mediasource/MediaSource.cpp:639
> > +    for (unsigned long i = 0; i < m_sourceBuffers->length(); ++i)
> 
> Nit: m_sourceBuffers won't change length during the loop so you can cache length() in a variable during initialization.

Sure.

> > Source/WebCore/Modules/mediasource/MediaSource.cpp:649
> > +    for (size_t i = 0; i < m_activeSourceBuffers->length(); ++i)
> 
> Ditto.

Sure.

[WebKit Commit Bot]

Attachment 218455 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/Modules/mediasource/DOMURLMediaSource.cpp', u'Source/WebCore/Modules/mediasource/DOMURLMediaSource.h', u'Source/WebCore/Modules/mediasource/MediaSource.cpp', u'Source/WebCore/Modules/mediasource/MediaSource.h', u'Source/WebCore/Modules/mediasource/MediaSourceBase.cpp', u'Source/WebCore/Modules/mediasource/MediaSourceBase.h', u'Source/WebCore/Modules/mediasource/MediaSourceRegistry.cpp', u'Source/WebCore/Modules/mediasource/MediaSourceRegistry.h', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', '--commit-queue']" exit_code: 1
ERROR: Source/WebCore/Modules/mediasource/MediaSource.cpp:171:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebCore/Modules/mediasource/MediaSource.cpp:182:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebCore/Modules/mediasource/MediaSource.cpp:199:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 3 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jer Noble]

Committed r160258: <http://trac.webkit.org/changeset/160258>