Bug 226846 – [Monterey] Support building WebKit

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Jonathan Bedard]

We need to add some SPI declaration to build WebKit with the Monterey SDK.

[Radar WebKit Bug Importer]

<rdar://problem/79095148>

[Jonathan Bedard]

Created attachment 431005 [details]
Patch

[Tim Horton]

Comment on attachment 431005 [details]
Patch

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

> Source/WebCore/PAL/pal/spi/cocoa/CryptoKitCBridgingSPI.h:51
> +- (instancetype)initWithPublicKey:(NSData*)spkiBytes error:(NSError * __autoreleasing *)error;

It seems like in nearby code we have fixed up the star-sidedness of the copied SPI to match WebKit style. But I can also see an argument for "just paste it as-is". Not sure which is preferable.

> Source/WebCore/testing/Internals.cpp:5644
> +        , Vector<TextRecognitionDataDetector>()

I don't understand this and the changelog doesn't help

> Source/WebKit/UIProcess/API/Cocoa/WKWebViewTesting.mm:457
> -                return { WebCore::ExceptionData { WebCore::InvalidStateError } };
> +                return { WebCore::ExceptionData { WebCore::InvalidStateError, String() } };

I don't understand these, and the changelog only says what you are doing, not why.

[Wenson Hsieh]

Comment on attachment 431005 [details]
Patch

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

> Source/WebCore/PAL/pal/spi/mac/QuickLookMacSPI.h:76
> +#if ENABLE(IMAGE_ANALYSIS)

Nit - we should avoid using an ENABLE feature flag to guard this SPI header. Maybe we could add `HAVE_QUICKLOOK_PREVIEW_ACTIVITY` and use it here?

[Wenson Hsieh]

Comment on attachment 431005 [details]
Patch

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

> Source/WebKit/Platform/spi/Cocoa/VisionKitSPI.h:26
> +#if ENABLE(IMAGE_ANALYSIS)

(Nit - similar comment here — maybe `HAVE_VK_IMAGE_ANALYSIS`?)

[Jonathan Bedard]

Comment on attachment 431005 [details]
Patch

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

>> Source/WebCore/PAL/pal/spi/cocoa/CryptoKitCBridgingSPI.h:51
>> +- (instancetype)initWithPublicKey:(NSData*)spkiBytes error:(NSError * __autoreleasing *)error;
> 
> It seems like in nearby code we have fixed up the star-sidedness of the copied SPI to match WebKit style. But I can also see an argument for "just paste it as-is". Not sure which is preferable.

Agree with fixing the star-sidedness, style checker didn't complain, so I didn't notice it.

That being said, I'm unclear what the right thing to do with star-sidedness and __autoreleasing is

>> Source/WebCore/testing/Internals.cpp:5644
>> +        , Vector<TextRecognitionDataDetector>()
> 
> I don't understand this and the changelog doesn't help

I'll add the information to the changelog, but basically, we're getting this compiler error:

missing field 'message' initializer [-Werror,-Wmissing-field-initializers]

[Jonathan Bedard]

Created attachment 431013 [details]
Patch

[EWS]

Tools/Scripts/svn-apply failed to apply attachment 431013 [details] to trunk.
Please resolve the conflicts and upload a new patch.

[Jonathan Bedard]

Created attachment 431222 [details]
Patch for landing

[EWS]

Committed r278780 (238737@main): <https://commits.webkit.org/238737@main>

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

[Chris Dumez]

Seems to have broken internal build?

/Volumes/Data/WebKit/OpenSource/WebKitBuild/Debug/usr/local/include/pal/spi/mac/QuickLookMacSPI.h:78:28: error: redefinition of 'QLPreviewActivity'
typedef NS_ENUM(NSInteger, QLPreviewActivity) {

[Jonathan Bedard]

Committed r278781 (238738@main): <https://commits.webkit.org/238738@main>

[Jonathan Bedard]

Reopening to attach new patch.

[Jonathan Bedard]

Created attachment 431285 [details]
Patch

[Chris Dumez]

Comment on attachment 431285 [details]
Patch

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

> Source/WebCore/html/HTMLElement.cpp:1377
> +                TextRecognitionLineElements lineElements { lineOrDataDetector, Vector<Ref<HTMLDivElement>>() };

Can probably use:
TextRecognitionLineElements lineElements { lineOrDataDetector, { } };

> Source/WebCore/html/HTMLElement.cpp:1430
> +            TextRecognitionLineElements lineElements { lineContainer, Vector<Ref<HTMLDivElement>>() };

ditto.

[Jonathan Bedard]

Created attachment 431338 [details]
Patch

[Jonathan Bedard]

(In reply to Chris Dumez from comment #15)
> Comment on attachment 431285 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=431285&action=review
> 
> > Source/WebCore/html/HTMLElement.cpp:1377
> > +                TextRecognitionLineElements lineElements { lineOrDataDetector, Vector<Ref<HTMLDivElement>>() };
> 
> Can probably use:
> TextRecognitionLineElements lineElements { lineOrDataDetector, { } };
> 
> > Source/WebCore/html/HTMLElement.cpp:1430
> > +            TextRecognitionLineElements lineElements { lineContainer, Vector<Ref<HTMLDivElement>>() };
> 
> ditto.

Good call, that does work is certainly an improvement.

[EWS]

Committed r278834 (238785@main): <https://commits.webkit.org/238785@main>

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

[Jonathan Bedard]

Reopening to attach new patch.

[Jonathan Bedard]

Created attachment 431455 [details]
Patch for landing

[EWS]

Committed r278889 (238831@main): <https://commits.webkit.org/238831@main>

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