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

Issue 1278103002: Oilpan: tidy up some inter-stack object references. (Closed)

Created:
5 years, 4 months ago by sof
Modified:
5 years, 4 months ago
CC:
blink-reviews, blink-reviews-style_chromium.org, blink-reviews-css, dglazkov+blink, apavlov+blink_chromium.org, darktears, rwlbuis
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Oilpan: tidy up some inter-stack object references. Add uses of STACK_ALLOCATED() so as to allow these objects to safely keep references to other stack allocated objects. For other heap allocated objects that keep references to GCed objects, convert these into appropriate Member<> refs. R=haraken, philipj@opera.com BUG=509911 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200242

Patch Set 1 #

Total comments: 4

Patch Set 2 : use STACK_ALLOCATED() more often #

Unified diffs Side-by-side diffs Delta from patch set Stats (+44 lines, -31 lines) Patch
M Source/core/css/parser/CSSParserObserverWrapper.h View 1 1 chunk +3 lines, -2 lines 0 comments Download
M Source/core/css/parser/CSSSupportsParser.h View 1 1 chunk +3 lines, -0 lines 0 comments Download
M Source/core/css/resolver/FontBuilder.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/css/resolver/FontBuilder.cpp View 6 chunks +9 lines, -9 lines 0 comments Download
M Source/core/css/resolver/SharedStyleFinder.h View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/css/resolver/SharedStyleFinder.cpp View 4 chunks +10 lines, -10 lines 0 comments Download
M Source/core/css/resolver/StyleResolverParentScope.h View 3 chunks +6 lines, -6 lines 0 comments Download
M Source/modules/crypto/CryptoKey.cpp View 1 1 chunk +1 line, -0 lines 0 comments Download
M Source/modules/webaudio/AudioParam.h View 1 2 chunks +2 lines, -1 line 0 comments Download
M public/platform/WebCryptoKeyAlgorithmParams.h View 1 1 chunk +7 lines, -0 lines 0 comments Download

Messages

Total messages: 14 (4 generated)
sof
please take a look. It would be possible to split these into per-class CLs, if ...
5 years, 4 months ago (2015-08-07 07:57:40 UTC) #2
haraken
https://codereview.chromium.org/1278103002/diff/1/Source/core/css/parser/CSSParserObserverWrapper.h File Source/core/css/parser/CSSParserObserverWrapper.h (right): https://codereview.chromium.org/1278103002/diff/1/Source/core/css/parser/CSSParserObserverWrapper.h#newcode42 Source/core/css/parser/CSSParserObserverWrapper.h:42: CSSParserObserver& m_observer; Shall we change this to a RawPtrWillBeMember? ...
5 years, 4 months ago (2015-08-07 08:17:56 UTC) #4
sof
https://codereview.chromium.org/1278103002/diff/1/Source/core/css/parser/CSSParserObserverWrapper.h File Source/core/css/parser/CSSParserObserverWrapper.h (right): https://codereview.chromium.org/1278103002/diff/1/Source/core/css/parser/CSSParserObserverWrapper.h#newcode42 Source/core/css/parser/CSSParserObserverWrapper.h:42: CSSParserObserver& m_observer; On 2015/08/07 08:17:56, haraken wrote: > > ...
5 years, 4 months ago (2015-08-07 08:20:20 UTC) #5
haraken
LGTM https://codereview.chromium.org/1278103002/diff/1/Source/core/css/parser/CSSParserObserverWrapper.h File Source/core/css/parser/CSSParserObserverWrapper.h (right): https://codereview.chromium.org/1278103002/diff/1/Source/core/css/parser/CSSParserObserverWrapper.h#newcode42 Source/core/css/parser/CSSParserObserverWrapper.h:42: CSSParserObserver& m_observer; On 2015/08/07 08:20:20, sof wrote: > ...
5 years, 4 months ago (2015-08-07 08:27:38 UTC) #6
sof
Updated + found another problematic reference in webaudio/ The use STACK_ALLOCATED() in a public/platform/ decl ...
5 years, 4 months ago (2015-08-07 09:11:07 UTC) #7
haraken
LGTM
5 years, 4 months ago (2015-08-07 13:08:38 UTC) #8
sof
philipj@, jochen@: could you have a look at public/platform/ change? tia.
5 years, 4 months ago (2015-08-07 13:16:47 UTC) #10
philipj_slow
On 2015/08/07 09:11:07, sof wrote: > The use STACK_ALLOCATED() in a public/platform/ decl is acceptable ...
5 years, 4 months ago (2015-08-10 09:56:05 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1278103002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1278103002/20001
5 years, 4 months ago (2015-08-10 10:33:49 UTC) #13
commit-bot: I haz the power
5 years, 4 months ago (2015-08-10 11:48:17 UTC) #14
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=200242

Powered by Google App Engine
This is Rietveld 408576698