Bug 225797 – Add ScriptDisallowedScope to MediaPlayerPrivateAVFoundation

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Ryosuke Niwa]

As a follow up of https://commits.webkit.org/r277379, add ScriptDisallowedScope
to a bunch of member functions of MediaPlayerPrivateAVFoundation.

[Ryosuke Niwa]

Created attachment 428594 [details]
Adds ScriptDisallowedScope

[Eric Carlson]

Comment on attachment 428594 [details]
Adds ScriptDisallowedScope

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

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:3623
> +            ScriptDisallowedScope scriptDisallowedScope;

Can't this use the less expensive `ScriptDisallowedScope::InMainThread`?

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:3722
> +            ScriptDisallowedScope scriptDisallowedScope;

Ditto.

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:3755
> +        ScriptDisallowedScope scriptDisallowedScope;

Ditto

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:3827
> +            ScriptDisallowedScope scriptDisallowedScope;

Ditto

[Ryosuke Niwa]

(In reply to Eric Carlson from comment #2)
> Comment on attachment 428594 [details]
> Adds ScriptDisallowedScope
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=428594&action=review
> 
> > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:3623
> > +            ScriptDisallowedScope scriptDisallowedScope;
> 
> Can't this use the less expensive `ScriptDisallowedScope::InMainThread`?

Is it guaranteed that all these code will always run in the main thread?

> > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:3722
> > +            ScriptDisallowedScope scriptDisallowedScope;
> 
> Ditto.
> 
> > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:3755
> > +        ScriptDisallowedScope scriptDisallowedScope;
> 
> Ditto
> 
> > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:3827
> > +            ScriptDisallowedScope scriptDisallowedScope;
> 
> Ditto

[Eric Carlson]

Comment on attachment 428594 [details]
Adds ScriptDisallowedScope

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

>>> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:3623
>>> +            ScriptDisallowedScope scriptDisallowedScope;
>> 
>> Can't this use the less expensive `ScriptDisallowedScope::InMainThread`?
> 
> Is it guaranteed that all these code will always run in the main thread?

Each of these tasks runs inside of an `ensureOnMainThread` block for the same reason you want to disallow script - because they may call up into HTMLMediaElement.

[Ryosuke Niwa]

(In reply to Eric Carlson from comment #4)
> Comment on attachment 428594 [details]
> Adds ScriptDisallowedScope
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=428594&action=review
> 
> >>> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:3623
> >>> +            ScriptDisallowedScope scriptDisallowedScope;
> >> 
> >> Can't this use the less expensive `ScriptDisallowedScope::InMainThread`?
> > 
> > Is it guaranteed that all these code will always run in the main thread?
> 
> Each of these tasks runs inside of an `ensureOnMainThread` block for the
> same reason you want to disallow script - because they may call up into
> HTMLMediaElement.

Oh, right. Will change to use ScriptDisallowedScope::InMainThread.

[Ryosuke Niwa]

Created attachment 428663 [details]
Patch for landing

[EWS]

Committed r277512 (237742@main): <https://commits.webkit.org/237742@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 428663 [details].

[Radar WebKit Bug Importer]

<rdar://problem/78035454>