Bug 238655 - _WKDataTask doesn't work in macCatalyst
Summary: _WKDataTask doesn't work in macCatalyst
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-04-01 00:34 PDT by Tim Horton
Modified: 2022-04-03 03:59 PDT (History)
11 users (show)

See Also:


Attachments
Patch (3.67 KB, patch)
2022-04-01 00:34 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (16.24 KB, patch)
2022-04-01 14:46 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (26.26 KB, patch)
2022-04-01 15:22 PDT, Tim Horton
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (26.26 KB, patch)
2022-04-01 15:24 PDT, Tim Horton
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (26.33 KB, patch)
2022-04-01 15:26 PDT, Tim Horton
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (26.44 KB, patch)
2022-04-01 15:46 PDT, Tim Horton
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2022-04-01 00:34:22 PDT
_WKDataTask doesn't work in macCatalyst
Comment 1 Tim Horton 2022-04-01 00:34:35 PDT
Created attachment 456331 [details]
Patch
Comment 2 Alexey Proskuryakov 2022-04-01 07:32:34 PDT
Comment on attachment 456331 [details]
Patch

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

> Source/WTF/wtf/PlatformHave.h:458
> +    || (PLATFORM(MACCATALYST) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 140000) \

Please remove obsolete OS version checks instead of adding more.
Comment 3 Brent Fulgham 2022-04-01 10:17:51 PDT
Comment on attachment 456331 [details]
Patch

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

> Source/WTF/wtf/PlatformHave.h:1063
> +// FIXME: Does this intend to exclude macCatalyst?

I believe we do want this on, too.

> Source/WTF/wtf/PlatformHave.h:1201
> +// FIXME: Does this intend to exclude macCatalyst?

We should support this on macCatalyst, too.
Comment 4 Tim Horton 2022-04-01 10:19:28 PDT
Comment on attachment 456331 [details]
Patch

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

>> Source/WTF/wtf/PlatformHave.h:458
>> +    || (PLATFORM(MACCATALYST) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 140000) \
> 
> Please remove obsolete OS version checks instead of adding more.

Good.... point

> Source/WTF/wtf/PlatformHave.h:1201
> +// FIXME: Does this intend to exclude macCatalyst?

Per Arne says this one should be OK to add catalyst
Comment 5 Tim Horton 2022-04-01 14:46:15 PDT
Created attachment 456405 [details]
Patch
Comment 6 Alexey Proskuryakov 2022-04-01 14:54:47 PDT
Comment on attachment 456405 [details]
Patch

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

> Source/WTF/wtf/PlatformHave.h:465
>  #if PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED < 140000
> +// FIXME: Does this intend to exclude macCatalyst?

This is always true, so the whole section can be deleted.

> Source/WTF/wtf/PlatformHave.h:746
> +#if (PLATFORM(IOS) || PLATFORM(MACCATALYST)) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 150000

I don't think that this OS version check is still relevant?
Comment 7 Alexey Proskuryakov 2022-04-01 15:20:15 PDT
Comment on attachment 456405 [details]
Patch

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

>> Source/WTF/wtf/PlatformHave.h:465
>> +// FIXME: Does this intend to exclude macCatalyst?
> 
> This is always true, so the whole section can be deleted.

Always false. Was thinking along the lines of "it's no longer broken" :)
Comment 8 Tim Horton 2022-04-01 15:22:52 PDT
Created attachment 456409 [details]
Patch
Comment 9 Tim Horton 2022-04-01 15:24:57 PDT
Created attachment 456410 [details]
Patch
Comment 10 Tim Horton 2022-04-01 15:26:22 PDT
Created attachment 456411 [details]
Patch
Comment 11 Tim Horton 2022-04-01 15:46:48 PDT
Created attachment 456412 [details]
Patch
Comment 12 EWS 2022-04-03 01:05:16 PDT
Committed r292275 (249173@main): <https://commits.webkit.org/249173@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 456412 [details].
Comment 13 Radar WebKit Bug Importer 2022-04-03 01:06:24 PDT
<rdar://problem/91209620>