Bug 236100

Summary: Add PageClient plumbing for video frame extraction
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: PlatformAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: akeerthi, bdakin, eric.carlson, ews-watchlist, glenn, jer.noble, megan_gardner, philipj, sergio, thorton
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 236083, 236094    
Bug Blocks:    
Attachments:
Description Flags
WIP
none
Rebase on trunk
eric.carlson: review+, ews-feeder: commit-queue-
For landing none

Description Wenson Hsieh 2022-02-03 13:11:37 PST Comment hidden (obsolete)
Comment 1 Wenson Hsieh 2022-02-03 13:12:33 PST 
rdar://86237608
Comment 2 Wenson Hsieh 2022-02-03 13:53:32 PST Comment hidden (obsolete)
Comment 3 Wenson Hsieh 2022-02-07 09:27:54 PST 
Created attachment 451116 [details]
Rebase on trunk
Comment 4 Eric Carlson 2022-02-07 09:43:20 PST 
Comment on attachment 451116 [details]
Rebase on trunk

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

> Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm:511
> +        if (RetainPtr controller = protectedThis->m_videoFullscreenManager->playerViewController(identifier))

You null-checked m_videoFullscreenManager before the completion handler, but is it guaranteed to still be valid here?
Comment 5 Wenson Hsieh 2022-02-07 10:03:37 PST 
Comment on attachment 451116 [details]
Rebase on trunk

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

Thanks for the review!

>> Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm:511
>> +        if (RetainPtr controller = protectedThis->m_videoFullscreenManager->playerViewController(identifier))
> 
> You null-checked m_videoFullscreenManager before the completion handler, but is it guaranteed to still be valid here?

Great catch! I don't see any (obvious) reason why it's still guaranteed to be valid here, so I'll add a null check for `protectedThis->m_videoFullscreenManager` as well.
Comment 6 Wenson Hsieh 2022-02-07 10:13:03 PST 
Created attachment 451122 [details]
For landing
Comment 7 EWS 2022-02-07 12:20:00 PST 
Committed r289237 (246921@main): <https://commits.webkit.org/246921@main>

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