From d1959f141a777397bd5eeb5d40bcfe7195538557 Mon Sep 17 00:00:00 2001 From: bobzel Date: Mon, 12 Oct 2020 10:23:29 -0400 Subject: fixed computedFn()'s to be used correctly in several places. fixed major memory leak in PDFs --- src/client/util/SelectionManager.ts | 8 ++++---- src/client/util/SnappingManager.ts | 5 ++--- src/client/views/nodes/PDFBox.tsx | 31 ++++++++++++++++++------------- src/fields/Doc.ts | 19 ++++++++----------- src/fields/util.ts | 20 ++++++++++---------- 5 files changed, 42 insertions(+), 41 deletions(-) (limited to 'src') diff --git a/src/client/util/SelectionManager.ts b/src/client/util/SelectionManager.ts index 008ce281c..34e88c7b0 100644 --- a/src/client/util/SelectionManager.ts +++ b/src/client/util/SelectionManager.ts @@ -2,7 +2,6 @@ import { observable, action, runInAction, ObservableMap } from "mobx"; import { Doc, Opt } from "../../fields/Doc"; import { DocumentView } from "../views/nodes/DocumentView"; import { computedFn } from "mobx-utils"; -import { List } from "../../fields/List"; import { CollectionSchemaView } from "../views/collections/CollectionSchemaView"; import { CollectionViewType } from "../views/collections/CollectionView"; @@ -67,15 +66,16 @@ export namespace SelectionManager { manager.SelectSchemaDoc(colSchema, document); } + const IsSelectedCache = computedFn(function isSelected(doc: DocumentView) { // wraapping get() in a computedFn only generates mobx() invalidations when the return value of the function for the specific get parameters has changed + return manager.SelectedDocuments.get(doc) ? true : false; + }); // computed functions, such as used in IsSelected generate errors if they're called outside of a // reaction context. Specifying the context with 'outsideReaction' allows an efficiency feature // to avoid unnecessary mobx invalidations when running inside a reaction. export function IsSelected(doc: DocumentView | undefined, outsideReaction?: boolean): boolean { return !doc ? false : outsideReaction ? manager.SelectedDocuments.get(doc) ? true : false : // get() accesses a hashtable -- setting anything in the hashtable generates a mobx invalidation for every get() - computedFn(function isSelected(doc: DocumentView) { // wraapping get() in a computedFn only generates mobx() invalidations when the return value of the function for the specific get parameters has changed - return manager.SelectedDocuments.get(doc) ? true : false; - })(doc); + IsSelectedCache(doc); } export function DeselectAll(except?: Doc): void { diff --git a/src/client/util/SnappingManager.ts b/src/client/util/SnappingManager.ts index a615f0247..069f81d38 100644 --- a/src/client/util/SnappingManager.ts +++ b/src/client/util/SnappingManager.ts @@ -32,9 +32,8 @@ export namespace SnappingManager { /// bcz; argh!! TODO; These do not belong here, but there were include order problems with leaving them in util.ts // need to investigate further what caused the mobx update problems and move to a better location. + const getCachedGroupByNameCache = computedFn(function (name: string) { return manager.cachedGroups.includes(name); }, true); + export function GetCachedGroupByName(name: string) { return getCachedGroupByNameCache(name); } export function SetCachedGroups(groups: string[]) { manager.setCachedGroups(groups); } - export function GetCachedGroupByName(name: string) { - return computedFn(function (name: string) { return manager.cachedGroups.includes(name); }, true)(name); - } } diff --git a/src/client/views/nodes/PDFBox.tsx b/src/client/views/nodes/PDFBox.tsx index 74b431bea..79e00eed7 100644 --- a/src/client/views/nodes/PDFBox.tsx +++ b/src/client/views/nodes/PDFBox.tsx @@ -35,7 +35,7 @@ export class PDFBox extends ViewBoxAnnotatableComponent(); private _keyRef = React.createRef(); @@ -44,7 +44,6 @@ export class PDFBox extends ViewBoxAnnotatableComponent; @observable private _pageControls = false; @@ -54,6 +53,11 @@ export class PDFBox extends ViewBoxAnnotatableComponent this._pdf = PDFBox.pdfcache.get(pdfUrl.url.href)); + else if (PDFBox.pdfpromise.get(pdfUrl.url.href)) PDFBox.pdfpromise.get(pdfUrl.url.href)?.then(action(pdf => this._pdf = pdf)); + } const backup = "oldPath"; const { Document } = this.props; @@ -265,22 +269,23 @@ export class PDFBox extends ViewBoxAnnotatableComponent; } - _pdfjsRequested = false; + static pdfcache = new Map(); + static pdfpromise = new Map>(); render() { TraceMobx(); - const pdfUrl = Cast(this.dataDoc[this.props.fieldKey], PdfField, null); if (true) {//this.props.isSelected() || (this.props.active() && this.props.renderDepth === 0) || this.props.Document._scrollY !== undefined) { - this._everActive = true; + this._displayPdfLive = true; } - if (pdfUrl && this._everActive) { - if (pdfUrl instanceof PdfField && this._pdf) { - return this.renderPdfView; - } - if (!this._pdfjsRequested) { - this._pdfjsRequested = true; - const promise = Pdfjs.getDocument(pdfUrl.url.href).promise; - promise.then(action(pdf => this._pdf = pdf)); + if (this._displayPdfLive) { + if (this._pdf) return this.renderPdfView; + const href = Cast(this.dataDoc[this.props.fieldKey], PdfField, null)?.url.href; + if (href) { + if (PDFBox.pdfcache.get(href)) setTimeout(action(() => this._pdf = PDFBox.pdfcache.get(href))); + else { + if (!PDFBox.pdfpromise.get(href)) PDFBox.pdfpromise.set(href, Pdfjs.getDocument(href).promise); + PDFBox.pdfpromise.get(href)?.then(action(pdf => PDFBox.pdfcache.set(href, this._pdf = pdf))); + } } } return this.renderTitleBox; diff --git a/src/fields/Doc.ts b/src/fields/Doc.ts index 086b7777f..2452ab408 100644 --- a/src/fields/Doc.ts +++ b/src/fields/Doc.ts @@ -891,12 +891,11 @@ export namespace Doc { export function GetSelectedTool(): InkTool { return StrCast(Doc.UserDoc().activeInkTool, InkTool.None) as InkTool; } export function SetUserDoc(doc: Doc) { return (manager._user_doc = doc); } - export function IsSearchMatch(doc: Doc) { - return computedFn(function IsSearchMatch(doc: Doc) { - return brushManager.SearchMatchDoc.has(doc) ? brushManager.SearchMatchDoc.get(doc) : - brushManager.SearchMatchDoc.has(Doc.GetProto(doc)) ? brushManager.SearchMatchDoc.get(Doc.GetProto(doc)) : undefined; - })(doc); - } + const isSearchMatchCache = computedFn(function IsSearchMatch(doc: Doc) { + return brushManager.SearchMatchDoc.has(doc) ? brushManager.SearchMatchDoc.get(doc) : + brushManager.SearchMatchDoc.has(Doc.GetProto(doc)) ? brushManager.SearchMatchDoc.get(Doc.GetProto(doc)) : undefined; + }); + export function IsSearchMatch(doc: Doc) { return isSearchMatchCache(doc); } export function IsSearchMatchUnmemoized(doc: Doc) { return brushManager.SearchMatchDoc.has(doc) ? brushManager.SearchMatchDoc.get(doc) : brushManager.SearchMatchDoc.has(Doc.GetProto(doc)) ? brushManager.SearchMatchDoc.get(Doc.GetProto(doc)) : undefined; @@ -918,11 +917,9 @@ export namespace Doc { brushManager.SearchMatchDoc.clear(); } - export function IsBrushed(doc: Doc) { - return computedFn(function IsBrushed(doc: Doc) { - return brushManager.BrushedDoc.has(doc) || brushManager.BrushedDoc.has(Doc.GetProto(doc)); - })(doc); - } + const isBrushedCache = computedFn(function IsBrushed(doc: Doc) { return brushManager.BrushedDoc.has(doc) || brushManager.BrushedDoc.has(Doc.GetProto(doc)); }); + export function IsBrushed(doc: Doc) { return isBrushedCache(doc); } + // don't bother memoizing (caching) the result if called from a non-reactive context. (plus this avoids a warning message) export function IsBrushedDegreeUnmemoized(doc: Doc) { if (!doc || GetEffectiveAcl(doc) === AclPrivate || GetEffectiveAcl(Doc.GetProto(doc)) === AclPrivate) return 0; diff --git a/src/fields/util.ts b/src/fields/util.ts index b68d961b1..5cd8df564 100644 --- a/src/fields/util.ts +++ b/src/fields/util.ts @@ -154,30 +154,30 @@ export enum SharingPermissions { None = "Not Shared" } +// return acl from cache or cache the acl and return. +const getEffectiveAclCache = computedFn(function (target: any, playgroundProp: boolean, user?: string) { return getEffectiveAcl(target, playgroundProp, user); }, true); + /** * Calculates the effective access right to a document for the current user. */ export function GetEffectiveAcl(target: any, in_prop?: string | symbol | number, user?: string): symbol { - return computedFn(function (target: any, in_prop?: string | symbol | number, user?: string) { - return getEffectiveAcl(target, in_prop, user); - }, true)(target, in_prop, user); -} -function getEffectiveAcl(target: any, in_prop?: string | symbol | number, user?: string): symbol { if (!target) return AclPrivate; + if (in_prop === UpdatingFromServer) return AclAdmin; // requesting the UpdatingFromServer prop must always go through to keep the local DB consistent + const playgroundProp = in_prop && DocServer.PlaygroundFields?.includes(in_prop.toString()) ? true : false; + return getEffectiveAclCache(target, playgroundProp, user); +} - // all changes received fromt the server must be processed as Admin - if (in_prop === UpdatingFromServer || target[UpdatingFromServer]) return AclAdmin; - +function getEffectiveAcl(target: any, playgroundProp: boolean, user?: string): symbol { + if (target[UpdatingFromServer]) return AclAdmin; // all changes received from the server must be processed as Admin // if the current user is the author of the document / the current user is a member of the admin group const userChecked = user || Doc.CurrentUserEmail; if (userChecked === (target.__fields?.author || target.author)) return AclAdmin; if (SnappingManager.GetCachedGroupByName("Admin")) return AclAdmin; - if (target[AclSym] && Object.keys(target[AclSym]).length) { // if the acl is being overriden or the property being modified is one of the playground fields (which can be freely modified) - if (_overrideAcl || (in_prop && DocServer.PlaygroundFields?.includes(in_prop.toString()))) return AclEdit; + if (_overrideAcl || playgroundProp) return AclEdit; let effectiveAcl = AclPrivate; const HierarchyMapping = new Map([ -- cgit v1.2.3-70-g09d2