Bug 210815

Summary:

Fix MACCATALYST build failures

Product:

WebKit

Reporter:

Peng Liu <peng.liu6>

Component:

New Bugs

Assignee:

Peng Liu <peng.liu6>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

benjamin, cdumez, cmarcelo, eric.carlson, ews-watchlist, glenn, jer.noble, keith_miller, mark.lam, msaboff, philipj, saam, sergio, thorton, tzagallo, webkit-bot-watchers-bugzilla, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
WIP patch	none
Patch	thorton: review+
Patch for landing	none

Description Peng Liu 2020-04-21 13:51:01 PDT

Fix MACCATALYST build failures

Comment 1 Peng Liu 2020-04-21 13:52:34 PDT

<rdar://problem/62111278>

Comment 2 Peng Liu 2020-04-21 13:56:11 PDT

Created attachment 397117 [details]
Patch

Comment 3 Peng Liu 2020-04-21 14:47:06 PDT

Created attachment 397124 [details]
WIP patch

Comment 4 Peng Liu 2020-04-21 16:02:20 PDT

Created attachment 397135 [details]
Patch

Comment 5 Tim Horton 2020-04-21 17:18:28 PDT

Comment on attachment 397135 [details]
Patch

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

In the future feel free to land build fixes unreviewed

> Tools/TestWebKitAPI/Tests/WebKitCocoa/RequestTextInputContext.mm:550
> -    IPhoneUserInterfaceSwizzler userInterfaceSwizzler;
> +    TestWebKitAPI::IPhoneUserInterfaceSwizzler userInterfaceSwizzler;

I think someone else already landed this

Comment 6 Jer Noble 2020-04-21 17:24:25 PDT

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

r=me with just a couple nits.

> Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm:185
> -#if !HAVE(AVKIT)
> -    return false;
> -#else
> +#if ENABLE(VIDEO_PRESENTATION_MODE) && !PLATFORM(WATCHOS)
>      if (!_page || !_page->videoFullscreenManager())
>          return false;
>  
>      return _page->videoFullscreenManager()->hasMode(WebCore::HTMLMediaElementEnums::VideoFullscreenModePictureInPicture);
> +#else
> +    return false;

Nit: I think it's safe to drop the "!PLATFORM(WATCHOS)" part of this if statement. The manager should already handle that watchOS doesn't support PiP.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/RequestTextInputContext.mm:550
> -    IPhoneUserInterfaceSwizzler userInterfaceSwizzler;
> +    TestWebKitAPI::IPhoneUserInterfaceSwizzler userInterfaceSwizzler;

This shouldn't be needed after r260469.

Comment 7 Peng Liu 2020-04-21 17:27:39 PDT

Comment on attachment 397135 [details]
Patch

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

>> Tools/TestWebKitAPI/Tests/WebKitCocoa/RequestTextInputContext.mm:550
>> +    TestWebKitAPI::IPhoneUserInterfaceSwizzler userInterfaceSwizzler;
> 
> I think someone else already landed this

Right. dbates landed a patch to fix it. Will revise the patch to remove my fixes.

Comment 8 Peng Liu 2020-04-21 17:36:42 PDT

Created attachment 397147 [details]
Patch for landing

Comment 9 Peng Liu 2020-04-21 17:38:50 PDT

(In reply to Jer Noble from comment #6)
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=397135&action=review
> 
> r=me with just a couple nits.
> 
> > Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm:185
> > -#if !HAVE(AVKIT)
> > -    return false;
> > -#else
> > +#if ENABLE(VIDEO_PRESENTATION_MODE) && !PLATFORM(WATCHOS)
> >      if (!_page || !_page->videoFullscreenManager())
> >          return false;
> >  
> >      return _page->videoFullscreenManager()->hasMode(WebCore::HTMLMediaElementEnums::VideoFullscreenModePictureInPicture);
> > +#else
> > +    return false;
> 
> Nit: I think it's safe to drop the "!PLATFORM(WATCHOS)" part of this if
> statement. The manager should already handle that watchOS doesn't support
> PiP.

Right. It is removed.

> 
> > Tools/TestWebKitAPI/Tests/WebKitCocoa/RequestTextInputContext.mm:550
> > -    IPhoneUserInterfaceSwizzler userInterfaceSwizzler;
> > +    TestWebKitAPI::IPhoneUserInterfaceSwizzler userInterfaceSwizzler;
> 
> This shouldn't be needed after r260469.

Correct.

Comment 10 EWS 2020-04-21 18:54:18 PDT

Committed r260486: <https://trac.webkit.org/changeset/260486>

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