Bug 237991 - Fix the build with the Monterey 12.3 SDK
Summary: Fix the build with the Monterey 12.3 SDK
Status: RESOLVED DUPLICATE of bug 238010
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: Safari 13
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryan Haddad
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-03-16 17:26 PDT by Ryan Haddad
Modified: 2022-03-17 10:47 PDT (History)
9 users (show)

See Also:


Attachments
Patch (9.18 KB, patch)
2022-03-16 17:33 PDT, Ryan Haddad
no flags Details | Formatted Diff | Diff
Patch (7.53 KB, patch)
2022-03-17 10:21 PDT, Ryan Haddad
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryan Haddad 2022-03-16 17:26:29 PDT
Fix the various build failures seen with the macOS Monterey 12.3 SDK
Comment 1 Ryan Haddad 2022-03-16 17:33:44 PDT
Created attachment 454919 [details]
Patch
Comment 2 Alexey Proskuryakov 2022-03-17 09:11:34 PDT
Looks like some of this was already fixed in bug 238010.
Comment 3 Ryan Haddad 2022-03-17 09:13:28 PDT
Comment on attachment 454919 [details]
Patch

I've been told that though this built for me, I have used ASSERT_UNUSED incorrectly.
Comment 4 Alexey Proskuryakov 2022-03-17 09:14:26 PDT
Comment on attachment 454919 [details]
Patch

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

> Source/WebCore/platform/audio/mac/AudioOutputUnitAdaptorMac.cpp:58
>      ASSERT(!result);
> +    ASSERT_UNUSED(result, result);

This is not correct. Should be just one line,

ASSERT_UNUSED(result, !result);

I'm surprised that we haven't seen this until now.
Comment 5 Jonathan Bedard 2022-03-17 09:40:15 PDT
Comment on attachment 454919 [details]
Patch

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

> Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.mm:380
> +    ASSERT_UNUSED(status, status);

Per Alexey's comment above, shouldn't this be:
ASSERT_UNUSED(status, status == noErr);

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:3543
> +        ASSERT_UNUSED(valuesOK, valuesOK);

Should replace the above line, right?

> Source/WebKit/Shared/cf/ArgumentCodersCF.cpp:654
> +    ASSERT_UNUSED(numConvertedBytes, numConvertedBytes);

Should replace the above line, right?

> Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm:441
> +    ASSERT_UNUSED(copied, copied);

Should replace the above line, right?

> Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewCloseAllMediaPresentations.mm:51
> +    ASSERT_UNUSED(presentationModeChanged, presentationModeChanged);

Feels wrong to have this assertion here, right? Because this variable is used on the line right below this.
Comment 6 Ryan Haddad 2022-03-17 10:21:02 PDT
Created attachment 454989 [details]
Patch
Comment 7 Ryan Haddad 2022-03-17 10:22:12 PDT
(In reply to Jonathan Bedard from comment #5)
> Comment on attachment 454919 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=454919&action=review
> 
> > Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.mm:380
> > +    ASSERT_UNUSED(status, status);
> 
> Per Alexey's comment above, shouldn't this be:
> ASSERT_UNUSED(status, status == noErr);
> 
> > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:3543
> > +        ASSERT_UNUSED(valuesOK, valuesOK);
> 
> Should replace the above line, right?
> 
> > Source/WebKit/Shared/cf/ArgumentCodersCF.cpp:654
> > +    ASSERT_UNUSED(numConvertedBytes, numConvertedBytes);
> 
> Should replace the above line, right?
> 
> > Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm:441
> > +    ASSERT_UNUSED(copied, copied);
> 
> Should replace the above line, right?
> 
> > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewCloseAllMediaPresentations.mm:51
> > +    ASSERT_UNUSED(presentationModeChanged, presentationModeChanged);
> 
> Feels wrong to have this assertion here, right? Because this variable is
> used on the line right below this.
I encountered a build failure without this, let me re-check with my corrected patch.
Comment 8 Ryan Haddad 2022-03-17 10:28:43 PDT
Oh, most of these were fixed by https://bugs.webkit.org/show_bug.cgi?id=238010, duping.

*** This bug has been marked as a duplicate of bug 238010 ***
Comment 9 Alexey Proskuryakov 2022-03-17 10:47:56 PDT
Comment on attachment 454989 [details]
Patch

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

> Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewCloseAllMediaPresentations.mm:51
> +    ASSERT_UNUSED(presentationModeChanged, presentationModeChanged);

This should be UNUSED_VARIABLE. ASSERT_UNUSED would fail anyway, as we just set it to false above.

But looking further into the code, the real fix is to change an ASSERT below to ASSERT_UNUSED. And in fact, this was also done in r291403!