FontAccess: Use browser backend for Mac enumeration

This change switches the Mac font enumeration API backend to use the
browser-side implementation. This makes it in-line with all the other
platforms and simplifies the code.

The renderer-side implementation is removed as well.

Fixed: 1119575,1061625,1063172
Bug: 2410594
Change-Id: I3751a5ac7496f6990f4e1f7132e068900e0a1cf2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2411330Reviewed-by: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Martin Barbella <mbarbella@chromium.org>
Reviewed-by: Joshua Bell <jsbell@chromium.org>
Commit-Queue: Olivier Yiptong <oyiptong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808981}

content/browser/font_access/font_access_manager_impl.cc

@@ -35,44 +35,6 @@ void FontAccessManagerImpl::BindReceiver(
   receivers_.Add(this, std::move(receiver), context);
 }
 
-#if defined(OS_MAC)
-void FontAccessManagerImpl::RequestPermission(
-    RequestPermissionCallback callback) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-
-  const BindingContext& context = receivers_.current_context();
-  RenderFrameHost* rfh = RenderFrameHost::FromID(context.frame_id);
-
-  auto status = PermissionControllerImpl::FromBrowserContext(
-                    rfh->GetProcess()->GetBrowserContext())
-                    ->GetPermissionStatusForFrame(PermissionType::FONT_ACCESS,
-                                                  rfh, context.origin.GetURL());
-
-  if (status != blink::mojom::PermissionStatus::ASK) {
-    // Permission has been requested before.
-    std::move(callback).Run(status);
-    return;
-  }
-
-  if (!rfh->HasTransientUserActivation()) {
-    std::move(callback).Run(blink::mojom::PermissionStatus::DENIED);
-    return;
-  }
-
-  PermissionControllerImpl::FromBrowserContext(
-      rfh->GetProcess()->GetBrowserContext())
-      ->RequestPermission(PermissionType::FONT_ACCESS, rfh,
-                          context.origin.GetURL(),
-                          /*user_gesture=*/true,
-                          base::BindOnce(
-                              [](RequestPermissionCallback callback,
-                                 blink::mojom::PermissionStatus status) {
-                                std::move(callback).Run(status);
-                              },
-                              std::move(callback)));
-}
-#endif
-
 void FontAccessManagerImpl::EnumerateLocalFonts(
     EnumerateLocalFontsCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

content/browser/font_access/font_access_manager_impl.h

@@ -35,12 +35,6 @@ class CONTENT_EXPORT FontAccessManagerImpl
       mojo::PendingReceiver<blink::mojom::FontAccessManager> receiver);
 
   // blink.mojom.FontAccessManager:
-#if defined(OS_MAC)
-  // TODO(crbug.com/1119575): Remove this IPC method. It is there due to
-  // the Mac enumeration implementation being done renderer-side and only
-  // the permission request being needed browser-side.
-  void RequestPermission(RequestPermissionCallback callback) override;
-#endif
   void EnumerateLocalFonts(EnumerateLocalFontsCallback callback) override;
 
  private:

content/browser/font_access/font_access_manager_impl_unittest.cc

@@ -166,73 +166,6 @@ class FontAccessManagerImplTest : public RenderViewHostImplTestHarness {
   base::test::ScopedFeatureList scoped_feature_list_;
 };
 
-#if defined(OS_MAC)
-TEST_F(FontAccessManagerImplTest, NoUserActivationPermissionDenied) {
-  ASSERT_TRUE(manager_remote_.is_bound() && manager_remote_.is_connected());
-  AskGrantPermission();
-
-  base::RunLoop loop;
-  bool permission_requested = false;
-  manager_remote_->RequestPermission(
-      base::BindLambdaForTesting([&](blink::mojom::PermissionStatus status) {
-        permission_requested = true;
-        EXPECT_EQ(blink::mojom::PermissionStatus::DENIED, status)
-            << "No user activation yields a permission denied status";
-        loop.Quit();
-      }));
-
-  loop.Run();
-  EXPECT_TRUE(permission_requested) << "Permission has been requested";
-}
-
-TEST_F(FontAccessManagerImplTest, UserActivationPermissionManagerTriggered) {
-  ASSERT_TRUE(manager_remote_.is_bound() && manager_remote_.is_connected());
-  AskGrantPermission();
-  SimulateUserActivation();
-
-  base::RunLoop loop;
-  bool permission_requested = false;
-  manager_remote_->RequestPermission(
-      base::BindLambdaForTesting([&](blink::mojom::PermissionStatus status) {
-        permission_requested = true;
-        EXPECT_EQ(blink::mojom::PermissionStatus::GRANTED, status)
-            << "User activation yields a permission granted status";
-        loop.Quit();
-      }));
-
-  loop.Run();
-  EXPECT_TRUE(permission_requested) << "Permission has been requested";
-}
-
-TEST_F(FontAccessManagerImplTest, PreviouslyGrantedPermissionNoActivation) {
-  ASSERT_TRUE(manager_remote_.is_bound() && manager_remote_.is_connected());
-  AutoGrantPermission();
-
-  base::RunLoop loop;
-  manager_remote_->RequestPermission(
-      base::BindLambdaForTesting([&](blink::mojom::PermissionStatus status) {
-        EXPECT_EQ(blink::mojom::PermissionStatus::GRANTED, status)
-            << "Permission status is granted";
-        loop.Quit();
-      }));
-  loop.Run();
-}
-
-TEST_F(FontAccessManagerImplTest, PreviouslyDeniedPermissionNoActivation) {
-  ASSERT_TRUE(manager_remote_.is_bound() && manager_remote_.is_connected());
-  AutoDenyPermission();
-
-  base::RunLoop loop;
-  manager_remote_->RequestPermission(
-      base::BindLambdaForTesting([&](blink::mojom::PermissionStatus status) {
-        EXPECT_EQ(blink::mojom::PermissionStatus::DENIED, status)
-            << "Permission status is denied";
-        loop.Quit();
-      }));
-  loop.Run();
-}
-#endif
-
 #if defined(OS_WIN) || defined(OS_LINUX) || defined(OS_CHROMEOS) || \
     defined(OS_MAC)
 namespace {

third_party/blink/public/mojom/font_access/font_access.mojom

@@ -21,11 +21,6 @@ enum FontEnumerationStatus {
 // and workers in renderer processes.
 // Provides methods related to enumerating locally installed fonts.
 interface FontAccessManager {
-  // Provides a permission request response. Used on Mac because fonts are
-  // enumerated in-renderer on that platform.
-  [EnableIf=is_mac]
-  RequestPermission() => (PermissionStatus status);
-
   // Enumerate locally installed fonts. Results will be gated by a permission
   // check.
   EnumerateLocalFonts() => (FontEnumerationStatus enumeration_status, mojo_base.mojom.ReadOnlySharedMemoryRegion? enumeration_table);

third_party/blink/renderer/modules/font_access/font_iterator.cc

@@ -14,7 +14,6 @@
 #include "third_party/blink/renderer/core/dom/dom_exception.h"
 #include "third_party/blink/renderer/modules/font_access/font_metadata.h"
 #include "third_party/blink/renderer/platform/bindings/script_state.h"
-#include "third_party/blink/renderer/platform/fonts/font_cache.h"
 #include "third_party/blink/renderer/platform/wtf/functional.h"
 
 namespace blink {
@@ -30,13 +29,8 @@ FontIterator::FontIterator(ExecutionContext* context)
 ScriptPromise FontIterator::next(ScriptState* script_state) {
   if (permission_status_ == PermissionStatus::ASK) {
     if (!pending_resolver_) {
-#if defined(OS_MAC)
-      remote_manager_->RequestPermission(WTF::Bind(
-          &FontIterator::DidGetPermissionResponse, WrapWeakPersistent(this)));
-#else
       remote_manager_->EnumerateLocalFonts(WTF::Bind(
           &FontIterator::DidGetEnumerationResponse, WrapWeakPersistent(this)));
-#endif
       pending_resolver_ =
           MakeGarbageCollected<ScriptPromiseResolver>(script_state);
     }
@@ -73,26 +67,6 @@ FontIteratorEntry* FontIterator::GetNextEntry() {
   return result;
 }
 
-void FontIterator::DidGetPermissionResponse(PermissionStatus status) {
-  permission_status_ = status;
-
-  if (permission_status_ != PermissionStatus::GRANTED) {
-    pending_resolver_->Reject(MakeGarbageCollected<DOMException>(
-        DOMExceptionCode::kNotAllowedError, "Permission Error"));
-    pending_resolver_.Clear();
-    return;
-  }
-
-  FontCache* font_cache = FontCache::GetFontCache();
-  auto metadata = font_cache->EnumerateAvailableFonts();
-  for (const auto& entry : metadata) {
-    entries_.push_back(FontMetadata::Create(entry));
-  }
-
-  pending_resolver_->Resolve(GetNextEntry());
-  pending_resolver_.Clear();
-}
-
 void FontIterator::DidGetEnumerationResponse(
     FontEnumerationStatus status,
     base::ReadOnlySharedMemoryRegion region) {

third_party/blink/renderer/modules/font_access/font_iterator.h

@@ -39,7 +39,6 @@ class FontIterator final : public ScriptWrappable,
 
  private:
   FontIteratorEntry* GetNextEntry();
-  void DidGetPermissionResponse(PermissionStatus);
   void DidGetEnumerationResponse(FontEnumerationStatus,
                                  base::ReadOnlySharedMemoryRegion);
   void ContextDestroyed() override;

third_party/blink/renderer/modules/font_access/font_metadata.h

@@ -14,7 +14,12 @@ namespace blink {
 class ScriptState;
 class ScriptPromise;
 class ScriptPromiseResolver;
-struct FontEnumerationEntry;
+
+struct FontEnumerationEntry {
+  String postscript_name;
+  String full_name;
+  String family;
+};
 
 class BLINK_EXPORT FontMetadata final : public ScriptWrappable {
   DEFINE_WRAPPERTYPEINFO();

third_party/blink/renderer/platform/fonts/font_cache.cc

@@ -397,11 +397,6 @@ void FontCache::InvalidateShapeCache() {
   PurgeFallbackListShaperCache();
 }
 
-void FontCache::InvalidateEnumerationCache() {
-  TRACE_EVENT0("fonts,ui", "FontCache::InvalidateEnumerationCache");
-  font_enumeration_cache_.clear();
-}
-
 void FontCache::Purge(PurgeSeverity purge_severity) {
   // Ideally we should never be forcing the purge while the
   // FontCachePurgePreventer is in scope, but we call purge() at any timing
@@ -414,7 +409,6 @@ void FontCache::Purge(PurgeSeverity purge_severity) {
 
   PurgePlatformFontDataCache();
   PurgeFallbackListShaperCache();
-  InvalidateEnumerationCache();
 }
 
 void FontCache::AddClient(FontCacheClient* client) {
@@ -435,10 +429,6 @@ uint16_t FontCache::Generation() {
 void FontCache::Invalidate() {
   TRACE_EVENT0("fonts,ui", "FontCache::Invalidate");
   font_platform_data_cache_.clear();
-  // TODO(https://crbug.com/1061630): Determine optimal cache invalidation
-  // strategy for enumeration. As implemented, the enumeration cache might not
-  // get invalidated when the system fonts change.
-  InvalidateEnumerationCache();
   generation_++;
 
   if (font_cache_clients_) {
@@ -545,17 +535,6 @@ FontCache::Bcp47Vector FontCache::GetBcp47LocaleForRequest(
   return result;
 }
 
-const std::vector<FontEnumerationEntry>& FontCache::EnumerateAvailableFonts() {
-  if (font_enumeration_cache_.size() == 0) {
-    base::TimeTicks enum_start = base::TimeTicks::Now();
-    font_enumeration_cache_ = EnumeratePlatformAvailableFonts();
-    base::TimeDelta time_taken = base::TimeTicks::Now() - enum_start;
-    UMA_HISTOGRAM_TIMES("Blink.Fonts.Enumeration.Duration", time_taken);
-  }
-
-  return font_enumeration_cache_;
-}
-
 FontFallbackMap& FontCache::GetFontFallbackMap() {
   if (!font_fallback_map_) {
     font_fallback_map_ = MakeGarbageCollected<FontFallbackMap>(nullptr);

third_party/blink/renderer/platform/fonts/font_cache.h

@@ -91,12 +91,6 @@ enum class AlternateFontName {
   kLastResort
 };
 
-struct FontEnumerationEntry {
-  String postscript_name;
-  String full_name;
-  String family;
-};
-
 typedef HashMap<unsigned,
                 std::unique_ptr<FontPlatformData>,
                 WTF::IntHash<unsigned>,
@@ -108,7 +102,6 @@ typedef HashMap<FallbackListCompositeKey,
                 FallbackListCompositeKeyHash,
                 FallbackListCompositeKeyTraits>
     FallbackListShaperCache;
-typedef std::vector<FontEnumerationEntry> FontEnumerationCache;
 
 // "und-Zsye", the special locale for retrieving the color emoji font defined
 // in UTS #51: https://unicode.org/reports/tr51/#Emoji_Script
@@ -260,13 +253,7 @@ class PLATFORM_EXPORT FontCache {
       ShouldRetain = kRetain,
       bool subpixel_ascent_descent = false);
 
-  const std::vector<FontEnumerationEntry>& EnumerateAvailableFonts();
-  size_t EnumerationCacheSizeForTesting() {
-    return font_enumeration_cache_.size();
-  }
-
   void InvalidateShapeCache();
-  void InvalidateEnumerationCache();
 
   static void CrashWithFontInfo(const FontDescription*);
 
@@ -328,7 +315,6 @@ class PLATFORM_EXPORT FontCache {
       const FontDescription&,
       const FontFaceCreationParams&,
       float font_size);
-  std::vector<FontEnumerationEntry> EnumeratePlatformAvailableFonts();
 
   sk_sp<SkTypeface> CreateTypeface(const FontDescription&,
                                    const FontFaceCreationParams&,
@@ -389,9 +375,6 @@ class PLATFORM_EXPORT FontCache {
   FontPlatformDataCache font_platform_data_cache_;
   FallbackListShaperCache fallback_list_shaper_cache_;
   FontDataCache font_data_cache_;
-  // TODO(https://crbug.com/1061625): Move to the browser process for better
-  // resource utilization.
-  FontEnumerationCache font_enumeration_cache_;
 
   Persistent<FontFallbackMap> font_fallback_map_;
 

third_party/blink/renderer/platform/fonts/font_cache_test.cc

@@ -144,79 +144,4 @@ TEST(FontCache, systemFont) {
 }
 #endif
 
-class EnumerationConsumer {
- public:
-  explicit EnumerationConsumer(
-      const std::vector<FontEnumerationEntry>& expectations) {
-    for (const auto& f : expectations) {
-      ps_name_set_.insert(f.postscript_name.Utf8());
-      full_name_set_.insert(f.full_name.Utf8());
-      family_set_.insert(f.family.Utf8());
-    }
-  }
-
-  void Consume(const std::vector<FontEnumerationEntry>& entries) {
-    for (auto f : entries) {
-      ps_name_set_.erase(f.postscript_name.Utf8());
-      full_name_set_.erase(f.full_name.Utf8());
-      family_set_.erase(f.family.Utf8());
-    }
-  }
-
-  bool AllExpectationsMet() {
-    return ps_name_set_.empty() && full_name_set_.empty() &&
-           family_set_.empty();
-  }
-
- private:
-  std::set<std::string> ps_name_set_;
-  std::set<std::string> full_name_set_;
-  std::set<std::string> family_set_;
-};
-
-TEST(FontCache, EnumerateAvailableFonts) {
-  FontCache* font_cache = FontCache::GetFontCache();
-  ASSERT_TRUE(font_cache);
-
-  std::vector<FontEnumerationEntry> expectations;
-
-#if defined(OS_MAC)
-  expectations.push_back(FontEnumerationEntry{"Monaco", "Monaco", "Monaco"});
-  expectations.push_back(
-      FontEnumerationEntry{"Menlo-Regular", "Menlo Regular", "Menlo"});
-  expectations.push_back(
-      FontEnumerationEntry{"Menlo-Bold", "Menlo Bold", "Menlo"});
-  expectations.push_back(
-      FontEnumerationEntry{"Menlo-BoldItalic", "Menlo Bold Italic", "Menlo"});
-#endif
-
-  auto entries = font_cache->EnumerateAvailableFonts();
-  auto consumer = EnumerationConsumer(expectations);
-
-  consumer.Consume(entries);
-  ASSERT_TRUE(consumer.AllExpectationsMet());
-}
-
-TEST(FontCache, EnumerateAvailableFontsInvalidation) {
-  FontCache* font_cache = FontCache::GetFontCache();
-  ASSERT_TRUE(font_cache);
-
-  // Make sure we start at zero.
-  font_cache->Invalidate();
-  size_t zero = 0;
-  ASSERT_EQ(zero, font_cache->EnumerationCacheSizeForTesting());
-
-  // The cache gets populated.
-  size_t enum_size_1 = font_cache->EnumerateAvailableFonts().size();
-  ASSERT_EQ(enum_size_1, font_cache->EnumerationCacheSizeForTesting());
-
-  // Invalidation clears the cache.
-  font_cache->Invalidate();
-  ASSERT_EQ(zero, font_cache->EnumerationCacheSizeForTesting());
-
-  // The cache gets re-populated.
-  size_t enum_size_2 = font_cache->EnumerateAvailableFonts().size();
-  ASSERT_EQ(enum_size_1, enum_size_2);
-}
-
 }  // namespace blink

third_party/blink/renderer/platform/fonts/mac/font_cache_mac.mm

@@ -64,22 +64,6 @@
              inLanguage:(id)useNil;
 @end
 
-namespace {
-
-NSString* GetLocalizedString(CTFontDescriptorRef fd, CFStringRef attribute) {
-  base::ScopedCFTypeRef<CFStringRef> cf_str(base::mac::CFCast<CFStringRef>(
-      CTFontDescriptorCopyLocalizedAttribute(fd, attribute, nullptr)));
-  return [base::mac::CFToNSCast(cf_str.release()) autorelease];
-}
-
-NSString* GetString(CTFontDescriptorRef fd, CFStringRef attribute) {
-  base::ScopedCFTypeRef<CFStringRef> cf_str(base::mac::CFCast<CFStringRef>(
-      CTFontDescriptorCopyAttribute(fd, attribute)));
-  return [base::mac::CFToNSCast(cf_str.release()) autorelease];
-}
-
-}  // namespace
-
 namespace blink {
 
 const char kColorEmojiFontMac[] = "Apple Color Emoji";
@@ -327,38 +311,4 @@ std::unique_ptr<FontPlatformData> FontCache::CreateFontPlatformData(
   return platform_data;
 }
 
-std::vector<FontEnumerationEntry> FontCache::EnumeratePlatformAvailableFonts() {
-  @autoreleasepool {
-    base::ElapsedTimer timer;
-    std::vector<FontEnumerationEntry> output;
-
-    CFTypeRef values[1] = {kCFBooleanTrue};
-    base::ScopedCFTypeRef<CFDictionaryRef> options(CFDictionaryCreate(
-        kCFAllocatorDefault,
-        (const void**)kCTFontCollectionRemoveDuplicatesOption,
-        (const void**)&values,
-        /*numValues=*/1, &kCFTypeDictionaryKeyCallBacks,
-        &kCFTypeDictionaryValueCallBacks));
-    base::ScopedCFTypeRef<CTFontCollectionRef> collection(
-        CTFontCollectionCreateFromAvailableFonts(options));
-
-    base::ScopedCFTypeRef<CFArrayRef> font_descs(
-        CTFontCollectionCreateMatchingFontDescriptors(collection));
-
-    for (CFIndex i = 0; i < CFArrayGetCount(font_descs); ++i) {
-      CTFontDescriptorRef fd = base::mac::CFCast<CTFontDescriptorRef>(
-          CFArrayGetValueAtIndex(font_descs, i));
-      NSString* postscript_name = GetString(fd, kCTFontNameAttribute);
-      NSString* full_name = GetLocalizedString(fd, kCTFontDisplayNameAttribute);
-      NSString* family = GetLocalizedString(fd, kCTFontFamilyNameAttribute);
-      output.push_back(FontEnumerationEntry{String(postscript_name),
-                                            String(full_name), String(family)});
-    }
-
-    UMA_HISTOGRAM_MEDIUM_TIMES("Fonts.AccessAPI.EnumerationTime",
-                               timer.Elapsed());
-    return output;
-  }
-}
-
 }  // namespace blink

third_party/blink/renderer/platform/fonts/skia/font_cache_skia.cc

@@ -247,13 +247,6 @@ sk_sp<SkTypeface> FontCache::CreateTypeface(
       name.c_str(), font_description.SkiaFontStyle());
 }
 
-#if !defined(OS_MAC)
-std::vector<FontEnumerationEntry> FontCache::EnumeratePlatformAvailableFonts() {
-  NOTIMPLEMENTED();
-  return std::vector<FontEnumerationEntry>();
-}
-#endif  // !defined(OS_MAC)
-
 #if !defined(OS_WIN)
 std::unique_ptr<FontPlatformData> FontCache::CreateFontPlatformData(
     const FontDescription& font_description,