Summary: | Stop defining CGFloat in WebKit | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||||
Component: | WebKit Misc. | Assignee: | Alexey Proskuryakov <ap> | ||||||
Status: | ASSIGNED --- | ||||||||
Severity: | Normal | CC: | achristensen, darin, simon.fraser, thorton | ||||||
Priority: | P2 | ||||||||
Version: | Other | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=192557 | ||||||||
Attachments: |
|
Description
Alexey Proskuryakov
2018-12-05 14:14:41 PST
Created attachment 356656 [details]
proposed patch
Comment on attachment 356656 [details]
proposed patch
r=me if this doesn't break anything.
Comment on attachment 356656 [details]
proposed patch
Well, it does break the build.
Created attachment 356747 [details]
...and include CoreGraphics.h
Let's include CoreGraphics.h instead. Having it in the prefix file may even make the build faster (and if it gets slower, we'll roll back).
Comment on attachment 356747 [details] ...and include CoreGraphics.h View in context: https://bugs.webkit.org/attachment.cgi?id=356747&action=review > Source/WebCore/WebCorePrefix.h:90 > +#include <CoreGraphics/CoreGraphics.h> Why did you remove all the CG headers? "The project should be able to build without this header, although we rarely test that." > Source/WebKitLegacy/mac/ChangeLog:18 > +2018-12-05 Alexey Proskuryakov <ap@apple.com> > + > + Stop defining CGFloat in WebKit > + https://bugs.webkit.org/show_bug.cgi?id=192427 > + > + Reviewed by NOBODY (OOPS!). > + > + * WebKitPrefix.h: Double changelog Comment on attachment 356747 [details] ...and include CoreGraphics.h View in context: https://bugs.webkit.org/attachment.cgi?id=356747&action=review >> Source/WebCore/WebCorePrefix.h:90 >> +#include <CoreGraphics/CoreGraphics.h> > > Why did you remove all the CG headers? > > "The project should be able to build without this header, although we rarely test that." Should we just include CGGeometry.h instead? > "The project should be able to build without this header, although we rarely test that." I didn't notice this comment, but it looks like a lie to me. There are lots of things that seem like they are required for Mac build in this header. And I don't know why that would be undesirable. Windows did fail to build, so maybe it's actually building without the prefix. I'm thinking that I should fix WebCoreTestSupport to use the prefix. > Should we just include CGGeometry.h instead? I'm hoping that having all of CoreGraphics in precompiled header will make our builds faster. Comment on attachment 356747 [details] ...and include CoreGraphics.h View in context: https://bugs.webkit.org/attachment.cgi?id=356747&action=review >>> Source/WebCore/WebCorePrefix.h:90 >>> +#include <CoreGraphics/CoreGraphics.h> >> >> Why did you remove all the CG headers? >> >> "The project should be able to build without this header, although we rarely test that." > > Should we just include CGGeometry.h instead? As discussed in webkit-dev, if we add CoreGraphics.h to both this and to "config.h" then I think it’s legit to remove all those includes elsewhere. And I think the decision should probably be based on compile speed. Agreed that it would be useful to test the build speed theory before moving forward. Posted a patch that just adds the include to WebCorePrefix in bug 192557. Adding CoreGraphics to WebCorePrefix didn't change build speed on Mac at all. |