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

Issue 1321913002: Modified platform bitmap to support GDI being unavailable (Closed)

Created:
5 years, 3 months ago by forshaw
Modified:
5 years, 2 months ago
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Modifed platform bitmap to support win32k lockdown. This patch detects if the current process is under win32k lockdown and instead of using the GDI route it maps the shared section directly. This is done for supporting the introduction of win32k lockdown for PPAPI processes. To minimize the chances of disruption when not under win32k lockdown the code path is kept the same. BUG=523278 Committed: https://crrev.com/e201ebab608f5ff0a12b3e1de96585bcafb28517 Cr-Commit-Position: refs/heads/master@{#350828}

Patch Set 1 #

Total comments: 9

Patch Set 2 : Minor stylistic changes for review. #

Total comments: 1

Patch Set 3 : Made policy check static and added comment. #

Total comments: 1

Patch Set 4 : Used common Win32k check. #

Patch Set 5 : Fixes from review. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+35 lines, -12 lines) Patch
M skia/ext/bitmap_platform_device_win.cc View 1 2 3 4 4 chunks +35 lines, -12 lines 0 comments Download

Messages

Total messages: 29 (9 generated)
forshaw
Please take a look.
5 years, 3 months ago (2015-09-03 00:04:36 UTC) #2
Will Harris
+scottmg knows this code. I had a feeling there was a non DC based platform ...
5 years, 3 months ago (2015-09-04 02:25:50 UTC) #4
scottmg
Sorry, I'm not familiar with a non-DC device. Maybe you were thinking of +ananta (I ...
5 years, 3 months ago (2015-09-04 02:46:20 UTC) #6
forshaw
On 2015/09/04 02:25:50, Will Harris wrote: > +scottmg knows this code. I had a feeling ...
5 years, 3 months ago (2015-09-04 09:28:55 UTC) #7
forshaw
PTAL. https://codereview.chromium.org/1321913002/diff/1/skia/ext/bitmap_platform_device_win.cc File skia/ext/bitmap_platform_device_win.cc (right): https://codereview.chromium.org/1321913002/diff/1/skia/ext/bitmap_platform_device_win.cc#newcode22 skia/ext/bitmap_platform_device_win.cc:22: GetProcessMitigationPolicyType GetProcessMitigationPolicyFunc = On 2015/09/04 02:46:19, scottmg wrote: ...
5 years, 3 months ago (2015-09-04 09:58:05 UTC) #8
Will Harris
I think reed should look at this, as he's familiar with this code. https://codereview.chromium.org/1321913002/diff/20001/skia/ext/bitmap_platform_device_win.cc File ...
5 years, 3 months ago (2015-09-04 16:26:33 UTC) #10
reed1
This is pretty far above my head. I don't understand (certainly not from reading the ...
5 years, 3 months ago (2015-09-04 17:23:00 UTC) #12
forshaw
On 2015/09/04 17:23:00, reed1 wrote: > This is pretty far above my head. I don't ...
5 years, 3 months ago (2015-09-07 11:26:34 UTC) #13
forshaw
Updated the code, PTAL.
5 years, 3 months ago (2015-09-07 14:31:36 UTC) #14
Will Harris
https://codereview.chromium.org/1321913002/diff/40001/skia/ext/bitmap_platform_device_win.cc File skia/ext/bitmap_platform_device_win.cc (right): https://codereview.chromium.org/1321913002/diff/40001/skia/ext/bitmap_platform_device_win.cc#newcode69 skia/ext/bitmap_platform_device_win.cc:69: static base::LazyInstance<LazyIsWin32kLockdownEnabled> win32k_lockdown; I think this needs a LAZY_INSTANCE_INITIALIZER
5 years, 3 months ago (2015-09-10 17:34:45 UTC) #15
forshaw
PTAL, hopefully all fixed now.
5 years, 3 months ago (2015-09-12 00:04:16 UTC) #16
Will Harris
The Windows code lgtm, so we should land this if the skia people are happy ...
5 years, 3 months ago (2015-09-14 22:59:24 UTC) #17
Will Harris
ananta do you have views on this CL? otherwise, if we can get skia owners ...
5 years, 3 months ago (2015-09-15 19:57:28 UTC) #18
palmer
LGTM, FWIW.
5 years, 3 months ago (2015-09-15 20:49:02 UTC) #19
forshaw
reed@ PTAL. I've changed the patch slightly to make it more explicit what the code ...
5 years, 2 months ago (2015-09-25 11:49:44 UTC) #22
reed1
lgtm
5 years, 2 months ago (2015-09-25 13:24:32 UTC) #23
forshaw
On 2015/09/25 13:24:32, reed1 wrote: > lgtm Thanks very much for your review. It's very ...
5 years, 2 months ago (2015-09-25 13:26:18 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1321913002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1321913002/120001
5 years, 2 months ago (2015-09-25 14:40:59 UTC) #27
commit-bot: I haz the power
Committed patchset #5 (id:120001)
5 years, 2 months ago (2015-09-25 15:09:34 UTC) #28
commit-bot: I haz the power
5 years, 2 months ago (2015-09-25 15:10:58 UTC) #29
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/e201ebab608f5ff0a12b3e1de96585bcafb28517
Cr-Commit-Position: refs/heads/master@{#350828}

Powered by Google App Engine
This is Rietveld 408576698