Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(86)

Issue 7655040: Work around a ridiculous bug in ATS, a deprecated system framework (Closed)

Created:
9 years, 4 months ago by Mark Mentovai
Modified:
9 years, 4 months ago
CC:
chromium-reviews, joi+watch-content_chromium.org, jam
Visibility:
Public.

Description

Work around a ridiculous bug in ATS, a deprecated system framework. ATS likes to write to memory it doesn't own on Mac OS X 10.7 ("Lion"). This happens in SendDeactivateFontsInContainerMessage, called by ATSFontDeactivate, used by Chrome. SendDeactivateFontsInContainerMessage has some really sloppy memory handling that can be avoided by making sure that a certain symbol, __CTFontManagerUnregisterFontForData, is available. Note that the system's CoreText framework has a _CTFontManagerUnregisterFontForData symbol, which is probably what ATS should be looking for, but that's actually an entirely different ridiculous bug in ATS. ATS seems to have this bug on 10.6 ("Snow Leopard") too, but we haven't noticed the corruption there. Maybe we're just lucky. BUG=93191, 90884 TEST=1. Visit http://www.justgiving.nl/ 2. Click the British flag in the top-right 3. Click the blue JustGiving logo in the top-left. 4. Wait for the page to finish loading, then repeat step 3. Expect: to be able to continue performing step 4 repeatedly. No sad tabs. No renderer crashes. No messages logged to the console about memory (malloc) errors Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=97320

Patch Set 1 #

Total comments: 1

Patch Set 2 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+37 lines, -0 lines) Patch
M chrome/app/framework.order View 1 chunk +1 line, -0 lines 0 comments Download
M content/common/font_loader_mac.mm View 1 1 chunk +36 lines, -0 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Mark Mentovai
9 years, 4 months ago (2011-08-18 06:50:02 UTC) #1
jeremy
LGTM http://codereview.chromium.org/7655040/diff/1/chrome/app/framework.order File chrome/app/framework.order (right): http://codereview.chromium.org/7655040/diff/1/chrome/app/framework.order#newcode25 chrome/app/framework.order:25: __CTFontManagerUnregisterFontForData Can you add a comment here pointing ...
9 years, 4 months ago (2011-08-18 07:58:34 UTC) #2
Mark Mentovai
brett or darin or jam for content/common OWNERS review (I wish I could just OWN ...
9 years, 4 months ago (2011-08-18 15:43:31 UTC) #3
brettw
9 years, 4 months ago (2011-08-18 15:48:09 UTC) #4
LGTM, in the future, please only ask one owner for review to reduce spam.

Powered by Google App Engine
This is Rietveld 408576698