Bug 189998 - [MediaStream] Clean up RealtimeMediaSource settings change handling
Summary: [MediaStream] Clean up RealtimeMediaSource settings change handling
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords: InRadar
Depends on: 190008
Blocks:
  Show dependency treegraph
 
Reported: 2018-09-26 09:12 PDT by Eric Carlson
Modified: 2018-09-30 07:32 PDT (History)
6 users (show)

See Also:


Attachments
Patch (10.10 KB, patch)
2018-09-26 10:31 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Updated patch (9.90 KB, patch)
2018-09-27 09:18 PDT, Eric Carlson
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-sierra-wk2 (3.10 MB, application/zip)
2018-09-27 10:24 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews121 for ios-simulator-wk2 (2.44 MB, application/zip)
2018-09-27 19:05 PDT, EWS Watchlist
no flags Details
Updated patch (20.91 KB, patch)
2018-09-27 20:26 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Updated patch (20.90 KB, patch)
2018-09-27 20:28 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Updated patch (22.51 KB, patch)
2018-09-27 20:49 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews125 for ios-simulator-wk2 (2.31 MB, application/zip)
2018-09-28 06:51 PDT, EWS Watchlist
no flags Details
Patch for landing (24.62 KB, patch)
2018-09-30 02:37 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2018-09-26 09:12:03 PDT
Clean up RealtimeMediaSource settings change handling
Comment 1 Radar WebKit Bug Importer 2018-09-26 09:12:33 PDT
<rdar://problem/44797884>
Comment 2 Eric Carlson 2018-09-26 10:31:27 PDT
Created attachment 350869 [details]
Patch
Comment 3 youenn fablet 2018-09-26 10:43:26 PDT
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?
Comment 4 WebKit Commit Bot 2018-09-26 11:22:01 PDT
Comment on attachment 350869 [details]
Patch

Clearing flags on attachment: 350869

Committed r236511: <https://trac.webkit.org/changeset/236511>
Comment 5 WebKit Commit Bot 2018-09-26 11:22:03 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Chris Dumez 2018-09-26 12:56:26 PDT
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
Comment 7 youenn fablet 2018-09-26 13:23:27 PDT
(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...
Comment 8 Chris Dumez 2018-09-26 13:24:17 PDT
(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
Comment 9 Chris Dumez 2018-09-26 13:25:03 PDT
(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? :)
Comment 10 youenn fablet 2018-09-26 13:26:03 PDT
(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!
Comment 11 WebKit Commit Bot 2018-09-26 13:28:56 PDT
Re-opened since this is blocked by bug 190008
Comment 12 youenn fablet 2018-09-26 13:29:48 PDT
> Do you have the "Only unexpected results" checkbox unchecked by any chance?
> :)

Indeed!
Comment 13 youenn fablet 2018-09-26 13:30:05 PDT
(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...
Comment 14 Eric Carlson 2018-09-27 09:18:48 PDT
Created attachment 350967 [details]
Updated patch
Comment 15 EWS Watchlist 2018-09-27 10:24:49 PDT
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
Comment 16 EWS Watchlist 2018-09-27 10:24:51 PDT
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
Comment 17 EWS Watchlist 2018-09-27 19:05:53 PDT
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
Comment 18 EWS Watchlist 2018-09-27 19:05:55 PDT
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
Comment 19 Eric Carlson 2018-09-27 20:26:08 PDT
Created attachment 351043 [details]
Updated patch
Comment 20 Eric Carlson 2018-09-27 20:28:55 PDT
Created attachment 351044 [details]
Updated patch
Comment 21 Eric Carlson 2018-09-27 20:49:46 PDT
Created attachment 351046 [details]
Updated patch
Comment 22 EWS Watchlist 2018-09-28 06:51:40 PDT
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
Comment 23 EWS Watchlist 2018-09-28 06:51:42 PDT
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
Comment 24 Eric Carlson 2018-09-30 02:37:57 PDT
Created attachment 351213 [details]
Patch for landing
Comment 25 WebKit Commit Bot 2018-09-30 07:32:23 PDT
Comment on attachment 351213 [details]
Patch for landing

Clearing flags on attachment: 351213

Committed r236646: <https://trac.webkit.org/changeset/236646>
Comment 26 WebKit Commit Bot 2018-09-30 07:32:25 PDT
All reviewed patches have been landed.  Closing bug.