Local Font Access: remove getTables method

After developer feedback, the API design has shifted towards returning
font files as a whole (as blobs) rather than requesting individual
tables. Remove the obsolete getTables() method, and associated tests.

Change-Id: I54de7b46ed892d4c7f755e9a9ea7d2db45c6acf2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2354961
Commit-Queue: Kentaro Hara <haraken@chromium.org>
Auto-Submit: Joshua Bell <jsbell@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Olivier Yiptong <oyiptong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798403}

third_party/blink/renderer/modules/BUILD.gn

@@ -500,7 +500,6 @@ source_set("unit_tests") {
     "//third_party/blink/public:blink_headers",
     "//third_party/blink/renderer/core",
     "//third_party/blink/renderer/core:unit_test_support",
-    "//third_party/blink/renderer/modules/font_access:unit_tests",
     "//third_party/blink/renderer/modules/gamepad:unit_tests",
     "//third_party/blink/renderer/modules/hid:unit_tests",
     "//third_party/blink/renderer/modules/native_file_system:unit_tests",

third_party/blink/renderer/modules/font_access/BUILD.gn

@@ -20,21 +20,3 @@ blink_modules_sources("font_access") {
 
   deps = [ "//third_party/blink/renderer/platform" ]
 }
-
-source_set("unit_tests") {
-  testonly = true
-  sources = [ "font_metadata_unittest.cc" ]
-
-  configs += [
-    "//third_party/blink/renderer:config",
-    "//third_party/blink/renderer:inside_blink",
-    "//third_party/blink/renderer/core:blink_core_pch",
-  ]
-
-  deps = [
-    "//testing/gtest",
-    "//third_party/blink/renderer/modules",
-    "//third_party/blink/renderer/platform",
-    "//third_party/blink/renderer/platform/wtf",
-  ]
-}

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

@@ -17,99 +17,6 @@
 #include "third_party/skia/include/core/SkStream.h"
 #include "third_party/skia/include/core/SkTypes.h"
 
-namespace {
-
-enum FontAccessStatus {
-  kOk,
-  // An invalid argument was passed to a method.
-  kInvalidArgument,
-};
-
-struct FontAccessError {
-  FontAccessStatus status;
-  String message;
-
-  bool IsOk() { return status == FontAccessStatus::kOk; }
-
-  static FontAccessError CreateOk() {
-    return FontAccessError{FontAccessStatus::kOk, ""};
-  }
-
-  static FontAccessError CreateInvalidArgument(String message) {
-    return FontAccessError{FontAccessStatus::kInvalidArgument, message};
-  }
-};
-
-bool isValidTableNameCharacter(unsigned int name_char) {
-  // According to the OpenType specifications, each byte in a Tag has a value
-  // between 0x20 and 0x7E.
-  // https://docs.microsoft.com/en-us/typography/opentype/spec/otff#data-types
-  return (name_char >= 0x20 && name_char <= 0x7E);
-}
-
-bool isValidTableName(const String& table_name) {
-  // According to the OpenType specifications, a Tag may be interpreted as a
-  // an array with 4 elements. The String representation will be 4 characters,
-  // each character representing one byte of a Tag.
-  if (table_name.length() != 4) {
-    return false;
-  }
-
-  for (unsigned i = 0; i < 4; ++i) {
-    if (!isValidTableNameCharacter(table_name[i]))
-      return false;
-  }
-  return true;
-}
-
-String convertTagToString(uint32_t tag) {
-#if defined(ARCH_CPU_LITTLE_ENDIAN)
-  // Tag assumed to be in Big Endian byte ordering.
-  // Swap byte ordering if on Little Endian architecture.
-  tag = base::ByteSwap(tag);
-#endif
-  char buf[5];
-  std::memcpy(&buf, &tag, 4);
-  buf[4] = '\0';
-  return String(&buf[0], size_t{4});
-}
-
-std::pair<FontAccessError, uint32_t> convertStringToTag(
-    const String& table_name) {
-  if (!isValidTableName(table_name)) {
-    return std::make_pair(
-        FontAccessError::CreateInvalidArgument(String::Format(
-            "Invalid table name: %s", table_name.Latin1().c_str())),
-        0u);
-  }
-
-  // Tags in OpenType are always in Big Endian order.
-  // https://docs.microsoft.com/en-us/typography/opentype/spec/otff#data-types
-  uint32_t tag;
-
-  // Since we know that table_name is "valid", i.e. all characters are within
-  // the ASCII range, we can safely assume the following works, whether or not
-  // each character is 8 or 16 bit encoded.
-  base::ReadBigEndian(table_name.Ascii().c_str(), &tag);
-
-  return std::make_pair(FontAccessError::CreateOk(), tag);
-}
-
-std::pair<FontAccessError, Vector<uint32_t>> convertTableNamesToTags(
-    const Vector<String>& table_names) {
-  Vector<uint32_t> output;
-  for (auto tname : table_names) {
-    auto tag_result = convertStringToTag(tname);
-    if (!tag_result.first.IsOk()) {
-      return std::make_pair(tag_result.first, Vector<uint32_t>());
-    }
-    output.push_back(tag_result.second);
-  }
-  return std::make_pair(FontAccessError::CreateOk(), std::move(output));
-}
-
-}  // namespace
-
 namespace blink {
 
 FontMetadata::FontMetadata(const FontEnumerationEntry& entry)
@@ -121,34 +28,6 @@ FontMetadata* FontMetadata::Create(const FontEnumerationEntry& entry) {
   return MakeGarbageCollected<FontMetadata>(entry);
 }
 
-ScriptPromise FontMetadata::getTables(ScriptState* script_state) {
-  ScriptPromiseResolver* resolver =
-      MakeGarbageCollected<ScriptPromiseResolver>(script_state);
-  ScriptPromise promise = resolver->Promise();
-
-  Vector<String> tables;
-
-  Thread::Current()->GetTaskRunner()->PostTask(
-      FROM_HERE,
-      WTF::Bind(&FontMetadata::getTablesImpl, WrapPersistent(resolver),
-                postscriptName_, std::move(tables)));
-
-  return promise;
-}
-
-ScriptPromise FontMetadata::getTables(ScriptState* script_state,
-                                      const Vector<String>& tables) {
-  ScriptPromiseResolver* resolver =
-      MakeGarbageCollected<ScriptPromiseResolver>(script_state);
-  ScriptPromise promise = resolver->Promise();
-
-  Thread::Current()->GetTaskRunner()->PostTask(
-      FROM_HERE, WTF::Bind(&FontMetadata::getTablesImpl,
-                           WrapPersistent(resolver), postscriptName_, tables));
-
-  return promise;
-}
-
 ScriptPromise FontMetadata::blob(ScriptState* script_state) {
   ScriptPromiseResolver* resolver =
       MakeGarbageCollected<ScriptPromiseResolver>(script_state);
@@ -165,96 +44,6 @@ void FontMetadata::Trace(blink::Visitor* visitor) const {
   ScriptWrappable::Trace(visitor);
 }
 
-// static
-void FontMetadata::getTablesImpl(ScriptPromiseResolver* resolver,
-                                 const String& postscriptName,
-                                 const Vector<String>& tableNames) {
-  if (!resolver->GetScriptState()->ContextIsValid())
-    return;
-
-  FontDescription description;
-  scoped_refptr<SimpleFontData> font_data =
-      FontCache::GetFontCache()->GetFontData(description,
-                                             AtomicString(postscriptName));
-  if (!font_data) {
-    auto message = String::Format("The font %s could not be accessed.",
-                                  postscriptName.Latin1().c_str());
-    ScriptState::Scope scope(resolver->GetScriptState());
-    resolver->Reject(V8ThrowException::CreateTypeError(
-        resolver->GetScriptState()->GetIsolate(), message));
-    return;
-  }
-
-  const SkTypeface* typeface = font_data->PlatformData().Typeface();
-  Vector<SkFontTableTag> tags;
-  if (tableNames.IsEmpty()) {
-    // No tables supplied. Fetch all.
-    const int num_tables = typeface->countTables();
-
-    SkFontTableTag sk_tags[num_tables];
-    const unsigned int ret_val = typeface->getTableTags(sk_tags);
-    if (ret_val == 0) {
-      auto message = String::Format("Error enumerating tables for font %s.",
-                                    postscriptName.Latin1().c_str());
-      ScriptState::Scope scope(resolver->GetScriptState());
-      resolver->Reject(V8ThrowException::CreateTypeError(
-          resolver->GetScriptState()->GetIsolate(), message));
-      return;
-    }
-    tags.Append(sk_tags, num_tables);
-    DCHECK(ret_val == tags.size());
-  } else {
-    // Table names supplied. Validate and convert for retrieval.
-    auto result = convertTableNamesToTags(tableNames);
-    if (!result.first.IsOk()) {
-      ScriptState::Scope scope(resolver->GetScriptState());
-      resolver->Reject(V8ThrowException::CreateTypeError(
-          resolver->GetScriptState()->GetIsolate(), result.first.message));
-      return;
-    }
-    tags = result.second;
-  }
-
-  auto* output = MakeGarbageCollected<HeapHashMap<String, Member<Blob>>>();
-
-  for (auto tag : tags) {
-    wtf_size_t table_size = SafeCast<wtf_size_t>(typeface->getTableSize(tag));
-    String output_table_name = convertTagToString(tag);
-
-    if (!isValidTableName(output_table_name)) {
-      auto message = String::Format("The font %s could not be accessed.",
-                                    postscriptName.Latin1().c_str());
-      ScriptState::Scope scope(resolver->GetScriptState());
-      resolver->Reject(V8ThrowException::CreateTypeError(
-          resolver->GetScriptState()->GetIsolate(), message));
-      return;
-    };
-
-    if (!table_size) {
-      // If table is not found or has size zero, skip it.
-      continue;
-    }
-
-    // TODO(https://crbug.com/1069900): getTableData makes a copy of the bytes.
-    // Stream the font bytes instead.
-    Vector<char> bytes(table_size);
-    size_t returned_size =
-        typeface->getTableData(tag, 0ULL, table_size, bytes.data());
-    DCHECK_EQ(table_size, returned_size);
-
-    scoped_refptr<RawData> raw_data = RawData::Create();
-    bytes.swap(*raw_data->MutableData());
-    auto blob_data = std::make_unique<BlobData>();
-    blob_data->AppendData(std::move(raw_data));
-
-    auto* blob = MakeGarbageCollected<Blob>(
-        BlobDataHandle::Create(std::move(blob_data), table_size));
-    output->Set(output_table_name, blob);
-  }
-
-  auto* map = MakeGarbageCollected<FontTableMap>(*output);
-  resolver->Resolve(map);
-}
 
 // static
 void FontMetadata::blobImpl(ScriptPromiseResolver* resolver,

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

@@ -44,8 +44,6 @@ class BLINK_EXPORT FontMetadata final : public ScriptWrappable {
   String fullName() const { return fullName_; }
   String family() const { return family_; }
 
-  ScriptPromise getTables(ScriptState*);
-  ScriptPromise getTables(ScriptState*, const Vector<String>& tables);
   ScriptPromise blob(ScriptState*);
 
   void Trace(Visitor*) const override;

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

@@ -11,6 +11,5 @@
   readonly attribute USVString postscriptName;
   readonly attribute USVString fullName;
   readonly attribute USVString family;
-  [CallWith=ScriptState, Measure] Promise<FontTableMap> getTables(optional sequence<ByteString> tables);
   [CallWith=ScriptState, Measure] Promise<Blob> blob();
 };

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

-// Copyright 2020 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "third_party/blink/renderer/modules/font_access/font_metadata.h"
-
-#include "build/build_config.h"
-#include "testing/gtest/include/gtest/gtest.h"
-#include "third_party/blink/renderer/bindings/core/v8/script_promise_tester.h"
-#include "third_party/blink/renderer/bindings/core/v8/script_value.h"
-#include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_testing.h"
-#include "third_party/blink/renderer/bindings/modules/v8/v8_font_table_map.h"
-#include "third_party/blink/renderer/modules/font_access/font_table_map.h"
-#include "third_party/blink/renderer/platform/fonts/font_cache.h"
-#include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
-
-namespace blink {
-
-TEST(FontMetadata, TestStrings8And16Bits) {
-  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
-
-  for (auto f : expectations) {
-    // Set up 8 and 16 bit inputs.
-    const std::string tableNameStr = "name";
-
-    Vector<String> tableNames8;
-    const String tName8 = String(tableNameStr.c_str());
-    ASSERT_TRUE(tName8.Is8Bit());
-    tableNames8.push_back(tName8);
-
-    Vector<String> tableNames16;
-    String tName16 = String(tableNameStr.c_str());
-    tName16.Ensure16Bit();
-    ASSERT_FALSE(tName16.Is8Bit());
-    tableNames16.push_back(tName16);
-
-    Vector<Vector<String>> test_inputs;
-    test_inputs.push_back(tableNames8);
-    test_inputs.push_back(tableNames16);
-
-    V8TestingScope scope;
-    auto* script_state = scope.GetScriptState();
-    auto* metadata = FontMetadata::Create(f);
-    for (auto t : test_inputs) {
-      ScriptPromise promise = metadata->getTables(script_state, t);
-      ScriptPromiseTester tester(script_state, promise);
-      tester.WaitUntilSettled();
-      EXPECT_TRUE(tester.IsFulfilled());
-      ASSERT_TRUE(tester.Value().IsObject());
-
-      FontTableMap* tableMap =
-          V8FontTableMap::ToImpl(tester.Value()
-                                     .V8Value()
-                                     ->ToObject(script_state->GetContext())
-                                     .ToLocalChecked());
-      ASSERT_EQ(tableMap->size(), 1u);
-
-      const FontTableMap::MapType map = tableMap->GetHashMap();
-      EXPECT_TRUE(map.Contains("name"));
-    }
-  }
-}
-
-}  // namespace blink

third_party/blink/web_tests/TestExpectations

@@ -1396,10 +1396,7 @@ crbug.com/626703 external/wpt/css/css-text/text-transform/math/text-transform-ma
 crbug.com/626703 external/wpt/css/css-text/text-transform/math/text-transform-math-sans-serif-bold-italic-001.tentative.html [ Failure ]
 crbug.com/626703 external/wpt/css/css-text/text-transform/math/text-transform-math-sans-serif-001.tentative.html [ Failure ]
 crbug.com/1043295 [ Fuchsia ] virtual/font-access/http/tests/font-access/font-access-window-enumeration.html [ Skip ]
-crbug.com/1043295 [ Fuchsia ] virtual/font-access/http/tests/font-access/font-access-window-getTables.html [ Skip ]
-crbug.com/1043295 [ Fuchsia ] virtual/font-access/http/tests/font-access/font-access-window-getTables-large-fonts.html [ Skip ]
 crbug.com/1043295 [ Fuchsia ] virtual/font-access/http/tests/font-access/font-access-window-blob.html [ Skip ]
-crbug.com/1087671 [ Mac ] virtual/font-access/http/tests/font-access/font-access-window-getTables-large-fonts.html [ Skip ]
 
 # ====== Style team owned tests from here ======
 

third_party/blink/web_tests/virtual/font-access/http/tests/font-access/font-access-window-getTables-large-fonts.html

-<!DOCTYPE html>
-<script src="../../../../../resources/testharness.js"></script>
-<script src="../../../../../resources/testharnessreport.js"></script>
-<script src="../../../../../resources/testdriver.js"></script>
-<script src="../../../../../resources/testdriver-vendor.js"></script>
-<script src="resources/test-expectations.js"></script>
-<script src="resources/window-tests-getTables-large-fonts.js"></script>

third_party/blink/web_tests/virtual/font-access/http/tests/font-access/font-access-window-getTables.html

-<!DOCTYPE html>
-<script src="../../../../../resources/testharness.js"></script>
-<script src="../../../../../resources/testharnessreport.js"></script>
-<script src="../../../../../resources/testdriver.js"></script>
-<script src="../../../../../resources/testdriver-vendor.js"></script>
-<script src="resources/test-expectations.js"></script>
-<script src="resources/window-tests-getTables.js"></script>

third_party/blink/web_tests/virtual/font-access/http/tests/font-access/resources/window-tests-getTables-large-fonts.js

-'use strict';
-
-promise_test(async t => {
-  await test_driver.set_permission({name: 'font-access'}, 'granted');
-  const iterator = navigator.fonts.query();
-
-  const expectations = getEnumerationTestSet({labelFilter: [TEST_SIZE_CATEGORY.large]});
-  const expectedFonts = await filterEnumeration(iterator, expectations);
-  const additionalExpectedTables = getMoreExpectedTables(expectations);
-  for (const f of expectedFonts) {
-    const tables = await f.getTables();
-    assert_font_has_tables(f.postscriptName, tables, BASE_TABLES);
-    if (f.postscriptName in additionalExpectedTables) {
-      assert_font_has_tables(f.postscriptName,
-                             tables,
-                             additionalExpectedTables[f.postscriptName]);
-    }
-  }
-}, 'getTables(): large fonts have expected non-empty tables');

third_party/blink/web_tests/virtual/font-access/http/tests/font-access/resources/window-tests-getTables.js

-'use strict';
-
-promise_test(async t => {
-  await test_driver.set_permission({name: 'font-access'}, 'granted');
-  const iterator = navigator.fonts.query();
-
-  const expectations = getEnumerationTestSet({labelFilter: [TEST_SIZE_CATEGORY.small]});
-  const expectedFonts = await filterEnumeration(iterator, expectations);
-  const additionalExpectedTables = getMoreExpectedTables(expectations);
-  for (const f of expectedFonts) {
-    const tables = await f.getTables();
-    assert_font_has_tables(f.postscriptName, tables, BASE_TABLES);
-    if (f.postscriptName in additionalExpectedTables) {
-      assert_font_has_tables(f.postscriptName,
-                             tables,
-                             additionalExpectedTables[f.postscriptName]);
-    }
-  }
-}, 'getTables(): small sized fonts have expected non-empty tables');
-
-promise_test(async t => {
-  await test_driver.set_permission({name: 'font-access'}, 'granted');
-  const iterator = navigator.fonts.query();
-
-  const expectations = getEnumerationTestSet({labelFilter: [TEST_SIZE_CATEGORY.medium]});
-  const expectedFonts = await filterEnumeration(iterator, expectations);
-  const additionalExpectedTables = getMoreExpectedTables(expectations);
-  for (const f of expectedFonts) {
-    const tables = await f.getTables();
-    assert_font_has_tables(f.postscriptName, tables, BASE_TABLES);
-    if (f.postscriptName in additionalExpectedTables) {
-      assert_font_has_tables(f.postscriptName,
-                             tables,
-                             additionalExpectedTables[f.postscriptName]);
-    }
-  }
-}, 'getTables(): medium sized fonts have expected non-empty tables');
-
-promise_test(async t => {
-  await test_driver.set_permission({name: 'font-access'}, 'granted');
-  const iterator = navigator.fonts.query();
-
-  const expectedFonts = await filterEnumeration(iterator,
-                                                getEnumerationTestSet({
-                                                  labelFilter: [TEST_SIZE_CATEGORY.small]}));
-  const inputs = [
-    ['cmap'], // Single item.
-    ['cmap', 'head'], // Multiple items.
-    ['head', 'head'], // Duplicate items.
-  ];
-  for (const f of expectedFonts) {
-    for (const tableQuery of inputs) {
-      const tables = await f.getTables(tableQuery);
-      assert_font_has_tables(f.postscriptName, tables, tableQuery);
-
-      const querySet = new Set(tableQuery);
-      assert_true(tables.size == querySet.size,
-                  `Got ${tables.size} entries, want ${querySet.size}.`);
-    }
-  }
-}, 'getTables([...]) returns tables');
-
-promise_test(async t => {
-  await test_driver.set_permission({name: 'font-access'}, 'granted');
-  const iterator = navigator.fonts.query();
-
-  const expectedFonts = await filterEnumeration(iterator,
-                                                getEnumerationTestSet({
-                                                  labelFilter: [TEST_SIZE_CATEGORY.small]}));
-  const inputs = [
-    // Nonexistent Tag.
-    ['ABCD'],
-
-    // Boundary testing.
-    ['\x20\x20\x20\x20'],
-    ['\x7E\x7E\x7E\x7E'],
-  ];
-  for (const f of expectedFonts) {
-    for (const tableQuery of inputs) {
-      const tables = await f.getTables(tableQuery);
-      assert_true(tables.size == 0, `Got {tables.size} entries, want zero.`);
-    }
-  }
-}, 'getTables([tableName,...]) returns if a table name does not exist');
-
-promise_test(async t => {
-  await test_driver.set_permission({name: 'font-access'}, 'granted');
-  const iterator = navigator.fonts.query();
-
-  const expectedFonts = await filterEnumeration(iterator,
-                                                getEnumerationTestSet({
-                                                  labelFilter: [TEST_SIZE_CATEGORY.small]}));
-  const inputs = [
-    // empty string
-    [''],
-
-    // Invalid length.
-    [' '],
-    ['A'],
-    ['AB'],
-    ['ABC'],
-
-    // Boundary testing.
-    ['\x19\x19\x19\x19'],
-    ['\x7F\x7F\x7F\x7F'],
-
-    // Tag too long.
-    ['cmap is useful'],
-
-    // Not ByteString.
-    ['\u6568\u6461'], // UTF-16LE, encodes to the same first four bytes as "head" in ASCII.
-    ['\u6568\u6461\x00\x00'], // 4-character, 8-byte string.
-    ['\u6C34'], // U+6C34 CJK UNIFIED IDEOGRAPH (water)
-    ['\uD834\uDD1E'], // U+1D11E MUSICAL SYMBOL G-CLEF (UTF-16 surrogate pair)
-    ['\uFFFD'], // U+FFFD REPLACEMENT CHARACTER
-    ['\uD800'], // UTF-16 surrogate lead
-    ['\uDC00'], // UTF-16 surrogate trail
-  ];
-  for (const f of expectedFonts) {
-    for (const tableQuery of inputs) {
-      await promise_rejects_js(t, TypeError, f.getTables(tableQuery));
-    }
-  }
-}, 'getTables([tableName,...]) rejects for invalid input');
-
-promise_test(async t => {
-  await test_driver.set_permission({name: 'font-access'}, 'granted');
-  const iterator = navigator.fonts.query();
-
-  const expectedFonts = await filterEnumeration(iterator,
-                                                getEnumerationTestSet({
-                                                  labelFilter: [TEST_SIZE_CATEGORY.small]}));
-  const inputs = [
-    {
-      // One exists, one doesn't.
-      tableQuery: ['head', 'blah'],
-      expected: ['head'],
-    },
-    {
-      tableQuery: ['blah', 'head'],
-      expected: ['head'],
-    },
-    {
-      tableQuery: ['cmap', 'head', 'blah'],
-      expected: ['head', 'cmap'],
-    },
-    {
-      // Duplicate entry.
-      tableQuery: ['cmap', 'head', 'blah', 'cmap'],
-      expected: ['head', 'cmap'],
-    },
-  ];
-  for (const f of expectedFonts) {
-    for (const input of inputs) {
-      const tables = await f.getTables(input.tableQuery);
-      assert_font_has_tables(f.postscriptName, tables, input.expected);
-    }
-  }
-}, 'getTables([tableName,...]) returns even if a requested table is not found');