Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(10)

Issue 2699323002: Restrict cross-thread access to gfx::Image and gfx::Font in ResourceBundle (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 months ago by tzik
Modified:
7 months, 4 weeks ago
Reviewers:
sky, Yuki
CC:
chromium-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Restrict cross-thread access to gfx::Image and gfx::Font in ResourceBundle Since gfx::Image and gfx::Font are not thread safe, cached images and fonts in ResourceBundle can be used only on UI thread, regardless of the protection by ResourceBundle::images_and_fonts_lock_. This CL removes the lock and adds DCHECK to limit the access to UI thread. BUG=688072, 468010 Review-Url: https://codereview.chromium.org/2699323002 Cr-Commit-Position: refs/heads/master@{#452407} Committed: https://chromium.googlesource.com/chromium/src/+/645ad78c4d429cda9591f9be1486b911d7076f59

Patch Set 1 #

Patch Set 2 : fix #

Patch Set 3 : cros fix #

Patch Set 4 : ReadIconThreadID #

Patch Set 5 : rebase #

Total comments: 6

Patch Set 6 : rebase #

Patch Set 7 : count -> iterator. comment fix. #

Patch Set 8 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+30 lines, -34 lines) Patch
M ui/base/resource/resource_bundle.h View 1 2 3 4 5 6 4 chunks +4 lines, -4 lines 0 comments Download
M ui/base/resource/resource_bundle.cc View 1 2 3 4 5 6 7 chunks +12 lines, -15 lines 0 comments Download
M ui/base/resource/resource_bundle_ios.mm View 1 2 chunks +6 lines, -7 lines 0 comments Download
M ui/base/resource/resource_bundle_mac.mm View 1 2 3 4 5 6 2 chunks +8 lines, -8 lines 0 comments Download
Commit queue not available (can’t edit this change).

Depends on Patchset:

Messages

Total messages: 41 (35 generated)
tzik
PTAL
8 months ago (2017-02-21 05:12:55 UTC) #23
Yuki
LGTM. https://codereview.chromium.org/2699323002/diff/80001/ui/base/resource/resource_bundle.cc File ui/base/resource/resource_bundle.cc (right): https://codereview.chromium.org/2699323002/diff/80001/ui/base/resource/resource_bundle.cc#newcode777 ui/base/resource/resource_bundle.cc:777: void ResourceBundle::InitDefaultFontList() { You may want to add ...
8 months ago (2017-02-21 06:23:54 UTC) #24
sky
LGTM https://codereview.chromium.org/2699323002/diff/80001/ui/base/resource/resource_bundle_mac.mm File ui/base/resource/resource_bundle_mac.mm (right): https://codereview.chromium.org/2699323002/diff/80001/ui/base/resource/resource_bundle_mac.mm#newcode95 ui/base/resource/resource_bundle_mac.mm:95: if (images_.count(resource_id)) { Use an iterator to avoid ...
8 months ago (2017-02-21 18:01:05 UTC) #27
tzik
https://codereview.chromium.org/2699323002/diff/80001/ui/base/resource/resource_bundle.cc File ui/base/resource/resource_bundle.cc (right): https://codereview.chromium.org/2699323002/diff/80001/ui/base/resource/resource_bundle.cc#newcode777 ui/base/resource/resource_bundle.cc:777: void ResourceBundle::InitDefaultFontList() { On 2017/02/21 06:23:54, Yuki wrote: > ...
7 months, 4 weeks ago (2017-02-22 07:19:21 UTC) #29
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/2699323002/140001
7 months, 4 weeks ago (2017-02-23 04:31:53 UTC) #38
commit-bot: I haz the power
7 months, 4 weeks ago (2017-02-23 05:33:28 UTC) #41
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/645ad78c4d429cda9591f9be1486...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 81bcdb8aa