[PATCH] ivi-shell: Remove all surface transitions when it is being removed.

[Bryce Harrington]

On Wed, Mar 09, 2016 at 10:35:53AM +0000, Ucan, Emre (ADITG/SW1) wrote:
> Hi,
> 
> My apologies. I wrote it wrong. The API is in ivi-layout-private.h.
> 
> Then, it is good:
> 
> Reviewed-by: Emre Ucan <eucan at de.adit-jv.com>

>From a technical review, looks ok to me:

Reviewed-by: Bryce Harrington <bryce at osg.samsung.com>
 
> Best regards
> 
> Emre Ucan
> Software Group I (ADITG/SW1)
> 
> Tel. +49 5121 49 6937
> 
> > -----Original Message-----
> > From: wayland-devel [mailto:wayland-devel-
> > bounces at lists.freedesktop.org] On Behalf Of Ucan, Emre (ADITG/SW1)
> > Sent: Mittwoch, 9. März 2016 11:23
> > To: mateuszx.potrola at intel.com; wayland-devel at lists.freedesktop.org
> > Subject: RE: [PATCH] ivi-shell: Remove all surface transitions when it is being
> > removed.
> > 
> > Hello Mateusz,
> > 
> > Thank you for your patch. It is good catch.
> > 
> > Why do we need to make this new API public ?
> > 
> > ivi_layout_remove_all_surface_transitions is called only in ivi-layout.c.
> > It should be a static function and not a public API.
> > 
> > 
> > > -----Original Message-----
> > > From: wayland-devel [mailto:wayland-devel-
> > > bounces at lists.freedesktop.org] On Behalf Of mateuszx.potrola at intel.com
> > > Sent: Mittwoch, 9. März 2016 09:57
> > > To: wayland-devel at lists.freedesktop.org
> > > Cc: Mateusz Polrola
> > > Subject: [PATCH] ivi-shell: Remove all surface transitions when it is being
> > > removed.
> > >
> > > From: Mateusz Polrola <mateuszx.potrola at intel.com>
> > >
> > > If surface transitions are not removed when surface is being removed, it
> > can
> > > lead to crash later when transition will finish, as it will try to reference
> > already
> > > freed memory.
> > > This change exposes function that can remove all existing transitions for
> > > given surface and it is being called during surface cleanup.
> > >
> > > Signed-off-by: Mateusz Polrola <mateuszx.potrola at intel.com>
> > > ---
> > >  ivi-shell/ivi-layout-private.h    |  3 +++
> > >  ivi-shell/ivi-layout-transition.c | 16 ++++++++++++++++
> > >  ivi-shell/ivi-layout.c            |  2 ++
> > >  3 files changed, 21 insertions(+)
> > >
> > > diff --git a/ivi-shell/ivi-layout-private.h b/ivi-shell/ivi-layout-private.h index
> > > 3a23ef4..8c847e7 100644
> > > --- a/ivi-shell/ivi-layout-private.h
> > > +++ b/ivi-shell/ivi-layout-private.h
> > > @@ -158,6 +158,9 @@ ivi_layout_transition_fade_layer(struct
> > > ivi_layout_layer *layer,  int32_t  is_surface_transition(struct
> > > ivi_layout_surface *surface);
> > >
> > > +void
> > > +ivi_layout_remove_all_surface_transitions(struct ivi_layout_surface
> > > +*surface);
> > > +
> > >  /**
> > >   * methods of interaction between ivi-shell with ivi-layout
> > >   */
> > > diff --git a/ivi-shell/ivi-layout-transition.c b/ivi-shell/ivi-layout-transition.c
> > > index 3b613a5..ed11999 100644
> > > --- a/ivi-shell/ivi-layout-transition.c
> > > +++ b/ivi-shell/ivi-layout-transition.c
> > > @@ -99,6 +99,22 @@ is_surface_transition(struct ivi_layout_surface
> > > *surface)
> > >  	return 0;
> > >  }
> > >
> > > +void
> > > +ivi_layout_remove_all_surface_transitions(struct ivi_layout_surface
> > > +*surface) {
> > > +	struct ivi_layout *layout = get_instance();
> > > +	struct transition_node *node;
> > > +	struct transition_node *tmp;
> > > +	struct ivi_layout_transition *tran;
> > > +
> > > +	wl_list_for_each_safe(node, tmp, &layout->transitions-
> > > >transition_list, link) {
> > > +		tran = node->transition;
> > > +		if (tran->is_transition_func(tran->private_data, surface)) {
> > > +			layout_transition_destroy(tran);
> > > +		}
> > > +	};
> > > +}
> > > +
> > >  static void
> > >  tick_transition(struct ivi_layout_transition *transition, uint32_t timestamp)
> > {
> > > diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c index
> > c324b5e..61897ef
> > > 100644
> > > --- a/ivi-shell/ivi-layout.c
> > > +++ b/ivi-shell/ivi-layout.c
> > > @@ -241,6 +241,8 @@ ivi_layout_surface_destroy(struct
> > ivi_layout_surface
> > > *ivisurf)
> > >
> > >  	wl_signal_emit(&layout->surface_notification.removed, ivisurf);
> > >
> > > +	ivi_layout_remove_all_surface_transitions(ivisurf);
> > > +
> > >  	remove_configured_listener(ivisurf);
> > >
> > >  	ivi_layout_surface_remove_notification(ivisurf);
> > > --
> > > 2.1.0
> > >
> > > --------------------------------------------------------------
> > > Intel Research and Development Ireland Limited Registered in Ireland
> > > Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
> > > Registered Number: 308263
> > >
> > >
> > > This e-mail and any attachments may contain confidential material for the
> > > sole use of the intended recipient(s). Any review or distribution by others
> > is
> > > strictly prohibited. If you are not the intended recipient, please contact the
> > > sender and delete all copies.
> > >
> > > _______________________________________________
> > > wayland-devel mailing list
> > > wayland-devel at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> > 
> > Best regards
> > 
> > Emre Ucan
> > Software Group I (ADITG/SW1)
> > 
> > Tel. +49 5121 49 6937
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel