Bug 203728

Created attachment 382564 [details] Patch

Created attachment 382566 [details] Patch

Created attachment 382567 [details] Patch

Comment on attachment 382567 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=382567&action=review > Tools/Scripts/webkitpy/port/mac.py:294 > + self._config = Config(self._executive, self._filesystem, MacCatalystPort.port_name) What are we adding this for? Wouldn’t expect we needed it to get things building (we will need it to run them, but that’s a different change)

Comment on attachment 382567 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=382567&action=review > Tools/TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm:148 > +#if HAVE(SEC_KEY_PROXY) && HAVE(SSL) Nit - let’s update the #endif comment to match the new #if. > Tools/TestWebKitAPI/Tests/WebKitCocoa/DragAndDropTests.mm:28 > +#if ENABLE(DRAG_SUPPORT) && !PLATFORM(MACCATALYST) Ditto. > Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm:28 > +#if ENABLE(DRAG_SUPPORT) && PLATFORM(IOS_FAMILY) && !PLATFORM(MACCATALYST) Ditto. > Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm:29 > +#if ENABLE(DRAG_SUPPORT) && PLATFORM(IOS_FAMILY) && !PLATFORM(MACCATALYST) Ditto.

(In reply to Jonathan Bedard from comment #4) > Comment on attachment 382567 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=382567&action=review > > > Tools/Scripts/webkitpy/port/mac.py:294 > > + self._config = Config(self._executive, self._filesystem, MacCatalystPort.port_name) > > What are we adding this for? Wouldn’t expect we needed it to get things > building (we will need it to run them, but that’s a different change) This gets the tests building AND running. I should update the title.

Comment on attachment 382567 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=382567&action=review We don't have any support for macCatalyst in build-webkit, do we? > Tools/TestWebKitAPI/config.h:75 > +#if PLATFORM(MACCATALYST) > +#import <WebKit/WebKitLegacy.h> > +#endif I see the explanation in ChangeLog, but it doesn't tell me why it's needed at all - and for something as unexpected as this, a code comment would be appropriate.

(In reply to Alexey Proskuryakov from comment #7) > Comment on attachment 382567 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=382567&action=review > > We don't have any support for macCatalyst in build-webkit, do we? 🤷‍♂️ > > Tools/TestWebKitAPI/config.h:75 > > +#if PLATFORM(MACCATALYST) > > +#import <WebKit/WebKitLegacy.h> > > +#endif > > I see the explanation in ChangeLog, but it doesn't tell me why it's needed > at all - and for something as unexpected as this, a code comment would be > appropriate. Because many of the WebKitLegacy tests expect to get WebKitLegacy.h through WebKit.h? I thought the changelog comment was sufficient but I can beef it up and leave a comment in the code too.

(In reply to Tim Horton from comment #8) > (In reply to Alexey Proskuryakov from comment #7) > > Comment on attachment 382567 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=382567&action=review > > > > We don't have any support for macCatalyst in build-webkit, do we? > > 🤷‍♂️ > We do not. I looked into this a few weeks ago, it's not a small task, and probably closely related to some work Keith Rollin has been doing with tvOS and watchOS (to be clear, not volunteering him, just providing some context)

Created attachment 382611 [details] Patch

The commit-queue encountered the following flaky tests while processing attachment 382611 [details]: requestidlecallback/requestidlecallback-document-gc.html bug 203758 (author: rniwa@webkit.org) The commit-queue is continuing to process your patch.

Comment on attachment 382611 [details] Patch Clearing flags on attachment: 382611 Committed r251941: <https://trac.webkit.org/changeset/251941>

All reviewed patches have been landed. Closing bug.

<rdar://problem/56828694>