Bug 210815

Summary: Fix MACCATALYST build failures
Product: WebKit Reporter: Peng Liu <peng.liu6>
Component: New BugsAssignee: 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, sbarati, sergio, thorton, tzagallo, webkit-bot-watchers-bugzilla, webkit-bug-importer
Priority: P2 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].