Bug 161469

Summary: [MediaStream] applyConstraints pt. 1 - mandatory constraints
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: MediaAssignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, jer.noble, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 160579    
Attachments:
Description Flags
Proposed patch.
none
Rebased patch.
none
Rebased patch.
none
Updated patch.
jer.noble: review+
Patch for landing.
none
Patch for landing.
commit-queue: commit-queue-
Updated patch for landing.
buildbot: commit-queue-
Archive of layout-test-results from ews106 for mac-yosemite-wk2 none

Eric Carlson
Reported 2016-08-31 19:16:53 PDT
Add MediaStreamTrack.applyConstraints for mandatory constraints only.
Attachments
Proposed patch. (116.62 KB, patch)
2016-09-01 09:12 PDT, Eric Carlson
no flags
Rebased patch. (114.64 KB, patch)
2016-09-01 10:05 PDT, Eric Carlson
no flags
Rebased patch. (116.02 KB, patch)
2016-09-01 10:24 PDT, Eric Carlson
no flags
Updated patch. (114.65 KB, patch)
2016-09-01 10:34 PDT, Eric Carlson
jer.noble: review+
Patch for landing. (114.52 KB, patch)
2016-09-01 12:22 PDT, Eric Carlson
no flags
Patch for landing. (112.35 KB, patch)
2016-09-01 12:39 PDT, Eric Carlson
commit-queue: commit-queue-
Updated patch for landing. (112.98 KB, patch)
2016-09-02 10:07 PDT, Eric Carlson
buildbot: commit-queue-
Archive of layout-test-results from ews106 for mac-yosemite-wk2 (1.01 MB, application/zip)
2016-09-02 11:02 PDT, Build Bot
no flags
Radar WebKit Bug Importer
Comment 1 2016-08-31 19:17:52 PDT
Eric Carlson
Comment 2 2016-09-01 09:12:04 PDT
Created attachment 287618 [details] Proposed patch.
Eric Carlson
Comment 3 2016-09-01 10:05:21 PDT
Created attachment 287626 [details] Rebased patch.
Eric Carlson
Comment 4 2016-09-01 10:24:53 PDT
Created attachment 287628 [details] Rebased patch.
WebKit Commit Bot
Comment 5 2016-09-01 10:26:55 PDT
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
Comment 6 2016-09-01 10:34:02 PDT
Created attachment 287630 [details] Updated patch.
Jer Noble
Comment 7 2016-09-01 10:45:15 PDT
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
Comment 8 2016-09-01 12:22:15 PDT
Created attachment 287653 [details] Patch for landing.
Eric Carlson
Comment 9 2016-09-01 12:39:58 PDT
Created attachment 287656 [details] Patch for landing.
WebKit Commit Bot
Comment 10 2016-09-01 13:01:21 PDT
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
Comment 11 2016-09-02 10:07:04 PDT
Created attachment 287769 [details] Updated patch for landing.
WebKit Commit Bot
Comment 12 2016-09-02 10:38:24 PDT
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 13 2016-09-02 11:02:02 PDT
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
Comment 14 2016-09-02 11:02:06 PDT
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
Note You need to log in before you can comment on or make changes to this bug.