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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2649103007: Make PropertyHandle instances const references where possible (Closed)

Created:
3 years, 11 months ago by alancutter (OOO until 2018)
Modified:
3 years, 11 months ago
Reviewers:
Eric Willigers
CC:
darktears, blink-reviews, blink-reviews-animation_chromium.org, blink-reviews-style_chromium.org, chromium-reviews, Eric Willigers, rjwright, shans
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Make PropertyHandle instances const references where possible This change cleans up usage patterns of PropertyHandles to better reflect the fact that it holds ownership over an AtomicString. Previously PropertyHandles used to be the CSSPropertyID enum where it was normal to pass by copy all the time. This change helps avoid Stack-use-after-scope errors when using returned PropertyHandles. BUG=683493 Review-Url: https://codereview.chromium.org/2649103007 Cr-Commit-Position: refs/heads/master@{#445963} Committed: https://chromium.googlesource.com/chromium/src/+/13f2e28123b749889096143c033a79307510a3aa

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+24 lines, -23 lines) Patch
M third_party/WebKit/Source/core/animation/EffectModel.h View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/animation/Interpolation.h View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/animation/InterpolationEffect.h View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/animation/InterpolationEffect.cpp View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/animation/InvalidatableInterpolation.h View 2 chunks +3 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/animation/Keyframe.h View 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/animation/Keyframe.cpp View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/animation/KeyframeEffectModel.h View 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/animation/LegacyStyleInterpolation.h View 3 chunks +4 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/animation/LegacyStyleInterpolation.cpp View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/animation/StringKeyframe.h View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/animation/StringKeyframe.cpp View 1 chunk +2 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/animation/animatable/AnimatableValueKeyframe.h View 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/animation/animatable/AnimatableValueKeyframe.cpp View 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 8 (4 generated)
alancutter (OOO until 2018)
3 years, 11 months ago (2017-01-25 03:47:26 UTC) #2
Eric Willigers
lgtm
3 years, 11 months ago (2017-01-25 04:01:42 UTC) #3
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/2649103007/1
3 years, 11 months ago (2017-01-25 04:30:32 UTC) #5
commit-bot: I haz the power
3 years, 11 months ago (2017-01-25 05:55:23 UTC) #8
Message was sent while issue was closed. 
Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/13f2e28123b749889096143c033a...

Powered by Google App Engine
This is Rietveld 408576698