Summary: | [Cocoa] Make it easier to encode NSObjects for WebKit IPC | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Andy Estes <aestes> | ||||||||||||||||||
Component: | New Bugs | Assignee: | Andy Estes <aestes> | ||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||
Severity: | Normal | CC: | achristensen, aestes, andersca, commit-queue, dino, ews-watchlist, koivisto, sam, thorton, webkit-bug-importer, wenson_hsieh | ||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||
Attachments: |
|
Description
Andy Estes
2018-11-24 18:31:53 PST
Created attachment 355569 [details]
Patch
Created attachment 355570 [details]
Patch
Created attachment 355571 [details]
Patch
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.
Created attachment 355573 [details]
Patch
Created attachment 355576 [details]
Patch
Created attachment 355578 [details]
Patch
Created attachment 355649 [details]
Patch
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? (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. Created attachment 355664 [details]
Patch
Comment on attachment 355664 [details] Patch Clearing flags on attachment: 355664 Committed r238515: <https://trac.webkit.org/changeset/238515> All reviewed patches have been landed. Closing bug. |