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

Issue 873213003: Revert of Fix for the font files being maped multiple times (Fontconfig). (Closed)

Created:
5 years, 11 months ago by tbarzic
Modified:
5 years, 11 months ago
CC:
chromium-reviews, darin-cc_chromium.org, jam, Mostyn Bramley-Moore
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Fix for the font files being maped multiple times (Fontconfig). (patchset #4 id:60001 of https://codereview.chromium.org/697383002/) BUG=452227 Reason for revert: Suspected for breaking security_OpenFDs on cros (http://crbug.com/452227): 01/26 13:31:23.658 ERROR|security_O:0147| Found unexpected fds in chrome type=renderer: set(['0500 /usr/share/fonts/croscore/Tinos-Regular.ttf', '0500 /usr/share/fonts/noto/NotoSans-Regular.ttc', '0500 /usr/share/fonts/noto/NotoSans-Bold.ttc']) 01/26 13:31:23.658 DEBUG|security_O:0138| Found pid 8280 for chrome 01/26 13:31:23.660 ERROR|security_O:0147| Found unexpected fds in chrome type=renderer: set(['0500 /usr/share/fonts/noto/NotoSans-Regular.ttc']) Original issue's description: > Fix for font files being mmaped multiple times (Fontconfig). > > Before this change, when there were a lot of font fallbacks happening on > the web site, the fallback font was mmaped multiple times, reaching up to > 200 times in the TC url: http://jsfiddle.net/p5pe81vs/, leading to OOMs > and crashes for renderer process. > > This happens after the change introduced in > https://codereview.chromium.org/307243002 > > This CL keeps track of the mmaps for each font ID to avoid any further > unnecessary IPC requests and mmaps for future requests of the same font > ID that would result in new FD. > > BUG=430021 > > NOPRESUBMIT=true > brettw indicates this use of ScopedAllowIO is acceptable. > Previous code eluded IO checks, this change makes the IO use find-able. > > Committed: https://crrev.com/78db5e535ef48c596223fe272572e5679fbb44fd > Cr-Commit-Position: refs/heads/master@{#313102} TBR=behdad@google.com,brettw@chromium.org,bungeman@chromium.org,derat@chromium.org,dominik.rottsches@intel.com,eae@chromium.org,jln@chromium.org,reed@chromium.org,jam@chromium.org,jochen@chromium.org,kolczyk@opera.com NOPRESUBMIT=true NOTREECHECKS=true BUG=430021 Committed: https://crrev.com/2af12ff9d1f88963c3a3ff443002be21c78d74b3 Cr-Commit-Position: refs/heads/master@{#313171}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -79 lines) Patch
M content/common/font_config_ipc_linux.h View 2 chunks +0 lines, -13 lines 0 comments Download
M content/common/font_config_ipc_linux.cc View 4 chunks +10 lines, -66 lines 0 comments Download

Messages

Total messages: 6 (1 generated)
tbarzic
Created Revert of Fix for the font files being maped multiple times (Fontconfig).
5 years, 11 months ago (2015-01-26 22:52:17 UTC) #1
Daniel Erat
lgtm
5 years, 11 months ago (2015-01-26 22:53:40 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/873213003/1
5 years, 11 months ago (2015-01-26 22:54:23 UTC) #4
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 11 months ago (2015-01-26 23:42:50 UTC) #5
commit-bot: I haz the power
5 years, 11 months ago (2015-01-26 23:44:06 UTC) #6
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/2af12ff9d1f88963c3a3ff443002be21c78d74b3
Cr-Commit-Position: refs/heads/master@{#313171}

Powered by Google App Engine
This is Rietveld 408576698