Recent changes were made (bug# 192589) to refactor HarfBuzz font creation from OpenTypeMathData to FontPlatformData. In the process, conditional compilation directives were lost. The attached patch restores the conditional compilation logic.
Created attachment 358077 [details] Patch
Comment on attachment 358077 [details] Patch r=me to get back to the previous status quo, but it looks problematic. Jim, what port are you building, and with what build flags? Hey Don, here's another one. ENABLE(OPENTYPE_MATH) is not a valid feature flag, but it's being used throughout several files under platform/graphics/opentype. It's defined only in OptionsGTK.cmake using cpp flags: add_definitions(-DENABLE_OPENTYPE_MATH=1) Then looking at USE(HARFBUZZ), my initial reaction is confusion. Surely USE(FREETYPE) must imply USE(HARFBUZZ), but it looks like USE(HARFBUZZ) is only set in OptionsPlayStation.cmake, which is nuts because GTK and WPE both use harfbuzz too, so it must be set somewhere else, but I can't find it. It's used in FontCascade.cpp, HbUniquePtr.h, and OpenTypeMathData.[cpp,h] so that's surely not enough places to actually build if harfbuzz were actually not present.
(In reply to Michael Catanzaro from comment #2) > Jim, > what port are you building, and with what build flags? > > Hey Don, here's another one. ENABLE(OPENTYPE_MATH) is not a valid feature > flag, but it's being used throughout several files under > platform/graphics/opentype. It's defined only in OptionsGTK.cmake using cpp > flags Indeed, OPENTYPE_MATH is being set for me automatically, as I am building using an older version of HarfBuzz. OptionsGTK.cmake sets the flag if HarfBuzz is older than version 1.3.3, and I am using 1.0.6. When the code was in OpenTypeMathData, it was compiled according to the construct: #if ENABLE(OPENTYPE_MATH) #elif USE(HARFBUZZ) ..code that was refactored lived here.. Thus, the code was compiled only if USE(HARFBUZZ) && !ENABLE(OPENTYPE_MATH). Conditioning compilation on !ENABLE(OPENTYPE_MATH) is necessary, as the code references an API that is not exposed by the earlier versions of HarfBuzz.
Comment on attachment 358077 [details] Patch Clearing flags on attachment: 358077 Committed r239554: <https://trac.webkit.org/changeset/239554>
All reviewed patches have been landed. Closing bug.
<rdar://problem/46951923>
Thanks for the quick review, Michael. To summarize, !ENABLE(OPENTYPE_MATH) is the active part of this fix. Without it, users of older versions of HarfBuzz encounter effectively dead code that does not compile. USE(HARFBUZZ) was included in the fix, as it was part of the original logic. Whether it is necessary or redundant in other situations, I cannot say. However, I can say in this specific case, it is unnecessary.