Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1469353011:
[Oilpan] Replace a raw pointer with RawPtrWillBeWeakPersistent in ScrollAnimatorBase (Closed)
|
Created:
5 years ago by peria Modified:
5 years ago CC:
chromium-reviews, shans, rjwright, blink-reviews-animation_chromium.org, darktears, blink-reviews, Eric Willigers Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionThey are back references (*) in following style.
+-(*)-ProgrammaticScrollAnimtor<=(OwnPtr)=#
v |
ScrollableArea =(OwnPtr)=> ScrollableArea::ScrollAnimators
^ |
+--(*)-- ScrollAnimatorBase <=(OwnPtr)=#
Forward to: https://codereview.chromium.org/1477113003/
BUG=550325
Patch Set 1 #
Messages
Total messages: 17 (9 generated)
Description was changed from ========== Replace a raw pointer with RawPtrWillBeWeakPersistent BUG=550325 ========== to ========== Replace a raw pointer with RawPtrWillBeWeakPersistent in ScrollAnimatorBase. This pointer is a reference(*) in following cyclic reference. ScrollableArea =(OwnPtr)=> ScrollAnimators =(OwnPtr)=> ScrollAnimatorBase -(*)-> ScrollableArea BUG=550325 ==========
Description was changed from ========== Replace a raw pointer with RawPtrWillBeWeakPersistent in ScrollAnimatorBase. This pointer is a reference(*) in following cyclic reference. ScrollableArea =(OwnPtr)=> ScrollAnimators =(OwnPtr)=> ScrollAnimatorBase -(*)-> ScrollableArea BUG=550325 ========== to ========== Replace a raw pointer with RawPtrWillBeWeakPersistent in ScrollAnimatorBase. This pointer is a reference(*) in following reference cycle. ScrollableArea =(OwnPtr)=> ScrollAnimators =(OwnPtr)=> ScrollAnimatorBase -(*)-> ScrollableArea BUG=550325 ==========
Description was changed from ========== Replace a raw pointer with RawPtrWillBeWeakPersistent in ScrollAnimatorBase. This pointer is a reference(*) in following reference cycle. ScrollableArea =(OwnPtr)=> ScrollAnimators =(OwnPtr)=> ScrollAnimatorBase -(*)-> ScrollableArea BUG=550325 ========== to ========== Replace a raw pointer with RawPtrWillBeWeakPersistent in ScrollAnimatorBase. This pointer is a back reference (*) in following style. ScrollableArea =(OwnPtr)=> ScrollableArea::ScrollAnimators =(OwnPtr)=> ScrollAnimatorBase -(*)-> ScrollableArea BUG=550325 ==========
peria@chromium.org changed reviewers: + oilpan-reviews@chromium.org
PTL
LGTM
The CQ bit was checked by peria@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1469353011/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1469353011/1
sigbjornf@opera.com changed reviewers: + sigbjornf@opera.com
Hmm. If something really needs to change, why not work towards moving ScrollAnimatorBase to the heap?
On 2015/11/26 08:43:37, sof wrote: > Hmm. If something really needs to change, why not work towards moving > ScrollAnimatorBase to the heap? If it's doable, it's definitely better :) Worth taking a look.
The CQ bit was unchecked by haraken@chromium.org
On 2015/11/26 08:44:54, haraken wrote: > On 2015/11/26 08:43:37, sof wrote: > > Hmm. If something really needs to change, why not work towards moving > > ScrollAnimatorBase to the heap? > > If it's doable, it's definitely better :) Worth taking a look. Agree. And I find it better to work also for GC_PLUGIN_IGNORE() in ProgrammaticScrollAnimator.
Description was changed from
==========
Replace a raw pointer with RawPtrWillBeWeakPersistent
in ScrollAnimatorBase.
This pointer is a back reference (*) in following style.
ScrollableArea =(OwnPtr)=> ScrollableArea::ScrollAnimators
=(OwnPtr)=> ScrollAnimatorBase -(*)-> ScrollableArea
BUG=550325
==========
to
==========
They are back references (*) in following style.
+-(*)-ProgrammaticScrollAnimtor<=(OwnPtr)=#
v |
ScrollableArea =(OwnPtr)=> ScrollableArea::ScrollAnimators
^ |
+--(*)-- ScrollAnimatorBase <=(OwnPtr)=#
BUG=550325
==========
On 2015/11/26 08:50:49, peria wrote: > On 2015/11/26 08:44:54, haraken wrote: > > On 2015/11/26 08:43:37, sof wrote: > > > Hmm. If something really needs to change, why not work towards moving > > > ScrollAnimatorBase to the heap? > > > > If it's doable, it's definitely better :) Worth taking a look. > > Agree. > And I find it better to work also for GC_PLUGIN_IGNORE() in > ProgrammaticScrollAnimator. I don't think it is straightforward, but preferable state to end up in. WeakPersistent<> is the Oilpan reference type with the largest overhead, I'd argue.
On 2015/11/26 08:54:22, sof wrote: > On 2015/11/26 08:50:49, peria wrote: > > On 2015/11/26 08:44:54, haraken wrote: > > > On 2015/11/26 08:43:37, sof wrote: > > > > Hmm. If something really needs to change, why not work towards moving > > > > ScrollAnimatorBase to the heap? > > > > > > If it's doable, it's definitely better :) Worth taking a look. > > > > Agree. > > And I find it better to work also for GC_PLUGIN_IGNORE() in > > ProgrammaticScrollAnimator. > > I don't think it is straightforward, but preferable state to end up in. > > WeakPersistent<> is the Oilpan reference type with the largest overhead, I'd > argue. Hmm. I try it in another CL, because it will be very different from this CL. https://codereview.chromium.org/1477113003/
Description was changed from
==========
They are back references (*) in following style.
+-(*)-ProgrammaticScrollAnimtor<=(OwnPtr)=#
v |
ScrollableArea =(OwnPtr)=> ScrollableArea::ScrollAnimators
^ |
+--(*)-- ScrollAnimatorBase <=(OwnPtr)=#
BUG=550325
==========
to
==========
They are back references (*) in following style.
+-(*)-ProgrammaticScrollAnimtor<=(OwnPtr)=#
v |
ScrollableArea =(OwnPtr)=> ScrollableArea::ScrollAnimators
^ |
+--(*)-- ScrollAnimatorBase <=(OwnPtr)=#
Forward to: https://codereview.chromium.org/1477113003/
BUG=550325
==========
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
