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

Issue 2701183002: Avoid touching ResourceBundle fonts from non-UI thread (Closed)

Created:
3 years, 10 months ago by tzik
Modified:
3 years, 10 months ago
Reviewers:
Dan Beam
CC:
chromium-reviews, jam, darin-cc_chromium.org, sky, Robert Sesek, Yuki
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Avoid touching ResourceBundle fonts from non-UI thread SharedResourcesDataSource touches shared fonts in ResourceBundle from IO thread. However, gfx::Font is not thread safe due to non-thread-safe ref counts in gfx::PlatformFont and gfx::FontListImpl. That causes a data race on the ref count. So, we should touch fonts in ResourceBundle only from UI thread. BUG=688072, 468010 Review-Url: https://codereview.chromium.org/2701183002 Cr-Commit-Position: refs/heads/master@{#452390} Committed: https://chromium.googlesource.com/chromium/src/+/bd171b13a4e04434cf2ee4d3b79ce28f0aeddd0f

Patch Set 1 #

Patch Set 2 : rebase #

Patch Set 3 : rebase #

Patch Set 4 : rebase (swapped CL sequence order) #

Total comments: 2

Patch Set 5 : PostTaskAndReply -> TaskRunnerForRequestPath #

Total comments: 6

Patch Set 6 : . #

Total comments: 4

Patch Set 7 : s/IDR/Idr/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -3 lines) Patch
M content/browser/webui/shared_resources_data_source.cc View 1 2 3 4 5 6 4 chunks +16 lines, -3 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 44 (33 generated)
tzik
PTAL
3 years, 10 months ago (2017-02-20 08:21:37 UTC) #8
tzik
ping. dbeam: WDYT?
3 years, 10 months ago (2017-02-22 21:05:19 UTC) #25
Dan Beam
https://codereview.chromium.org/2701183002/diff/80001/content/browser/webui/shared_resources_data_source.cc File content/browser/webui/shared_resources_data_source.cc (right): https://codereview.chromium.org/2701183002/diff/80001/content/browser/webui/shared_resources_data_source.cc#newcode158 content/browser/webui/shared_resources_data_source.cc:158: return nullptr; i think you should only return nullptr ...
3 years, 10 months ago (2017-02-22 22:56:10 UTC) #26
tzik
https://codereview.chromium.org/2701183002/diff/80001/content/browser/webui/shared_resources_data_source.cc File content/browser/webui/shared_resources_data_source.cc (right): https://codereview.chromium.org/2701183002/diff/80001/content/browser/webui/shared_resources_data_source.cc#newcode158 content/browser/webui/shared_resources_data_source.cc:158: return nullptr; On 2017/02/22 22:56:10, Dan Beam wrote: > ...
3 years, 10 months ago (2017-02-22 23:47:02 UTC) #29
Dan Beam
https://codereview.chromium.org/2701183002/diff/100001/content/browser/webui/shared_resources_data_source.cc File content/browser/webui/shared_resources_data_source.cc (right): https://codereview.chromium.org/2701183002/diff/100001/content/browser/webui/shared_resources_data_source.cc#newcode100 content/browser/webui/shared_resources_data_source.cc:100: bytes = base::RefCountedString::TakeString(&css); these are the calls that actually ...
3 years, 10 months ago (2017-02-23 00:27:56 UTC) #30
Dan Beam
https://codereview.chromium.org/2701183002/diff/100001/content/browser/webui/shared_resources_data_source.cc File content/browser/webui/shared_resources_data_source.cc (right): https://codereview.chromium.org/2701183002/diff/100001/content/browser/webui/shared_resources_data_source.cc#newcode155 content/browser/webui/shared_resources_data_source.cc:155: return BrowserThread::GetTaskRunnerForThread(BrowserThread::UI); On 2017/02/23 00:27:56, Dan Beam wrote: > ...
3 years, 10 months ago (2017-02-23 00:32:26 UTC) #31
tzik
https://codereview.chromium.org/2701183002/diff/100001/content/browser/webui/shared_resources_data_source.cc File content/browser/webui/shared_resources_data_source.cc (right): https://codereview.chromium.org/2701183002/diff/100001/content/browser/webui/shared_resources_data_source.cc#newcode155 content/browser/webui/shared_resources_data_source.cc:155: return BrowserThread::GetTaskRunnerForThread(BrowserThread::UI); On 2017/02/23 00:32:26, Dan Beam wrote: > ...
3 years, 10 months ago (2017-02-23 00:58:19 UTC) #33
Dan Beam
lgtm https://codereview.chromium.org/2701183002/diff/100001/content/browser/webui/shared_resources_data_source.cc File content/browser/webui/shared_resources_data_source.cc (right): https://codereview.chromium.org/2701183002/diff/100001/content/browser/webui/shared_resources_data_source.cc#newcode155 content/browser/webui/shared_resources_data_source.cc:155: return BrowserThread::GetTaskRunnerForThread(BrowserThread::UI); On 2017/02/23 00:58:19, tzik wrote: > ...
3 years, 10 months ago (2017-02-23 02:11:30 UTC) #35
tzik
https://codereview.chromium.org/2701183002/diff/120001/content/browser/webui/shared_resources_data_source.cc File content/browser/webui/shared_resources_data_source.cc (right): https://codereview.chromium.org/2701183002/diff/120001/content/browser/webui/shared_resources_data_source.cc#newcode73 content/browser/webui/shared_resources_data_source.cc:73: int GetIDRForPath(const std::string& path) { On 2017/02/23 02:11:30, Dan ...
3 years, 10 months ago (2017-02-23 03:31:40 UTC) #38
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2701183002/140001
3 years, 10 months ago (2017-02-23 03:32:23 UTC) #41
commit-bot: I haz the power
3 years, 10 months ago (2017-02-23 04:24:44 UTC) #44
Message was sent while issue was closed.
Committed patchset #7 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/bd171b13a4e04434cf2ee4d3b79c...

Powered by Google App Engine
This is Rietveld 408576698