Make worker RTC insertable streams constructs controlled by webrtc insertable streams feature setting
Created attachment 414796 [details] Patch
<rdar://problem/71798249>
Ping review.
Thank you so much for working on making this part of Settings better! Would you mind breaking out the change to Settings into its own patch? I think it would be good to look at it separately.
(In reply to Sam Weinig from comment #4) > Thank you so much for working on making this part of Settings better! Would > you mind breaking out the change to Settings into its own patch? I think it > would be good to look at it separately. Sure. Do you have any preliminary comment on the Settings part of the patch? I for instance kept it as a subclass of Settings but contemplated to move it to its own class. Or maybe you have some other idea?
(In reply to youenn fablet from comment #5) > (In reply to Sam Weinig from comment #4) > > Thank you so much for working on making this part of Settings better! Would > > you mind breaking out the change to Settings into its own patch? I think it > > would be good to look at it separately. > > Sure. > Do you have any preliminary comment on the Settings part of the patch? > I for instance kept it as a subclass of Settings but contemplated to move it > to its own class. > Or maybe you have some other idea? Subobject seems fine, though I am not sure if it makes sense to constrain it to just the boolean values. I think I would prefer doing a deep (thread safe) copy of all the Settings if possible, just to keep things relatively normalized.
(In reply to Sam Weinig from comment #6) > (In reply to youenn fablet from comment #5) > > (In reply to Sam Weinig from comment #4) > > > Thank you so much for working on making this part of Settings better! Would > > > you mind breaking out the change to Settings into its own patch? I think it > > > would be good to look at it separately. > > > > Sure. > > Do you have any preliminary comment on the Settings part of the patch? > > I for instance kept it as a subclass of Settings but contemplated to move it > > to its own class. > > Or maybe you have some other idea? > > Subobject seems fine, though I am not sure if it makes sense to constrain it > to just the boolean values. I think I would prefer doing a deep (thread > safe) copy of all the Settings if possible, just to keep things relatively > normalized. I used the boolean heuristic as a way to restrict to the feature flags that are necessary when creating the global scope. I thought a bit about extending that but it did not seem useful right now so ended up not doing it.
(In reply to youenn fablet from comment #7) > (In reply to Sam Weinig from comment #6) > > (In reply to youenn fablet from comment #5) > > > (In reply to Sam Weinig from comment #4) > > > > Thank you so much for working on making this part of Settings better! Would > > > > you mind breaking out the change to Settings into its own patch? I think it > > > > would be good to look at it separately. > > > > > > Sure. > > > Do you have any preliminary comment on the Settings part of the patch? > > > I for instance kept it as a subclass of Settings but contemplated to move it > > > to its own class. > > > Or maybe you have some other idea? > > > > Subobject seems fine, though I am not sure if it makes sense to constrain it > > to just the boolean values. I think I would prefer doing a deep (thread > > safe) copy of all the Settings if possible, just to keep things relatively > > normalized. > > I used the boolean heuristic as a way to restrict to the feature flags that > are necessary when creating the global scope. > I thought a bit about extending that but it did not seem useful right now so > ended up not doing it. There are a number of non-bool settings that will be useful in workers eventually, usually around maximum on minimum sizes (e.g. maximumAccelerated2dCanvasSize for offscreen canvas at some point, etc).
Created attachment 416147 [details] Rebasing
Comment on attachment 416147 [details] Rebasing r=me too.
Committed r270842: <https://trac.webkit.org/changeset/270842> All reviewed patches have been landed. Closing bug and clearing flags on attachment 416147 [details].