Bug 236100 - Add PageClient plumbing for video frame extraction
Summary: Add PageClient plumbing for video frame extraction
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on: 236083 236094
Blocks:
  Show dependency tree / graph
 
Reported: 2022-02-03 13:11 PST by Wenson Hsieh
Modified: 2022-02-07 12:37 PST (History)
10 users (show)

See Also:

Attachments
WIP (15.69 KB, patch)
2022-02-03 13:53 PST, Wenson Hsieh 		no flags Details | Formatted Diff | Diff
Rebase on trunk (15.63 KB, patch)
2022-02-07 09:27 PST, Wenson Hsieh 		eric.carlson: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
For landing (15.73 KB, patch)
2022-02-07 10:13 PST, Wenson Hsieh 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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].