[webkit-reviews] review canceled: [Bug 203723] Update WebKitTestRunner to support running multiple video fullscreen and Picture-in-Picture tests simultaneously : [Attachment 398273] Fix iOS build failures

[bugzilla-daemon at webkit.org]

Peng Liu <peng.liu6 at apple.com> has canceled Peng Liu <peng.liu6 at apple.com>'s
request for review:
Bug 203723: Update WebKitTestRunner to support running multiple video
fullscreen and Picture-in-Picture tests simultaneously
https://bugs.webkit.org/show_bug.cgi?id=203723

Attachment 398273: Fix iOS build failures

https://bugs.webkit.org/attachment.cgi?id=398273&action=review




--- Comment #22 from Peng Liu <peng.liu6 at apple.com> ---
Comment on attachment 398273
  --> https://bugs.webkit.org/attachment.cgi?id=398273
Fix iOS build failures

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

>> Source/WebKit/ChangeLog:10
>> +	    messages to the UI process.
> 
> Testing IPC messages sending and receiving is nice and will validate code
running in UIProcess as well.
> Can you explain why the mock is done in WebProcess instead of UIProcess?
> Is it the idea that API tests would cover UIProcess code and layout test the
WebProcess code?

Thanks for the review.

Yes, it was the plan to only test the web process code (for WK2) with the
layout tests. However, I agree with your point that it is a good idea to test
the IPC message related code as well.

Will update the patch to move the mock to the UI process.

>> Source/WebKit/UIProcess/WebPageProxy.h:2803
>> +
> 
> Unneeded change

Will remove it.

>> Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.mm:288
>> +	if (!m_mockVideoPresentationModeEnabled)
> 
> We usually return early, something like:
> 
> if (m_mockVideoPresentationModeEnabled) {
>     dispatch_async(...);
>     return;
> }

Right.
The changes in the VideoFullscreenManager will be removed in the new patch.

>> LayoutTests/media/video-presentation-mode-expected.txt:20
>> +** Entered picture-in-pictur
> 
> picture

Good catch! Will fix it.