140015 – Remove GlyphPageTree

Bug 140015 - Remove GlyphPageTree

Summary: Remove GlyphPageTree

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	Layout and Rendering (show other bugs)
Version:	528+ (Nightly build)
Hardware:	Unspecified Unspecified

Importance:	P2 Normal
Assignee:	Nobody

URL:
Keywords:

Depends on:	140123
Blocks:
	Show dependency tree / graph

Reported:	2014-12-31 09:02 PST by Antti Koivisto
Modified:	2015-01-06 05:40 PST (History)
CC List:	13 users (show)

See Also:

Attachments
wip (31.86 KB, patch) 2014-12-31 09:09 PST, Antti Koivisto	no flags	Details \| Formatted Diff \| Diff
wip2 (33.54 KB, patch) 2014-12-31 09:31 PST, Antti Koivisto	buildbot: commit-queue-	Details \| Formatted Diff \| Diff
Archive of layout-test-results from ews106 for mac-mountainlion-wk2 (1.04 MB, application/zip) 2014-12-31 10:29 PST, Build Bot	no flags	Details
Archive of layout-test-results from ews102 for mac-mountainlion (1.06 MB, application/zip) 2014-12-31 10:29 PST, Build Bot	no flags	Details
wip3 (80.13 KB, patch) 2015-01-02 07:53 PST, Antti Koivisto	no flags	Details \| Formatted Diff \| Diff
wip4 (99.81 KB, patch) 2015-01-02 08:07 PST, Antti Koivisto	buildbot: commit-queue-	Details \| Formatted Diff \| Diff
Archive of layout-test-results from ews103 for mac-mountainlion (331.78 KB, application/zip) 2015-01-02 08:58 PST, Build Bot	no flags	Details
Archive of layout-test-results from ews107 for mac-mountainlion-wk2 (343.07 KB, application/zip) 2015-01-02 09:00 PST, Build Bot	no flags	Details
patch (103.09 KB, patch) 2015-01-02 09:11 PST, Antti Koivisto	buildbot: commit-queue-	Details \| Formatted Diff \| Diff
Archive of layout-test-results from ews101 for mac-mountainlion (787.02 KB, application/zip) 2015-01-02 10:18 PST, Build Bot	no flags	Details
patch (106.75 KB, patch) 2015-01-02 10:20 PST, Antti Koivisto	no flags	Details \| Formatted Diff \| Diff
patch (113.84 KB, patch) 2015-01-02 11:11 PST, Antti Koivisto	darin: review+ buildbot: commit-queue-	Details \| Formatted Diff \| Diff
Archive of layout-test-results from ews104 for mac-mountainlion-wk2 (526.40 KB, application/zip) 2015-01-02 12:15 PST, Build Bot	no flags	Details
patch (115.64 KB, patch) 2015-01-03 04:37 PST, Antti Koivisto	buildbot: commit-queue-	Details \| Formatted Diff \| Diff
Archive of layout-test-results from ews102 for mac-mountainlion (1.26 MB, application/zip) 2015-01-03 05:41 PST, Build Bot	no flags	Details
Archive of layout-test-results from ews107 for mac-mountainlion-wk2 (1.05 MB, application/zip) 2015-01-03 05:41 PST, Build Bot	no flags	Details
another (117.09 KB, patch) 2015-01-04 07:57 PST, Antti Koivisto	no flags	Details \| Formatted Diff \| Diff
Show Obsolete (16) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Antti Koivisto 2014-12-31 09:02:06 PST

This arcane data structure maps characters to glyphs taking font fallbacks into account. We can replace it with a simpler and more efficient caching scheme.

Comment 1 Antti Koivisto 2014-12-31 09:09:37 PST

Created attachment 243844 [details]
wip

Comment 2 WebKit Commit Bot 2014-12-31 09:12:13 PST

Attachment 243844 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:196:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:255:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 2 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 3 Antti Koivisto 2014-12-31 09:31:20 PST

Created attachment 243846 [details]
wip2

Comment 4 WebKit Commit Bot 2014-12-31 09:32:14 PST

Attachment 243846 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:196:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:255:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 2 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 5 Build Bot 2014-12-31 10:28:57 PST

Comment on attachment 243846 [details]
wip2

Attachment 243846 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5704538879164416

New failing tests:
fast/css/font-face-unicode-range.html
editing/deleting/5144139-2.html
http/tests/webfont/fallback-font-while-loading.html
fast/css/font-face-multiple-ranges-for-unicode-range.html
svg/custom/svg-fonts-segmented.xhtml
fast/css/font-face-implicit-local-font.html
transforms/2d/hindi-rotated.html
fast/css/font-face-download-error.html
fast/css/font-face-locally-installed.html
svg/custom/glyph-selection-non-bmp.svg
svg/custom/glyph-selection-bidi-mirror.svg
svg/custom/glyph-selection-arabic-forms.svg

Comment 6 Build Bot 2014-12-31 10:29:01 PST

Created attachment 243848 [details]
Archive of layout-test-results from ews106 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

Comment 7 Build Bot 2014-12-31 10:29:07 PST

Comment on attachment 243846 [details]
wip2

Attachment 243846 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5125486556479488

New failing tests:
fast/css/font-face-unicode-range.html
editing/deleting/5144139-2.html
http/tests/webfont/fallback-font-while-loading.html
fast/css/font-face-multiple-ranges-for-unicode-range.html
svg/custom/svg-fonts-segmented.xhtml
fast/css/font-face-implicit-local-font.html
transforms/2d/hindi-rotated.html
fast/css/font-face-download-error.html
fast/css/font-face-locally-installed.html
svg/custom/glyph-selection-non-bmp.svg
svg/custom/glyph-selection-bidi-mirror.svg
svg/custom/glyph-selection-arabic-forms.svg

Comment 8 Build Bot 2014-12-31 10:29:11 PST

Created attachment 243849 [details]
Archive of layout-test-results from ews102 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

Comment 9 Myles C. Maxfield 2014-12-31 21:46:22 PST

Woah.

Comment 10 Antti Koivisto 2015-01-02 07:53:55 PST

Created attachment 243889 [details]
wip3

Comment 11 WebKit Commit Bot 2015-01-02 07:55:11 PST

Attachment 243889 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:196:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:255:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 2 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 12 Antti Koivisto 2015-01-02 08:07:40 PST

Created attachment 243890 [details]
wip4

Comment 13 WebKit Commit Bot 2015-01-02 08:10:29 PST

Attachment 243890 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/mac/GlyphPageMac.cpp:84:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:196:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:255:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 3 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 14 Build Bot 2015-01-02 08:58:29 PST

Comment on attachment 243890 [details]
wip4

Attachment 243890 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5070704785489920

Number of test failures exceeded the failure limit.

Comment 15 Build Bot 2015-01-02 08:58:34 PST

Created attachment 243892 [details]
Archive of layout-test-results from ews103 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

Comment 16 Build Bot 2015-01-02 09:00:35 PST

Comment on attachment 243890 [details]
wip4

Attachment 243890 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6574334583767040

Number of test failures exceeded the failure limit.

Comment 17 Build Bot 2015-01-02 09:00:47 PST

Created attachment 243893 [details]
Archive of layout-test-results from ews107 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

Comment 18 Antti Koivisto 2015-01-02 09:11:12 PST

Created attachment 243894 [details]
patch

Comment 19 WebKit Commit Bot 2015-01-02 09:21:11 PST

Attachment 243894 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/mac/GlyphPageMac.cpp:84:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:196:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:255:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 3 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 20 Build Bot 2015-01-02 10:18:11 PST

Comment on attachment 243894 [details]
patch

Attachment 243894 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6376431214919680

New failing tests:
http/tests/webfont/fallback-font-while-loading.html

Comment 21 Build Bot 2015-01-02 10:18:16 PST

Created attachment 243898 [details]
Archive of layout-test-results from ews101 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

Comment 22 Antti Koivisto 2015-01-02 10:20:40 PST

Created attachment 243900 [details]
patch

Comment 23 WebKit Commit Bot 2015-01-02 10:22:01 PST

Attachment 243900 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/mac/GlyphPageMac.cpp:84:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:198:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:260:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebKit2/WebProcess/WebProcess.cpp:73:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 4 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 24 Antti Koivisto 2015-01-02 11:11:55 PST

Created attachment 243903 [details]
patch

Comment 25 WebKit Commit Bot 2015-01-02 11:14:22 PST

Attachment 243903 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:198:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:260:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 2 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 26 Build Bot 2015-01-02 12:15:19 PST

Comment on attachment 243903 [details]
patch

Attachment 243903 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6568718511374336

New failing tests:
mathml/opentype/munderover-layout-resize.html

Comment 27 Build Bot 2015-01-02 12:15:26 PST

Created attachment 243907 [details]
Archive of layout-test-results from ews104 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

Comment 28 Darin Adler 2015-01-02 14:04:36 PST

Comment on attachment 243903 [details]
patch

Any concern about the test failures?

Comment 29 Antti Koivisto 2015-01-02 14:39:36 PST

Yeah, looks related. Can't repro locally easily. I'll figure it out before landing.

Exception Type:  EXC_BAD_ACCESS (SIGSEGV)
Exception Codes: KERN_INVALID_ADDRESS at 0x00000000000004c8

CRASHING TEST: mathml/opentype/munderover-layout-resize.html

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebCore             	0x000000010c33d358 WebCore::RenderMathMLOperator::advanceForGlyph(WebCore::GlyphData const&) const + 24 (SimpleFontData.h:165)
1   com.apple.WebCore             	0x000000010c33c92e WebCore::RenderMathMLOperator::updateStyle() + 446 (RenderMathMLOperator.h:68)
2   com.apple.WebCore             	0x000000010c33ea87 WebCore::RenderMathMLOperator::rebuildTokenContent(WTF::String const&) + 343 (RenderObject.h:943)
3   com.apple.WebCore             	0x000000010c33b88f WebCore::RenderMathMLOperator::RenderMathMLOperator(WebCore::MathMLElement&, WTF::Ref<WebCore::RenderStyle>&&) + 207 (RefPtr.h:59)
4   com.apple.WebCore             	0x000000010c1afba5 WebCore::MathMLTextElement::createElementRenderer(WTF::Ref<WebCore::RenderStyle>&&) + 133 (RenderPtr.h:158)

Comment 30 Antti Koivisto 2015-01-02 15:15:56 PST

That's a clean null despite the high offset.

Comment 31 Antti Koivisto 2015-01-03 04:37:57 PST

Created attachment 243914 [details]
patch

Comment 32 WebKit Commit Bot 2015-01-03 04:39:52 PST

Attachment 243914 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:203:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:262:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 2 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 33 Build Bot 2015-01-03 05:41:29 PST

Comment on attachment 243914 [details]
patch

Attachment 243914 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5378610990940160

New failing tests:
fast/css/font-face-implicit-local-font.html
fast/css/font-face-weight-matching.html
fast/css/font-face-multiple-ranges-for-unicode-range.html
fast/css/font-face-multiple-faces.html
fast/css/font-face-unicode-range.html
fast/css/font-face-in-media-rule.html
fast/css/font-face-default-font.html
svg/custom/font-face-cascade-order.svg
fast/css/font-face-download-error.html
fast/css/font-face-locally-installed.html

Comment 34 Build Bot 2015-01-03 05:41:34 PST

Created attachment 243915 [details]
Archive of layout-test-results from ews102 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

Comment 35 Build Bot 2015-01-03 05:41:40 PST

Comment on attachment 243914 [details]
patch

Attachment 243914 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6236357164793856

New failing tests:
fast/css/font-face-implicit-local-font.html
fast/css/font-face-multiple-faces.html
fast/css/font-face-multiple-ranges-for-unicode-range.html
fast/css/font-face-weight-matching.html
fast/css/font-face-unicode-range.html
fast/css/font-face-in-media-rule.html
fast/css/font-face-default-font.html
svg/custom/font-face-cascade-order.svg
fast/css/font-face-download-error.html
fast/css/font-face-locally-installed.html

Comment 36 Build Bot 2015-01-03 05:41:45 PST

Created attachment 243916 [details]
Archive of layout-test-results from ews107 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

Comment 37 Antti Koivisto 2015-01-04 07:57:36 PST

Created attachment 243929 [details]
another

Comment 38 WebKit Commit Bot 2015-01-04 07:59:13 PST

Attachment 243929 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:203:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WebCore/platform/graphics/SimpleFontData.cpp:263:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 2 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 39 Antti Koivisto 2015-01-04 13:26:48 PST

https://trac.webkit.org/r177876

Comment 40 Antti Koivisto 2015-01-04 15:39:07 PST

And https://trac.webkit.org/r177878

Comment 41 Antti Koivisto 2015-01-04 15:41:14 PST

Also http://trac.webkit.org/changeset/177877

Comment 42 Alexey Proskuryakov 2015-01-04 16:19:25 PST

This broke another test on bots: fonts/font-fallback-prefers-pictographs.html

WE are actually getting the wrong glyph, see https://build.webkit.org/results/Apple%20MountainLion%20Release%20WK2%20(Tests)/r177878%20(23244)/fonts/font-fallback-prefers-pictographs-diffs.html

Comment 43 Alexey Proskuryakov 2015-01-04 16:21:18 PST

And by "this", I mean the latest follow-up, r177878.

Comment 44 Antti Koivisto 2015-01-04 16:47:11 PST

https://trac.webkit.org/r177881 should help.