Bug 226978

Summary:

[Mac] CoreMedia WrapperClass does not need alignment fixup

Product:

WebKit

Reporter:

Eric Carlson <eric.carlson>

Component:

Media

Assignee:

Eric Carlson <eric.carlson>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ews-watchlist, ggaren, glenn, jer.noble, mitz, philipj, sergio, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none

Description Eric Carlson 2021-06-14 09:30:50 PDT

CoreMedia WrapperClass does not need the alignment fixup required by CMBaseClass

Comment 1 Eric Carlson 2021-06-14 09:31:08 PDT

rdar://78864290

Comment 2 Eric Carlson 2021-06-14 11:51:12 PDT

Created attachment 431348 [details]
Patch

Comment 3 Geoffrey Garen 2021-06-14 11:57:34 PDT

Comment on attachment 431348 [details]
Patch

r=me

Comment 4 mitz 2021-06-14 11:57:48 PDT

Comment on attachment 431348 [details]
Patch

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

> Source/WebKit/Shared/mac/MediaFormatReader/CoreMediaWrapped.h:158
> +    static constexpr struct { WrapperClass derivedClass; } derivedClass { Wrapped::wrapperClass() };

Don’t really need this to be a struct anymore.

Comment 5 mitz 2021-06-14 11:58:30 PDT

Comment on attachment 431348 [details]
Patch

Didn’t mean to reset Geoff’s r+ though.

Comment 6 Geoffrey Garen 2021-06-14 11:59:35 PDT

> > Source/WebKit/Shared/mac/MediaFormatReader/CoreMediaWrapped.h:158
> > +    static constexpr struct { WrapperClass derivedClass; } derivedClass { Wrapped::wrapperClass() };
> 
> Don’t really need this to be a struct anymore.

👍🏻

Comment 7 Eric Carlson 2021-06-14 12:46:58 PDT

(In reply to mitz from comment #4)
> Comment on attachment 431348 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=431348&action=review
> 
> > Source/WebKit/Shared/mac/MediaFormatReader/CoreMediaWrapped.h:158
> > +    static constexpr struct { WrapperClass derivedClass; } derivedClass { Wrapped::wrapperClass() };
> 
> Don’t really need this to be a struct anymore.

Good point, thanks!

Comment 8 Eric Carlson 2021-06-14 13:01:05 PDT

Created attachment 431357 [details]
Patch

Comment 9 EWS 2021-06-14 14:47:08 PDT

Found 1 new test failure: imported/w3c/web-platform-tests/webrtc-extensions/transfer-datachannel-service-worker.https.html

Comment 10 Eric Carlson 2021-06-15 06:10:39 PDT

Comment on attachment 431357 [details]
Patch

Failing test is unrelated

Comment 11 EWS 2021-06-15 06:14:03 PDT

Committed r278870 (238814@main): <https://commits.webkit.org/238814@main>

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