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

Issue 2086543002: Move releaseIfNeeded to start, since it can change bitmapIsDummy_ (Closed)

Created:
4 years, 6 months ago by reed1
Modified:
4 years, 6 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

Move releaseIfNeeded to start, since it can change bitmapIsDummy_ In SkiaBitLocker::cgContext() We query the clip, which is empty, so we set clip_bounds to 0,0,1,1 (since CGBitmapContextCreate will fail for an empty width or height) and set bitmapIsDummy_ to true. Then we call releaseIfNeeded(), which can change bitmapIsDummy_. In that case, back in cgContext() we are confused, and then call canvas_->temporary_internal_describeTopLayer() which can reset clip_bounds to empty... and thus CGBitmapContextCreate fails (unexpectedly). The fix is to make the call to releaseIfNeeded() at the start of the function so any (hidden) mutations of fields is done before we start to set-and-use them. BUG=620981 Committed: https://crrev.com/6acf5977b62cb3d5986e0be8c68994287be7569d Cr-Commit-Position: refs/heads/master@{#400700}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -2 lines) Patch
M skia/ext/skia_utils_mac.mm View 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 15 (9 generated)
reed1
4 years, 6 months ago (2016-06-20 16:23:26 UTC) #4
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2086543002/1
4 years, 6 months ago (2016-06-20 16:23:42 UTC) #6
tomhudson
lgtm
4 years, 6 months ago (2016-06-20 16:32:37 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2086543002/1
4 years, 6 months ago (2016-06-20 16:37:26 UTC) #11
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 6 months ago (2016-06-20 17:01:02 UTC) #13
commit-bot: I haz the power
4 years, 6 months ago (2016-06-20 17:02:56 UTC) #15
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/6acf5977b62cb3d5986e0be8c68994287be7569d
Cr-Commit-Position: refs/heads/master@{#400700}

Powered by Google App Engine
This is Rietveld 408576698