Summary: | Gigacage: enable only for WebContent process and token executables | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | JF Bastien <jfbastien> | ||||||
Component: | bmalloc | Assignee: | JF Bastien <jfbastien> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, ews-watchlist, fpizlo, ggaren, jfbastien, keith_miller, mark.lam, msaboff, rmorisset, saam, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 182580 | ||||||||
Attachments: |
|
Description
JF Bastien
2018-02-02 21:30:45 PST
Created attachment 333027 [details]
patch
Attachment 333027 [details] did not pass style-queue:
ERROR: Source/bmalloc/bmalloc/ProcessCheck.mm:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4]
Total errors found: 1 in 7 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 333027 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=333027&action=review r=me with comment. > Source/bmalloc/bmalloc/ProcessCheck.h:34 > +bool gigacageEnabledForProcess() { return false; } Don't you need to mark this inline or forward declare the function for this to build? > Source/bmalloc/bmalloc/ProcessCheck.h:39 > +bool gigacageEnabledForProcess() { return true; } ditto, idk how this builds... Created attachment 333084 [details] patch > > Source/bmalloc/bmalloc/ProcessCheck.h:34 > > +bool gigacageEnabledForProcess() { return false; } > > Don't you need to mark this inline or forward declare the function for this > to build? > > > Source/bmalloc/bmalloc/ProcessCheck.h:39 > > +bool gigacageEnabledForProcess() { return true; } > > ditto, idk how this builds... Oops yeah, included in just one .cpp file so it Just Works, but it's also Just Wrong. Attachment 333084 [details] did not pass style-queue:
ERROR: Source/bmalloc/bmalloc/ProcessCheck.mm:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4]
Total errors found: 1 in 7 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 333084 [details] patch Clearing flags on attachment: 333084 Committed r228108: <https://trac.webkit.org/changeset/228108> All reviewed patches have been landed. Closing bug. Comment on attachment 333084 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=333084&action=review > Source/bmalloc/bmalloc/ProcessCheck.mm:36 > + static NSString *appName = [[NSBundle mainBundle] bundleIdentifier]; Style Nit: I’d just wrap this entire thing in a do_once and get rid of these static variables > Source/bmalloc/bmalloc/ProcessCheck.mm:45 > + static bool isOptInBinary = [processName isEqualToString:@"jsc"] Is it worth opting in minibrowser since we use that as a proxy for the full browser often. Also, what about DumpRenderTree and WebkitTestRunner? We should definitely opt those in |