Bug 161469 – [MediaStream] applyConstraints pt. 1 - mandatory constraints

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Eric Carlson]

Add MediaStreamTrack.applyConstraints for mandatory constraints only.

[Radar WebKit Bug Importer]

<rdar://problem/28109325>

[Eric Carlson]

Created attachment 287618 [details]
Proposed patch.

[Eric Carlson]

Created attachment 287626 [details]
Rebased patch.

[Eric Carlson]

Created attachment 287628 [details]
Rebased patch.

[WebKit Commit Bot]

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


ERROR: Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:55:  Should have a space between // and comment  [whitespace/comments] [4]
ERROR: Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:55:  Bad include order. Mixing system and custom headers.  [build/include_order] [4]
ERROR: Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:72:  Should have a space between // and comment  [whitespace/comments] [4]
ERROR: Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:74:  Should have a space between // and comment  [whitespace/comments] [4]
ERROR: Source/WTF/wtf/FeatureDefines.h:582:  Should have a space between // and comment  [whitespace/comments] [4]
Total errors found: 5 in 34 files


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

[Eric Carlson]

Created attachment 287630 [details]
Updated patch.

[Jer Noble]

Comment on attachment 287630 [details]
Updated patch.

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

r=me with a lonely nit:

> Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp:116
> +    {
> +        LockHolder lock(m_lock);
> +        if (m_pendingSettingsDidChangeNotification || m_suppressNotifications)
> +            return;
> +
> +        m_pendingSettingsDidChangeNotification = true;
> +    }
> +
> +    scheduleDeferredTask([this] {
> +        {
> +            LockHolder lock(m_lock);
> +            m_pendingSettingsDidChangeNotification = false;
> +        }
> +
> +        for (auto& observer : m_observers)
> +            observer->sourceSettingsChanged();
> +    });

Nit: are there expectations about what thread settingsDidChange() is called on?  Should there be an ASSERT(isMainThread()) or ASSERT(isWebThread()) in here somewhere?

[Eric Carlson]

Created attachment 287653 [details]
Patch for landing.

[Eric Carlson]

Created attachment 287656 [details]
Patch for landing.

[WebKit Commit Bot]

Comment on attachment 287656 [details]
Patch for landing.

Rejecting attachment 287656 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
ebCore::RealtimeMediaSource::applyConstraints(WebCore::MediaConstraints const&, std::__1::function<void ()>, std::__1::function<void (WTF::String const&, WTF::String const&)>) in RealtimeMediaSource.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

** BUILD FAILED **


The following build commands failed:
	Ld /Volumes/Data/EWS/WebKit/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore normal x86_64
(1 failure)

Full output: http://webkit-queues.webkit.org/results/1988823

[Eric Carlson]

Created attachment 287769 [details]
Updated patch for landing.

[WebKit Commit Bot]

Comment on attachment 287769 [details]
Updated patch for landing.

Clearing flags on attachment: 287769

Committed r205348: <http://trac.webkit.org/changeset/205348>

[Build Bot]

Comment on attachment 287769 [details]
Updated patch for landing.

Attachment 287769 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1994981

New failing tests:
fast/mediastream/MediaStreamTrack-getSettings.html

[Build Bot]

Created attachment 287779 [details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5