Bug 161469 - [MediaStream] applyConstraints pt. 1 - mandatory constraints
Summary: [MediaStream] applyConstraints pt. 1 - mandatory constraints
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords: InRadar
Depends on:
Blocks: 160579
  Show dependency treegraph
 
Reported: 2016-08-31 19:16 PDT by Eric Carlson
Modified: 2016-10-30 13:19 PDT (History)
5 users (show)

See Also:


Attachments
Proposed patch. (116.62 KB, patch)
2016-09-01 09:12 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Rebased patch. (114.64 KB, patch)
2016-09-01 10:05 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Rebased patch. (116.02 KB, patch)
2016-09-01 10:24 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Updated patch. (114.65 KB, patch)
2016-09-01 10:34 PDT, Eric Carlson
jer.noble: review+
Details | Formatted Diff | Diff
Patch for landing. (114.52 KB, patch)
2016-09-01 12:22 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Patch for landing. (112.35 KB, patch)
2016-09-01 12:39 PDT, Eric Carlson
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Updated patch for landing. (112.98 KB, patch)
2016-09-02 10:07 PDT, Eric Carlson
buildbot: commit-queue-
Details | Formatted Diff | Diff
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 Details

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2016-08-31 19:16:53 PDT
Add MediaStreamTrack.applyConstraints for mandatory constraints only.
Comment 1 Radar WebKit Bug Importer 2016-08-31 19:17:52 PDT
<rdar://problem/28109325>
Comment 2 Eric Carlson 2016-09-01 09:12:04 PDT
Created attachment 287618 [details]
Proposed patch.
Comment 3 Eric Carlson 2016-09-01 10:05:21 PDT
Created attachment 287626 [details]
Rebased patch.
Comment 4 Eric Carlson 2016-09-01 10:24:53 PDT
Created attachment 287628 [details]
Rebased patch.
Comment 5 WebKit Commit Bot 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.
Comment 6 Eric Carlson 2016-09-01 10:34:02 PDT
Created attachment 287630 [details]
Updated patch.
Comment 7 Jer Noble 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?
Comment 8 Eric Carlson 2016-09-01 12:22:15 PDT
Created attachment 287653 [details]
Patch for landing.
Comment 9 Eric Carlson 2016-09-01 12:39:58 PDT
Created attachment 287656 [details]
Patch for landing.
Comment 10 WebKit Commit Bot 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
Comment 11 Eric Carlson 2016-09-02 10:07:04 PDT
Created attachment 287769 [details]
Updated patch for landing.
Comment 12 WebKit Commit Bot 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>
Comment 13 Build Bot 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
Comment 14 Build Bot 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