Created attachment 86685 [details] fast/dynamic/text-combine.html in ToT Chromium The following tests are failing on Chromium on Snow Leopard due to vertical text not being rendered in the right place: fast/blockflow/border-vertical-lr.html fast/blockflow/broken-ideograph-small-caps.html fast/blockflow/broken-ideographic-font.html fast/blockflow/japanese-lr-selection.html fast/blockflow/japanese-lr-text.html fast/blockflow/japanese-rl-selection.html fast/blockflow/japanese-rl-text-with-broken-font.html fast/blockflow/japanese-rl-text.html fast/blockflow/japanese-ruby-vertical-lr.html fast/blockflow/japanese-ruby-vertical-rl.html fast/blockflow/Kusa-Makura-background-canvas.html fast/blockflow/vertical-align-table-baseline.html fast/blockflow/vertical-baseline-alignment.html fast/blockflow/vertical-font-fallback.html fast/dynamic/text-combine.html fast/repaint/japanese-rl-selection-clear.html fast/repaint/japanese-rl-selection-repaint.html fast/repaint/repaint-across-writing-mode-boundary.html fast/ruby/base-shorter-than-text.html fast/text/emphasis-combined-text.html fast/text/justify-ideograph-vertical.html Expected image: http://svn.webkit.org/repository/webkit/trunk/LayoutTests/platform/mac/fast/dynamic/text-combine-expected.png Actual image: http://build.chromium.org/f/chromium/layout_test_results/Webkit_Mac10_6/results/layout-test-results/fast/dynamic/text-combine-actual.png Looking at the actual result in ToT Chromium (see attachment), the vertical text is getting rendered, just in the wrong position. However, we do have the correct glyph positions when computing layout, shown by find-in-page highlighting the right area.
Based on http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Frepaint%2Frepaint-across-writing-mode-boundary.html&showExpectations=true&showAllRuns=true&group=%40ToT%20-%20chromium.org it looks like this may have started to happen after http://trac.webkit.org/changeset/80582/
It looks like this is caused by this block in FontMac.mm: http://trac.webkit.org/changeset/80740/trunk/Source/WebCore/platform/graphics/mac/FontMac.mm Unlike Safari (which builds separate binaries for Leopard and Snow Leopard), Chromium always builds with the 10.5 SDK (and on Leopard?) so we don't end up on the SL path through that code, because BUILDING_ON_SNOW_LEOPARD is not true for us. At least for Chromium this will have to be a run-time check. Besides ATSUI vs. CoreText (http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/mac/ComplexTextController.cpp#L258), is there any other precedent for Chromium making a run-time switch where the Mac port does it at build time?
I don't think it's reasonable to create a runtime configuration every time we use BUILDING_ON_SNOW_LEOPARD/BUILDING_ON_LEOPARD. This bug also means that we don't correctly render elliptical gradients on Snow Leopard. Chromium really just needs to create separate binaries for Snow Leopard and Leopard. I don't know the history here to know why we share a binary though.
Chromium won’t be creating different binaries for different OS releases.
(In reply to comment #4) > Chromium won’t be creating different binaries for different OS releases. In that case, for doing these checks at runtime, is using Gestalt with gestaltSystemVersionMajor/Minor still the way to go?
If WebKit already has a function to get the OS version, it’s probably better to call that. If you’re concerned about bloating the object code size for Apple-style WebKit, which is built for a specific OS release, you can use macros to hide your tests. In some cases, you can use this in conjunction with compiler optimizations to keep things clean and workable. For example: #if BUILD_TIME_CHECK_FOR_LEOPARD bool useNewThing = false; #elif BUILD_TIME_CHECK_FOR_SNOW_LEOPARD bool useNewThing = true; #else bool useNewThing = RuntimeCheckForSnowLeopard(); #endif if (useNewThing) { // ... } Depending on the macros available, it may be easier to write this as #if CHROMIUM bool useNewThing = RuntimeCheckForSnowLeopard(); #elif BUILD_TIME_CHECK_FOR_LEOPARD bool useNewThing = false; // ...
Created attachment 86855 [details] Patch
With this patch the tests mentioned in the initial report will start to pass. Some of them will need rebaselines (for both mac and and chromium) since the results are now shifted by one pixel (presumably a side effect of r80582, the mac/ baselines haven't been updated since then). Rebaselines are not in the uploaded patch since it's too big for Bugzilla.
Comment on attachment 86855 [details] Patch r=me
Committed r81921: <http://trac.webkit.org/changeset/81921>