Bug 189998 – [MediaStream] Clean up RealtimeMediaSource settings change handling

• Status: RESOLVED
• Resolution: FIXED
• Component: WebRTC

[Eric Carlson]

Clean up RealtimeMediaSource settings change handling

[Radar WebKit Bug Importer]

<rdar://problem/44797884>

[Eric Carlson]

Created attachment 350869 [details]
Patch

[youenn fablet]

Comment on attachment 350869 [details]
Patch

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

> Source/WebCore/platform/mediastream/RealtimeMediaSource.h:250
> +    void notifySettingsDidChangeObservers(OptionSet<RealtimeMediaSourceSettings::Flag>);

s/notifySettingsDidChangeObservers/notifySettingsDidChange/ maybe?

[WebKit Commit Bot]

Comment on attachment 350869 [details]
Patch

Clearing flags on attachment: 350869

Committed r236511: <https://trac.webkit.org/changeset/236511>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

Seems to have caused quite a few failures?
https://build.webkit.org/results/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/r236511%20(6956)/results.html

[youenn fablet]

(In reply to Chris Dumez from comment #6)
> Seems to have caused quite a few failures?
> https://build.webkit.org/results/
> Apple%20High%20Sierra%20Release%20WK2%20(Tests)/r236511%20(6956)/results.html

I doubt this change would cause issues in SVG, animations, compositing, CSS...

[Chris Dumez]

(In reply to Chris Dumez from comment #6)
> Seems to have caused quite a few failures?
> https://build.webkit.org/results/
> Apple%20High%20Sierra%20Release%20WK2%20(Tests)/r236511%20(6956)/results.html

Not sure what you're talking about. I am talking about these:
Tests that failed text/pixel/audio diff (7): flag all

 test 	results	image results	actual failure	expected failure	history
+fast/mediastream/MediaStream-video-element-displays-buffer.html	expected actual diff pretty diff		text	pass	history
+fast/mediastream/apply-constraints-advanced.html	expected actual diff pretty diff		text	pass	history
+fast/mediastream/apply-constraints-audio.html	expected actual diff pretty diff		text	pass	history
+fast/mediastream/apply-constraints-video.html	expected actual diff pretty diff		text	pass	history
+webrtc/captureCanvas-webrtc.html	expected actual diff pretty diff		text	pass timeout	history
+webrtc/video-replace-track.html	expected actual diff pretty diff		text	pass	history
+webrtc/video-rotation.html	expected actual diff pretty diff		text	pass	history
Tests that timed out (1): flag all

+webrtc/video-mute.html	expected actual diff pretty diff	history

[Chris Dumez]

(In reply to Chris Dumez from comment #8)
> (In reply to Chris Dumez from comment #6)
> > Seems to have caused quite a few failures?
> > https://build.webkit.org/results/
> > Apple%20High%20Sierra%20Release%20WK2%20(Tests)/r236511%20(6956)/results.html
> 
> Not sure what you're talking about. I am talking about these:
> Tests that failed text/pixel/audio diff (7): flag all
> 
>  test 	results	image results	actual failure	expected failure	history
> +fast/mediastream/MediaStream-video-element-displays-buffer.html	expected
> actual diff pretty diff		text	pass	history
> +fast/mediastream/apply-constraints-advanced.html	expected actual diff
> pretty diff		text	pass	history
> +fast/mediastream/apply-constraints-audio.html	expected actual diff pretty
> diff		text	pass	history
> +fast/mediastream/apply-constraints-video.html	expected actual diff pretty
> diff		text	pass	history
> +webrtc/captureCanvas-webrtc.html	expected actual diff pretty diff		text
> pass timeout	history
> +webrtc/video-replace-track.html	expected actual diff pretty diff		text	pass
> history
> +webrtc/video-rotation.html	expected actual diff pretty diff		text	pass
> history
> Tests that timed out (1): flag all
> 
> +webrtc/video-mute.html	expected actual diff pretty diff	history

Do you have the "Only unexpected results" checkbox unchecked by any chance? :)

[youenn fablet]

(In reply to Chris Dumez from comment #9)
> (In reply to Chris Dumez from comment #8)
> > (In reply to Chris Dumez from comment #6)
> > > Seems to have caused quite a few failures?
> > > https://build.webkit.org/results/
> > > Apple%20High%20Sierra%20Release%20WK2%20(Tests)/r236511%20(6956)/results.html
> > 
> > Not sure what you're talking about. I am talking about these:
> > Tests that failed text/pixel/audio diff (7): flag all
> > 
> >  test 	results	image results	actual failure	expected failure	history
> > +fast/mediastream/MediaStream-video-element-displays-buffer.html	expected
> > actual diff pretty diff		text	pass	history
> > +fast/mediastream/apply-constraints-advanced.html	expected actual diff
> > pretty diff		text	pass	history
> > +fast/mediastream/apply-constraints-audio.html	expected actual diff pretty
> > diff		text	pass	history
> > +fast/mediastream/apply-constraints-video.html	expected actual diff pretty
> > diff		text	pass	history
> > +webrtc/captureCanvas-webrtc.html	expected actual diff pretty diff		text
> > pass timeout	history
> > +webrtc/video-replace-track.html	expected actual diff pretty diff		text	pass
> > history
> > +webrtc/video-rotation.html	expected actual diff pretty diff		text	pass
> > history
> > Tests that timed out (1): flag all
> > 
> > +webrtc/video-mute.html	expected actual diff pretty diff	history
> 
> Do you have the "Only unexpected results" checkbox unchecked by any chance?
> :)

Oh, this could definitely create issues for these tests!

[WebKit Commit Bot]

Re-opened since this is blocked by bug 190008

[youenn fablet]

> Do you have the "Only unexpected results" checkbox unchecked by any chance?
> :)

Indeed!

[youenn fablet]

(In reply to youenn fablet from comment #12)
> > Do you have the "Only unexpected results" checkbox unchecked by any chance?
> > :)
> 
> Indeed!

Well, I had, but no longer have...

[Eric Carlson]

Created attachment 350967 [details]
Updated patch

[EWS Watchlist]

Comment on attachment 350967 [details]
Updated patch

Attachment 350967 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9369604

New failing tests:
webrtc/video-rotation.html
webrtc/video-replace-track.html

[EWS Watchlist]

Created attachment 350973 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

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

[EWS Watchlist]

Comment on attachment 350967 [details]
Updated patch

Attachment 350967 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9374814

New failing tests:
webrtc/video-rotation.html
webrtc/video-replace-track.html

[EWS Watchlist]

Created attachment 351038 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Eric Carlson]

Created attachment 351043 [details]
Updated patch

[Eric Carlson]

Created attachment 351044 [details]
Updated patch

[Eric Carlson]

Created attachment 351046 [details]
Updated patch

[EWS Watchlist]

Comment on attachment 351046 [details]
Updated patch

Attachment 351046 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9380157

New failing tests:
webrtc/video-disabled-black.html

[EWS Watchlist]

Created attachment 351071 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Eric Carlson]

Created attachment 351213 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 351213 [details]
Patch for landing

Clearing flags on attachment: 351213

Committed r236646: <https://trac.webkit.org/changeset/236646>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.