>so detaching skins does not have to be part of the lifecycle. I agree with that, that's why I was asking about removing listeners, rather than detaching skins. Is that the same ? IOW, do you mean that explicitly removing listeners from the skin to the component shouldn't be part of the component lifecycle, and all rely on GC ?
> Isn't the solution as simple as using weak reference listeners to the > stagetext events? Yes, it's probably that simple ( I have to check yet). But the events are not set in the skins, they are set in the component (SkinnableTextBase.partAdded / partRemoved). So doing it that way bothers me because the component is not supposed to know about the internals of the skins (pooling , or whatever). So setting weak listeners in the component because we KNOW that the skin is using a pool defeats that principle. But maybe I am too "purist" ;-) WDYT? Maurice -----Message d'origine----- De : Alex Harui [mailto:aha...@adobe.com] Envoyé : mardi 22 avril 2014 18:34 À : dev@flex.apache.org Objet : Re: Question about mobile StageText pool I would think that most components do not use a pool of subcomponents, so detaching skins does not have to be part of the lifecycle. IOW, the component and its skin and its subcomponents are all available for GC at the time the last reference to the component is broken. References from the skin back to the component do not prevent GC in normal situations. This makes me think the pooling changes the requirements and work to resolve this issue should be local to the pooling if possible. Isn't the solution as simple as using weak reference listeners to the stagetext events? Of course, I could be wrongŠ -Alex On 4/22/14 7:00 AM, "Maurice Amsellem" <maurice.amsel...@systar.com> wrote: >> Doing it on removing from stage would not be right in many cases >>since the widget could come back on stage later? > >I don't know the code well either, and I don't have any definite answer >to this question. >So you are probably right to not detach the skin when the widget is >removed from stage. > >On the other hand, the bulk of partAdded/partRemoved various >implementations are about adding / removing event listeners to skin part. > >So if it's not called by default, when do you remove the event >listeners when a widget skin is not in use (ie not on stage)? >This is not consistent, and probably confusing also. > >I maybe be wrong, but I would expect the listeners should be removed >when the widget and skins are removed from stage ? > >WDYT? > >Examples : >1) SkinnableTextBase . partRemoved(partName:String, > instance:Object):void > { > super.partRemoved(partName, instance); > > if (instance == textDisplay) > { > textDisplayRemoved(); > // Stop listening for various events from the IEditableText. > >textDisplay.removeEventListener(SelectionEvent.SELECTION_CHANGE, >textDisplay_selectionChangeHandler); > >textDisplay.removeEventListener(TextOperationEvent.CHANGING, > textDisplay_changingHandler); > textDisplay.removeEventListener(TextOperationEvent.CHANGE, >textDisplay_changeHandler); > textDisplay.removeEventListener(FlexEvent.ENTER, >textDisplay_enterHandler); > textDisplay.removeEventListener(FlexEvent.VALUE_COMMIT, >textDisplay_valueCommitHandler); > } > > if (instance == promptDisplay) > { > var newPromptDisplayProperties:Object = {}; > > if (BitFlagUtil.isSet(uint(promptDisplayProperties), > PROMPT_TEXT_PROPERTY_FLAG)) > { > newPromptDisplayProperties.prompt = > promptDisplay.text; > } > promptDisplayProperties = newPromptDisplayProperties; > } > } > >2) TitleWindow.partRemoved(partName:String, instance:Object):void > { > super.partRemoved(partName, instance); > if (instance == moveArea) > moveArea.removeEventListener(MouseEvent.MOUSE_DOWN, >moveArea_mouseDownHandler); > else if (instance == closeButton) > closeButton.removeEventListener(MouseEvent.CLICK, >closeButton_clickHandler); > } > >-----Message d'origine----- >De : Alex Harui [mailto:aha...@adobe.com] Envoyé : mardi 22 avril 2014 >15:12 À : dev@flex.apache.org Objet : Re: Question about mobile >StageText pool > >Don't know this code that well, but when would you trigger detaching of >the skin? Doing it on removing from stage would not be right in many >cases since the widget could come back on stage later. > >-Alex > >On 4/22/14 4:44 AM, "Maurice Amsellem" <maurice.amsel...@systar.com> >wrote: > >>Digging further into TextInput code, I made some curious findings (at >>least for me): >> >>StageText is not GCed because StyleableStageText sets event listeners >>on changing, change, enter, events etc... when the skin is attached >>(in TextInputBase partAdded). >>2) these event listeners should be removed when the skin is detached >>and partRemoved is called, but *partRemoved is never called*, => this >>explains why TI and TI window are not gced, they are locked by SST >>event listeners, which are themselves locked by the SST pool. >> >>Stepping through the code, it appears that detachSkin & partRemoved >>are called only if mx_internal::skinDestructionPolicy is set to "auto" >>(it's set to "never" by default). >> >>I made a small desktop testing app, and tested it with SDK 4.12 and >>even SDK 4.6 => detachSkin & partRemoved are never called by default. >> >>It seems that this is not a new problem: >>http://stackoverflow.com/questions/8150934/spark-skinnablecomponent-sk >>i >>nde >>structionpolicy >> >>This is rather weird behavior. Is that expected ? >> >>Thoughts? >> >>Maurice >> >>-----Message d'origine----- >>De : Maurice Amsellem [mailto:maurice.amsel...@systar.com] >>Envoyé : mardi 22 avril 2014 11:23 >>À : dev@flex.apache.org >>Objet : Question about mobile StageText pool >> >>Hi team, >> >>I have been working on FLEX-34230 (StageText skins prevents garbage >>collection of component with TextInputs and TextAreas) and it appears >>that GC is prevented by the internal pool of StageText used in >>StageText-based skins (precisely in old StyleableStageText and new >>ScrollableStageText). >> >>Apparently, the purpose of this cache pool is to avoid creating a new >>StageText if one with the same characteristics is already in the pool >>(and returning that one). >> >>Does someone know why this pool has been introduced ? is it to >>improve performance because StageText allocation is slow ? >> >>I mean, maybe if the reason for it is not valid anymore, the easiest >>way to fix that would be simply to remove the pool, and create a new >>StageText everytime time one is needed. >> >> >> >>Maurice >
