javascript/NativeScript
1
0
Fork 0
mirror of https://github.com/NativeScript/NativeScript.git synced 2025-08-16 03:31:45 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix: clear the resolvedPage when entry is being cleared, change the passed View to be a weak reference (#7327)

Browse Source 
* fix: clear the `resolvedPage` when entry is being cleared
fix: change the passed `View` to be a weak reference

* chore: add trace logs when weak ref has been cleared but is continuing to be used
chore: add condition to check if weak ref has not been cleared when it is being used

* chore: refactor the way the `resolvedPage` is cleared

* chore: add backward compatible property to avoid breaking changes

* chore: refactor condition to check if WeakRef is not cleared
chore: add tracelogs

* chore: refactor condition to check if WeakRef is not cleared
chore: add tracelogs

* refactor: weakRef usages

* chore: change the way WeakRef type check is done
This commit is contained in:
Vladimir Amiorkov
2019-06-21 11:48:49 +03:00
committed by Dimitar Topuzov
parent 81e1f54955
commit dfe76213dd
7 changed files with 239 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files

64
tns-core-modules/ui/core/properties/properties.ts
View File

@ -4,6 +4,12 @@ import { ViewBase } from "../view-base";
// Types.
import { WrappedValue, PropertyChangeData } from "../../../data/observable";
import {
write as traceWrite,
categories as traceCategories,
messageType as traceMessageType,
} from "../../../trace";
import { Style } from "../../styling/style";
import { profile } from "../../../profiling";
@ -466,6 +472,13 @@ export class CssProperty<T extends Style, U> implements definitions.CssProperty<
const property = this;
function setLocalValue(this: T, newValue: U | string): void {
const view = this.viewRef.get();
if (!view) {
traceWrite(`${newValue} not set to view because ".viewRef" is cleared`, traceCategories.Style, traceMessageType.warn);
return;
}
const reset = newValue === unsetValue || newValue === "";
let value: U;
if (reset) {
@ -482,7 +495,6 @@ export class CssProperty<T extends Style, U> implements definitions.CssProperty<
const changed: boolean = equalityComparer ? !equalityComparer(oldValue, value) : oldValue !== value;
if (changed) {
const view = this.view;
if (reset) {
delete this[key];
if (valueChanged) {
@ -534,6 +546,13 @@ export class CssProperty<T extends Style, U> implements definitions.CssProperty<
}
function setCssValue(this: T, newValue: U | string): void {
const view = this.viewRef.get();
if (!view) {
traceWrite(`${newValue} not set to view because ".viewRef" is cleared`, traceCategories.Style, traceMessageType.warn);
return;
}
const currentValueSource: number = this[sourceKey] || ValueSource.Default;
// We have localValueSource - NOOP.
@ -557,7 +576,6 @@ export class CssProperty<T extends Style, U> implements definitions.CssProperty<
const changed: boolean = equalityComparer ? !equalityComparer(oldValue, value) : oldValue !== value;
if (changed) {
const view = this.view;
if (reset) {
delete this[key];
if (valueChanged) {
@ -718,6 +736,12 @@ export class CssAnimationProperty<T extends Style, U> implements definitions.Css
enumerable, configurable,
get: getsComputed ? function (this: T) { return this[computedValue]; } : function (this: T) { return this[symbol]; },
set(this: T, boxedValue: U | string) {
const view = this.viewRef.get();
if (!view) {
traceWrite(`${boxedValue} not set to view because ".viewRef" is cleared`, traceCategories.Animation, traceMessageType.warn);
return;
}
const oldValue = this[computedValue];
const oldSource = this[computedSource];
@ -760,7 +784,6 @@ export class CssAnimationProperty<T extends Style, U> implements definitions.Css
valueChanged(this, oldValue, value);
}
const view = this.view;
if (view[setNative] && (computedValueChanged || isSet !== wasSet)) {
if (view._suspendNativeUpdatesCount) {
if (view._suspendedUpdates) {
@ -816,10 +839,16 @@ export class CssAnimationProperty<T extends Style, U> implements definitions.Css
}
public _initDefaultNativeValue(target: T): void {
const view = target.viewRef.get();
if (!view) {
traceWrite(`_initDefaultNativeValue not executed to view because ".viewRef" is cleared`, traceCategories.Animation, traceMessageType.warn);
return;
}
const defaultValueKey = this.defaultValueKey;
if (!(defaultValueKey in target)) {
const view = target.view;
const getDefault = this.getDefault;
target[defaultValueKey] = view[getDefault] ? view[getDefault]() : this.defaultValue;
}
@ -862,6 +891,13 @@ export class InheritedCssProperty<T extends Style, U> extends CssProperty<T, U>
const property = this;
const setFunc = (valueSource: ValueSource) => function (this: T, boxedValue: any): void {
const view = this.viewRef.get();
if (!view) {
traceWrite(`${boxedValue} not set to view's property because ".viewRef" is cleared`, traceCategories.Style, traceMessageType.warn);
return;
}
const reset = boxedValue === unsetValue || boxedValue === "";
const currentValueSource: number = this[sourceKey] || ValueSource.Default;
if (reset) {
@ -876,7 +912,6 @@ export class InheritedCssProperty<T extends Style, U> extends CssProperty<T, U>
}
const oldValue: U = key in this ? this[key] : defaultValue;
const view = this.view;
let value: U;
let unsetNativeValue = false;
if (reset) {
@ -907,7 +942,6 @@ export class InheritedCssProperty<T extends Style, U> extends CssProperty<T, U>
const changed: boolean = equalityComparer ? !equalityComparer(oldValue, value) : oldValue !== value;
if (changed) {
const view = this.view;
if (valueChanged) {
valueChanged(this, oldValue, value);
}
@ -997,7 +1031,14 @@ export class ShorthandProperty<T extends Style, P> implements definitions.Shorth
const converter = options.converter;
function setLocalValue(this: T, value: string | P): void {
this.view._batchUpdate(() => {
const view = this.viewRef.get();
if (!view) {
traceWrite(`setLocalValue not executed to view because ".viewRef" is cleared`, traceCategories.Animation, traceMessageType.warn);
return;
}
view._batchUpdate(() => {
for (let [p, v] of converter(value)) {
this[p.name] = v;
}
@ -1005,7 +1046,14 @@ export class ShorthandProperty<T extends Style, P> implements definitions.Shorth
}
function setCssValue(this: T, value: string): void {
this.view._batchUpdate(() => {
const view = this.viewRef.get();
if (!view) {
traceWrite(`setCssValue not executed to view because ".viewRef" is cleared`, traceCategories.Animation, traceMessageType.warn);
return;
}
view._batchUpdate(() => {
for (let [p, v] of converter(value)) {
this[p.cssName] = v;
}

4
tns-core-modules/ui/core/view-base/view-base.ts
View File

@ -192,7 +192,7 @@ export abstract class ViewBase extends Observable implements ViewBaseDefinition
public _domId: number;
public _context: any;
public _isAddedToNativeVisualTree: boolean;
public _cssState: ssm.CssState = new ssm.CssState(this);
public _cssState: ssm.CssState = new ssm.CssState(new WeakRef(this));
public _styleScope: ssm.StyleScope;
public _suspendedUpdates: { [propertyName: string]: Property<ViewBase, any> | CssProperty<Style, any> | CssAnimationProperty<Style, any> };
public _suspendNativeUpdatesCount: SuspendType;
@ -249,7 +249,7 @@ export abstract class ViewBase extends Observable implements ViewBaseDefinition
constructor() {
super();
this._domId = viewIdCounter++;
this._style = new Style(this);
this._style = new Style(new WeakRef(this));
}
// Used in Angular.

2
tns-core-modules/ui/frame/frame-common.ts
View File

@ -154,6 +154,8 @@ export class FrameBase extends CustomLayoutView implements FrameDefinition {
} else {
page._tearDownUI(true);
}
removed.resolvedPage = null;
}
// Attempts to implement https://github.com/NativeScript/NativeScript/issues/1311

82
tns-core-modules/ui/styling/style-properties.ts
View File

@ -25,6 +25,11 @@ import {
matrixArrayToCssMatrix,
multiplyAffine2d,
} from "../../matrix";
import {
write as traceWrite,
categories as traceCategories,
messageType as traceMessageType,
} from "../../trace";
import * as parser from "../../css/parser";
import { LinearGradient } from "./linear-gradient";
@ -175,7 +180,12 @@ export const zeroLength: Length = { value: 0, unit: "px" };
export const minWidthProperty = new CssProperty<Style, Length>({
name: "minWidth", cssName: "min-width", defaultValue: zeroLength, affectsLayout: isIOS, equalityComparer: Length.equals,
valueChanged: (target, oldValue, newValue) => {
target.view.effectiveMinWidth = Length.toDevicePixels(newValue, 0);
const view = target.viewRef.get();
if (view) {
view.effectiveMinWidth = Length.toDevicePixels(newValue, 0);
} else {
traceWrite(`${newValue} not set to view's property because ".viewRef" is cleared`, traceCategories.Style, traceMessageType.warn);
}
}, valueConverter: Length.parse
});
minWidthProperty.register(Style);
@ -183,7 +193,12 @@ minWidthProperty.register(Style);
export const minHeightProperty = new CssProperty<Style, Length>({
name: "minHeight", cssName: "min-height", defaultValue: zeroLength, affectsLayout: isIOS, equalityComparer: Length.equals,
valueChanged: (target, oldValue, newValue) => {
target.view.effectiveMinHeight = Length.toDevicePixels(newValue, 0);
const view = target.viewRef.get();
if (view) {
view.effectiveMinHeight = Length.toDevicePixels(newValue, 0);
} else {
traceWrite(`${newValue} not set to view's property because ".viewRef" is cleared`, traceCategories.Style, traceMessageType.warn);
}
}, valueConverter: Length.parse
});
minHeightProperty.register(Style);
@ -237,7 +252,12 @@ paddingProperty.register(Style);
export const paddingLeftProperty = new CssProperty<Style, Length>({
name: "paddingLeft", cssName: "padding-left", defaultValue: zeroLength, affectsLayout: isIOS, equalityComparer: Length.equals,
valueChanged: (target, oldValue, newValue) => {
target.view.effectivePaddingLeft = Length.toDevicePixels(newValue, 0);
const view = target.viewRef.get();
if (view) {
view.effectivePaddingLeft = Length.toDevicePixels(newValue, 0);
} else {
traceWrite(`${newValue} not set to view's property because ".viewRef" is cleared`, traceCategories.Style, traceMessageType.warn);
}
}, valueConverter: Length.parse
});
paddingLeftProperty.register(Style);
@ -245,7 +265,12 @@ paddingLeftProperty.register(Style);
export const paddingRightProperty = new CssProperty<Style, Length>({
name: "paddingRight", cssName: "padding-right", defaultValue: zeroLength, affectsLayout: isIOS, equalityComparer: Length.equals,
valueChanged: (target, oldValue, newValue) => {
target.view.effectivePaddingRight = Length.toDevicePixels(newValue, 0);
const view = target.viewRef.get();
if (view) {
view.effectivePaddingRight = Length.toDevicePixels(newValue, 0);
} else {
traceWrite(`${newValue} not set to view's property because ".viewRef" is cleared`, traceCategories.Style, traceMessageType.warn);
}
}, valueConverter: Length.parse
});
paddingRightProperty.register(Style);
@ -253,7 +278,12 @@ paddingRightProperty.register(Style);
export const paddingTopProperty = new CssProperty<Style, Length>({
name: "paddingTop", cssName: "padding-top", defaultValue: zeroLength, affectsLayout: isIOS, equalityComparer: Length.equals,
valueChanged: (target, oldValue, newValue) => {
target.view.effectivePaddingTop = Length.toDevicePixels(newValue, 0);
const view = target.viewRef.get();
if (view) {
view.effectivePaddingTop = Length.toDevicePixels(newValue, 0);
} else {
traceWrite(`${newValue} not set to view's property because ".viewRef" is cleared`, traceCategories.Style, traceMessageType.warn);
}
}, valueConverter: Length.parse
});
paddingTopProperty.register(Style);
@ -261,7 +291,12 @@ paddingTopProperty.register(Style);
export const paddingBottomProperty = new CssProperty<Style, Length>({
name: "paddingBottom", cssName: "padding-bottom", defaultValue: zeroLength, affectsLayout: isIOS, equalityComparer: Length.equals,
valueChanged: (target, oldValue, newValue) => {
target.view.effectivePaddingBottom = Length.toDevicePixels(newValue, 0);
const view = target.viewRef.get();
if (view) {
view.effectivePaddingBottom = Length.toDevicePixels(newValue, 0);
} else {
traceWrite(`${newValue} not set to view's property because ".viewRef" is cleared`, traceCategories.Style, traceMessageType.warn);
}
}, valueConverter: Length.parse
});
paddingBottomProperty.register(Style);
@ -822,7 +857,12 @@ export const borderTopWidthProperty = new CssProperty<Style, Length>({
throw new Error(`border-top-width should be Non-Negative Finite number. Value: ${value}`);
}
target.view.effectiveBorderTopWidth = value;
const view = target.viewRef.get();
if (view) {
view.effectiveBorderTopWidth = value;
} else {
traceWrite(`${newValue} not set to view's property because ".viewRef" is cleared`, traceCategories.Style, traceMessageType.warn);
}
const background = target.backgroundInternal.withBorderTopWidth(value);
target.backgroundInternal = background;
}, valueConverter: Length.parse
@ -837,7 +877,12 @@ export const borderRightWidthProperty = new CssProperty<Style, Length>({
throw new Error(`border-right-width should be Non-Negative Finite number. Value: ${value}`);
}
target.view.effectiveBorderRightWidth = value;
const view = target.viewRef.get();
if (view) {
view.effectiveBorderRightWidth = value;
} else {
traceWrite(`${newValue} not set to view's property because ".viewRef" is cleared`, traceCategories.Style, traceMessageType.warn);
}
const background = target.backgroundInternal.withBorderRightWidth(value);
target.backgroundInternal = background;
}, valueConverter: Length.parse
@ -852,7 +897,12 @@ export const borderBottomWidthProperty = new CssProperty<Style, Length>({
throw new Error(`border-bottom-width should be Non-Negative Finite number. Value: ${value}`);
}
target.view.effectiveBorderBottomWidth = value;
const view = target.viewRef.get();
if (view) {
view.effectiveBorderBottomWidth = value;
} else {
traceWrite(`${newValue} not set to view's property because ".viewRef" is cleared`, traceCategories.Style, traceMessageType.warn);
}
const background = target.backgroundInternal.withBorderBottomWidth(value);
target.backgroundInternal = background;
}, valueConverter: Length.parse
@ -867,7 +917,12 @@ export const borderLeftWidthProperty = new CssProperty<Style, Length>({
throw new Error(`border-left-width should be Non-Negative Finite number. Value: ${value}`);
}
target.view.effectiveBorderLeftWidth = value;
const view = target.viewRef.get();
if (view) {
view.effectiveBorderLeftWidth = value;
} else {
traceWrite(`${newValue} not set to view's property because ".viewRef" is cleared`, traceCategories.Style, traceMessageType.warn);
}
const background = target.backgroundInternal.withBorderLeftWidth(value);
target.backgroundInternal = background;
}, valueConverter: Length.parse
@ -1095,7 +1150,12 @@ export namespace Visibility {
export const visibilityProperty = new CssProperty<Style, Visibility>({
name: "visibility", cssName: "visibility", defaultValue: Visibility.VISIBLE, affectsLayout: isIOS, valueConverter: Visibility.parse, valueChanged: (target, oldValue, newValue) => {
target.view.isCollapsed = (newValue === Visibility.COLLAPSE);
const view = target.viewRef.get();
if (view) {
view.isCollapsed = (newValue === Visibility.COLLAPSE);
} else {
traceWrite(`${newValue} not set to view's property because ".viewRef" is cleared`, traceCategories.Style, traceMessageType.warn);
}
}
});
visibilityProperty.register(Style);

91
tns-core-modules/ui/styling/style-scope.ts
View File

@ -350,7 +350,7 @@ export class CssState {
_matchInvalid: boolean;
_playsKeyframeAnimations: boolean;
constructor(private view: ViewBase) {
constructor(private viewRef: WeakRef<ViewBase>) {
this._onDynamicStateChangeHandler = () => this.updateDynamicState();
}
@ -359,7 +359,8 @@ export class CssState {
* As a result, at some point in time, the selectors matched have to be requerried from the style scope and applied to the view.
*/
public onChange(): void {
if (this.view && this.view.isLoaded) {
const view = this.viewRef.get();
if (view && view.isLoaded) {
this.unsubscribeFromDynamicUpdates();
this.updateMatch();
this.subscribeForDynamicUpdates();
@ -370,7 +371,14 @@ export class CssState {
}
public isSelectorsLatestVersionApplied(): boolean {
return this.view._styleScope._getSelectorsVersion() === this._appliedSelectorsVersion;
const view = this.viewRef.get();
if (!view) {
traceWrite(`isSelectorsLatestVersionApplied returns default value "false" because "this.viewRef" cleared.`, traceCategories.Style, traceMessageType.warn);
return false;
}
return this.viewRef.get()._styleScope._getSelectorsVersion() === this._appliedSelectorsVersion;
}
public onLoaded(): void {
@ -387,20 +395,28 @@ export class CssState {
@profile
private updateMatch() {
if (this.view._styleScope) {
this._appliedSelectorsVersion = this.view._styleScope._getSelectorsVersion();
this._match = this.view._styleScope.matchSelectors(this.view);
const view = this.viewRef.get();
if (view && view._styleScope) {
this._appliedSelectorsVersion = view._styleScope._getSelectorsVersion();
this._match = view._styleScope.matchSelectors(view);
} else {
this._match = CssState.emptyMatch;
}
this._matchInvalid = false;
}
@profile
private updateDynamicState(): void {
const matchingSelectors = this._match.selectors.filter(sel => sel.dynamic ? sel.match(this.view) : true);
const view = this.viewRef.get();
if (!view) {
traceWrite(`updateDynamicState not executed to view because ".viewRef" is cleared`, traceCategories.Style, traceMessageType.warn);
this.view._batchUpdate(() => {
return;
}
const matchingSelectors = this._match.selectors.filter(sel => sel.dynamic ? sel.match(view) : true);
view._batchUpdate(() => {
this.stopKeyframeAnimations();
this.setPropertyValues(matchingSelectors);
this.playKeyframeAnimations(matchingSelectors);
@ -424,7 +440,14 @@ export class CssState {
});
if (this._playsKeyframeAnimations = animations.length > 0) {
animations.map(animation => animation.play(<View>this.view));
const view = this.viewRef.get();
if (!view) {
traceWrite(`KeyframeAnimations cannot play because ".viewRef" is cleared`, traceCategories.Animation, traceMessageType.warn);
return;
}
animations.map(animation => animation.play(<View>view));
Object.freeze(animations);
this._appliedAnimations = animations;
}
@ -440,13 +463,19 @@ export class CssState {
.forEach(animation => animation.cancel());
this._appliedAnimations = CssState.emptyAnimationArray;
this.view.style["keyframe:rotate"] = unsetValue;
this.view.style["keyframe:scaleX"] = unsetValue;
this.view.style["keyframe:scaleY"] = unsetValue;
this.view.style["keyframe:translateX"] = unsetValue;
this.view.style["keyframe:translateY"] = unsetValue;
this.view.style["keyframe:backgroundColor"] = unsetValue;
this.view.style["keyframe:opacity"] = unsetValue;
const view = this.viewRef.get();
if (view) {
view.style["keyframe:rotate"] = unsetValue;
view.style["keyframe:scaleX"] = unsetValue;
view.style["keyframe:scaleY"] = unsetValue;
view.style["keyframe:translateX"] = unsetValue;
view.style["keyframe:translateY"] = unsetValue;
view.style["keyframe:backgroundColor"] = unsetValue;
view.style["keyframe:opacity"] = unsetValue;
} else {
traceWrite(`KeyframeAnimations cannot be stopped because ".viewRef" is cleared`, traceCategories.Animation, traceMessageType.warn);
}
this._playsKeyframeAnimations = false;
}
@ -457,7 +486,14 @@ export class CssState {
* @param matchingSelectors
*/
private setPropertyValues(matchingSelectors: SelectorCore[]): void {
const newPropertyValues = new this.view.style.PropertyBag();
const view = this.viewRef.get();
if (!view) {
traceWrite(`${matchingSelectors} not set to view's property because ".viewRef" is cleared`, traceCategories.Style, traceMessageType.warn);
return;
}
const newPropertyValues = new view.style.PropertyBag();
matchingSelectors.forEach(selector =>
selector.ruleset.declarations.forEach(declaration =>
newPropertyValues[declaration.property] = declaration.value));
@ -466,8 +502,8 @@ export class CssState {
const oldProperties = this._appliedPropertyValues;
for (const key in oldProperties) {
if (!(key in newPropertyValues)) {
if (key in this.view.style) {
this.view.style[`css:${key}`] = unsetValue;
if (key in view.style) {
view.style[`css:${key}`] = unsetValue;
} else {
// TRICKY: How do we unset local value?
}
@ -479,14 +515,14 @@ export class CssState {
}
const value = newPropertyValues[property];
try {
if (property in this.view.style) {
this.view.style[`css:${property}`] = value;
if (property in view.style) {
view.style[`css:${property}`] = value;
} else {
const camelCasedProperty = property.replace(/-([a-z])/g, function (g) { return g[1].toUpperCase(); });
this.view[camelCasedProperty] = value;
view[camelCasedProperty] = value;
}
} catch (e) {
traceWrite(`Failed to apply property [${property}] with value [${value}] to ${this.view}. ${e}`, traceCategories.Error, traceMessageType.error);
traceWrite(`Failed to apply property [${property}] with value [${value}] to ${view}. ${e}`, traceCategories.Error, traceMessageType.error);
}
}
@ -535,7 +571,14 @@ export class CssState {
}
toString(): string {
return `${this.view}._cssState`;
const view = this.viewRef.get();
if (!view) {
traceWrite(`toString() of CssState cannot execute correctly because ".viewRef" is cleared`, traceCategories.Animation, traceMessageType.warn);
return "";
}
return `${view}._cssState`;
}
}
CssState.prototype._appliedChangeMap = CssState.emptyChangeMap;

9
tns-core-modules/ui/styling/style/style.d.ts vendored
View File

@ -139,7 +139,14 @@ export class Style extends Observable {
public statusBarStyle: "light" | "dark";
public androidStatusBarBackground: Color;
constructor(ownerView: ViewBase);
constructor(ownerView: ViewBase | WeakRef<ViewBase>);
public viewRef: WeakRef<ViewBase>;
/**
* @deprecated use `viewRef` instead.
*
* The `ViewBase` object associated with the Style!
*/
public view: ViewBase;
//flexbox layout properties

34
tns-core-modules/ui/styling/style/style.ts
View File

@ -10,16 +10,34 @@ import {
FlexDirection, FlexWrap, JustifyContent, AlignItems, AlignContent,
Order, FlexGrow, FlexShrink, FlexWrapBefore, AlignSelf
} from "../../layouts/flexbox-layout";
import {
write as traceWrite,
categories as traceCategories,
messageType as traceMessageType,
} from "../../../trace";
import { TextAlignment, TextDecoration, TextTransform, WhiteSpace } from "../../text-base";
export class Style extends Observable implements StyleDefinition {
constructor(public view: ViewBase) {
constructor(ownerView: ViewBase | WeakRef<ViewBase>) {
super();
// HACK: Could not find better way for cross platform WeakRef type checking.
if (ownerView.constructor.toString().indexOf("[native code]") !== -1) {
this.viewRef = <WeakRef<ViewBase>>ownerView;
} else {
this.viewRef = new WeakRef(<ViewBase>ownerView);
}
}
toString() {
return `${this.view}.style`;
const view = this.viewRef.get();
if (!view) {
traceWrite(`toString() of Style cannot execute correctly because ".viewRef" is cleared`, traceCategories.Animation, traceMessageType.warn);
return "";
}
return `${view}.style`;
}
public fontInternal: Font;
@ -125,5 +143,15 @@ export class Style extends Observable implements StyleDefinition {
public alignSelf: AlignSelf;
public PropertyBag: { new(): { [property: string]: string }, prototype: { [property: string]: string } };
public viewRef: WeakRef<ViewBase>;
public get view(): ViewBase {
if (this.viewRef) {
return this.viewRef.get();
}
return undefined;
}
}
Style.prototype.PropertyBag = class { [property: string]: string; }