Bug 191948 – [Cocoa] Make it easier to encode NSObjects for WebKit IPC

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Andy Estes]

[Cocoa] Make it easier to encode NSObjects

[Andy Estes]

Created attachment 355569 [details]
Patch

[Andy Estes]

Created attachment 355570 [details]
Patch

[Andy Estes]

Created attachment 355571 [details]
Patch

[EWS Watchlist]

Attachment 355571 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/applepay/PaymentMethod.h:44:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTreeTransaction.mm:923:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/WebKit/Shared/Cocoa/ArgumentCodersCocoa.h:39:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/applepay/Payment.h:42:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/Modules/applepay/PaymentContact.h:43:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 6 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Andy Estes]

Created attachment 355573 [details]
Patch

[Andy Estes]

Created attachment 355576 [details]
Patch

[Andy Estes]

Created attachment 355578 [details]
Patch

[Andy Estes]

Created attachment 355649 [details]
Patch

[Dean Jackson]

Comment on attachment 355649 [details]
Patch

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

Cool patch!

> Source/WebCore/Modules/applepay/cocoa/PaymentCocoa.mm:70
> +Payment::Payment() = default;

Why did you move this to the implementation?

> Source/WebKit/Shared/Cocoa/ArgumentCodersCocoa.h:57
> +    ASSERT([object isKindOfClass:allowedClasses[0]]);

Maybe ASSERT allowedClasses has entries?

> Source/WebKit/Shared/Cocoa/ArgumentCodersCocoa.h:58
> +    return { adoptNS((T *)object) };

Can you static_cast here?

[Andy Estes]

(In reply to Dean Jackson from comment #9)
> Comment on attachment 355649 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=355649&action=review
> 
> Cool patch!

Thanks for reviewing!

> 
> > Source/WebCore/Modules/applepay/cocoa/PaymentCocoa.mm:70
> > +Payment::Payment() = default;
> 
> Why did you move this to the implementation?

check-webkit-style wanted me to (because it doesn't like inline member functions on exported classes).

> 
> > Source/WebKit/Shared/Cocoa/ArgumentCodersCocoa.h:57
> > +    ASSERT([object isKindOfClass:allowedClasses[0]]);
> 
> Maybe ASSERT allowedClasses has entries?

Yes, good idea.

> 
> > Source/WebKit/Shared/Cocoa/ArgumentCodersCocoa.h:58
> > +    return { adoptNS((T *)object) };
> 
> Can you static_cast here?

Ok.

[Andy Estes]

Created attachment 355664 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 355664 [details]
Patch

Clearing flags on attachment: 355664

Committed r238515: <https://trac.webkit.org/changeset/238515>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/46254165>