[Cocoa] Add SPI to disallow user-installed fonts
Created attachment 327718 [details] WIP
Created attachment 327724 [details] WIP
Attachment 327724 [details] did not pass style-queue: 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: 1 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 327724 [details] WIP Attachment 327724 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5384514 New failing tests: fast/text/user-installed-disable.html
Created attachment 327730 [details] Archive of layout-test-results from ews125 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Created attachment 327761 [details] WIP
Attachment 327761 [details] did not pass style-queue: 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: 1 in 40 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 327821 [details] WIP
Attachment 327821 [details] did not pass style-queue: 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: 1 in 40 files If any of these errors are false positives, please file a bug against check-webkit-style.
My test font is insufficient because it has the same traits as real Helvetica, and the font selection algorithm currently selects the first font that CoreText returns. CoreText returns both Helveticas, with the system one first, which means that my fake Helvetica isn't used at all.
(In reply to Myles C. Maxfield from comment #10) > My test font is insufficient because it has the same traits as real > Helvetica, and the font selection algorithm currently selects the first font > that CoreText returns. CoreText returns both Helveticas, with the system one > first, which means that my fake Helvetica isn't used at all. The system Helvetica has weights 400, 300, and 700. If my fake Helvetica has weight 500, it should be able to be used.
Created attachment 327830 [details] Patch
Attachment 327830 [details] did not pass style-queue: 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: 1 in 40 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 327830 [details] Patch Attachment 327830 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5399492 New failing tests: fast/text/user-installed-fonts/shadow.html
Created attachment 327840 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Created attachment 327862 [details] Patch
Attachment 327862 [details] did not pass style-queue: 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: 1 in 40 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 327862 [details] Patch Attachment 327862 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5404954 New failing tests: fast/text/user-installed-fonts/shadow.html
Created attachment 327873 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Created attachment 328027 [details] WIP
<rdar://problem/35042408>
CTFontManagerRegisterFontsForURL() is returning false on iOS
iOS won't allow registering a font with a duplicate PostScript name.
Created attachment 328553 [details] Needs iOS test update
Attachment 328553 [details] did not pass style-queue: 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: 1 in 42 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 328553 [details] Needs iOS test update Attachment 328553 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5512216 New failing tests: fast/text/user-installed-fonts/shadow.html
Created attachment 328560 [details] Archive of layout-test-results from ews126 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Created attachment 328609 [details] Patch
Created attachment 328640 [details] Patch
Created attachment 328650 [details] Patch
Comment on attachment 328650 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328650&action=review > Source/WebCore/platform/graphics/FontCache.h:205 > + Vector<FontSelectionCapabilities> getFontSelectionCapabilitiesInFamily(const AtomicString&, bool shouldDisallowUserInstalledFonts); why not use the AllowUserInstalledFonts enum? > Source/WebCore/platform/graphics/FontDescription.h:117 > + bool mayRepresentUserInstalledFont() const { return m_mayRepresentUserInstalledFont; } AllowUserInstalledFonts? > Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:989 > + AllowUserInstalledFonts allowUserInstalledFonts; Why not m_ > Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:1179 > +Vector<FontSelectionCapabilities> FontCache::getFontSelectionCapabilitiesInFamily(const AtomicString& familyName, bool shouldDisallowUserInstalledFonts) AllowUserInstalledFonts? > Source/WebCore/testing/InternalSettings.cpp:735 > +ExceptionOr<void> InternalSettings::setShouldDisallowUserInstalledFonts(bool shouldDisallow) > +{ > + if (!m_page) > + return Exception { InvalidAccessError }; > + settings().setShouldDisallowUserInstalledFonts(shouldDisallow); > + return { }; > +} > + Not sure why you need this. Settings get exposed to tests already without any code. > Source/WebCore/testing/InternalSettings.idl:105 > + [MayThrowException] void setShouldDisallowUserInstalledFonts(boolean shouldDisallow); Don't think you need this. > Source/WebKit/UIProcess/API/Cocoa/WKPreferencesPrivate.h:118 > +@property (nonatomic, setter=_setShouldDisallowUserInstalledFonts:) BOOL _shouldDisallowUserInstalledFonts WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA)); "shouldDisallow" or just "disallows"? Is it a question of "allowing" them, or just using them? Maybe "setUsesUserInstalledFonts"? > Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mm:139 > + auto bufferSize = WKStringGetMaximumUTF8CStringSize(configuration); > + char characterBuffer[bufferSize]; > + WKStringGetUTF8CString(configuration, characterBuffer, bufferSize); Don't we have WKStringRef -> NSString helpers? > Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mm:153 > + NSMutableArray *fontsToRemove = [NSMutableArray arrayWithCapacity:0]; [NSMutableArray array];
Comment on attachment 328650 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328650&action=review >> Source/WebCore/platform/graphics/FontDescription.h:117 >> + bool mayRepresentUserInstalledFont() const { return m_mayRepresentUserInstalledFont; } > > AllowUserInstalledFonts? This one is a little different because it represents whether or not this object may represent a user installed font, rather than whether or not user installed fonts are allowed in general. >> Source/WebCore/testing/InternalSettings.idl:105 >> + [MayThrowException] void setShouldDisallowUserInstalledFonts(boolean shouldDisallow); > > Don't think you need this. Wow it's magic! >> Source/WebKit/UIProcess/API/Cocoa/WKPreferencesPrivate.h:118 >> +@property (nonatomic, setter=_setShouldDisallowUserInstalledFonts:) BOOL _shouldDisallowUserInstalledFonts WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA)); > > "shouldDisallow" or just "disallows"? > > Is it a question of "allowing" them, or just using them? Maybe "setUsesUserInstalledFonts"? I thought about this, but chose "should" because "disallow" sounds like an immediate verb, rather than setting a bit of state which gets used subsequently. >> Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mm:139 >> + WKStringGetUTF8CString(configuration, characterBuffer, bufferSize); > > Don't we have WKStringRef -> NSString helpers? Yep, in WKStringCF.h
Created attachment 328715 [details] Patch for committing
Comment on attachment 328715 [details] Patch for committing Clearing flags on attachment: 328715 Committed r225641: <https://trac.webkit.org/changeset/225641>
Filed Bug 180575 to fix GTK port WebKitTestRunner.
This caused a consistent Image only failure on the test fast/text/user-installed-fonts/shadow-postscript-family.html on all platforms except Windows. https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Ftext%2Fuser-installed-fonts%2Fshadow-postscript-family.html https://build.webkit.org/builders/Apple%20iOS%2011%20Simulator%20Debug%20WK2%20(Tests)/builds/1606
(In reply to Matt Lewis from comment #36) > This caused a consistent Image only failure on the test > fast/text/user-installed-fonts/shadow-postscript-family.html on all > platforms except Windows. > > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard. > html#showAllRuns=true&tests=fast%2Ftext%2Fuser-installed-fonts%2Fshadow- > postscript-family.html > https://build.webkit.org/builders/ > Apple%20iOS%2011%20Simulator%20Debug%20WK2%20(Tests)/builds/1606 The failure is only showing up on iOS Simulator due to expectations set.
(In reply to Matt Lewis from comment #37) > (In reply to Matt Lewis from comment #36) > > This caused a consistent Image only failure on the test > > fast/text/user-installed-fonts/shadow-postscript-family.html on all > > platforms except Windows. > > > > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard. > > html#showAllRuns=true&tests=fast%2Ftext%2Fuser-installed-fonts%2Fshadow- > > postscript-family.html > > https://build.webkit.org/builders/ > > Apple%20iOS%2011%20Simulator%20Debug%20WK2%20(Tests)/builds/1606 > > The failure is only showing up on iOS Simulator due to expectations set. I probably just messed up the expectations. Please don't roll this out quite yet - I can probably fix this today.
The flakiness dashboard shows that it is failing on WK1, which is expected. It's also failing on iOS WK2, which is unexpected. Investigating now.
Myles, Do you have any update on this?
(In reply to Matt Lewis from comment #40) > Myles, Do you have any update on this? https://trac.webkit.org/changeset/228147/webkit