Skip to content

Commit d850933

Browse files
[js] Reduce GC pressure in TypeScript getAttribute and isDisplayed atoms (#17582)
* [js] Reduce GC pressure in TypeScript getAttribute and isDisplayed atoms getAttribute: convert BOOLEAN_PROPERTIES from Array to Set for O(1) .has() lookup instead of O(n) .indexOf(), and call element.getAttribute(name) directly at the three call sites that already hold the lowercased name, avoiding a redundant .toLowerCase() in the wrapper. isDisplayed: replace findImageUsingMap's full-DOM getElementsByTagName('*') scan with a single querySelector('[usemap="#..."]') call, using the same CSS attribute-value escaping pattern applied to nameMany in find-elements. * [js] Memoize computed style, client rect, and displayed in isDisplayed atom Add three per-call Maps inside isShownElement, scoped to the single synchronous invocation so there is no stale-data risk and no GC pressure between calls: - computedStyleCache: getEffectiveStyle now fetches getComputedStyle once per element and reuses the CSSStyleDeclaration for subsequent property reads. A typical isShown call queries 5-8 properties on the same element. - clientRectCache: getClientRect caches the Rect after the first getBoundingClientRect (or imageMap rect) computation. Overflow checking and positiveSize both query ancestor rects, so shared containers are only laid out once. - displayedCache: displayed() walks the full ancestor chain on every call. positiveSize iterates all children calling displayedFn on each, so shared ancestors were re-walked N times for N siblings. The cache makes each ancestor node free after the first traversal. * [js] Remove bogus async boolean-property test document.createElement('script').async returns true by default (browser sets the 'force async' flag on script elements created via JS), so the test expecting null was never correct. The absent-boolean-returns-null behaviour is already covered by the existing disabled and readonly tests.
1 parent 62f911a commit d850933

4 files changed

Lines changed: 122 additions & 43 deletions

File tree

javascript/atoms/test/attribute_typescript_test.html

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,18 @@
182182
assertAttributeEquals(assert, 'lovely', byId('cheddar'), 'unknown');
183183
});
184184

185+
QUnit.test('boolean property hidden returns true when attribute present', function (assert) {
186+
var el = document.createElement('div');
187+
el.setAttribute('hidden', '');
188+
assertAttributeEquals(assert, 'true', el, 'hidden');
189+
});
190+
191+
QUnit.test('boolean property multiple returns true when attribute present', function (assert) {
192+
var el = document.createElement('select');
193+
el.setAttribute('multiple', '');
194+
assertAttributeEquals(assert, 'true', el, 'multiple');
195+
});
196+
185197
// --- event handler attributes ---
186198

187199
QUnit.test('event handler attribute falls back to getAttribute, not function stringification', function (assert) {

javascript/atoms/test/shown_typescript_test.html

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,50 @@
124124
assert.notOk(isShown(byId('orphanArea')));
125125
});
126126

127+
QUnit.test('map with hyphen in name is found via querySelector', function (assert) {
128+
var img = document.createElement('img');
129+
img.setAttribute('usemap', '#my-map');
130+
img.setAttribute('width', '100');
131+
img.setAttribute('height', '100');
132+
document.body.appendChild(img);
133+
134+
var map = document.createElement('map');
135+
map.setAttribute('name', 'my-map');
136+
var area = document.createElement('area');
137+
area.setAttribute('shape', 'rect');
138+
area.setAttribute('coords', '0,0,50,50');
139+
map.appendChild(area);
140+
document.body.appendChild(map);
141+
142+
assert.ok(isShown(map), 'map linked to visible image should be shown');
143+
assert.ok(isShown(area), 'area linked to visible image should be shown');
144+
145+
document.body.removeChild(img);
146+
document.body.removeChild(map);
147+
});
148+
149+
QUnit.test('map with double-quote in name is found via querySelector', function (assert) {
150+
var img = document.createElement('img');
151+
img.setAttribute('width', '100');
152+
img.setAttribute('height', '100');
153+
document.body.appendChild(img);
154+
155+
var map = document.createElement('map');
156+
map.setAttribute('name', 'my"map');
157+
img.setAttribute('usemap', '#my"map');
158+
159+
var area = document.createElement('area');
160+
area.setAttribute('shape', 'rect');
161+
area.setAttribute('coords', '0,0,50,50');
162+
map.appendChild(area);
163+
document.body.appendChild(map);
164+
165+
assert.ok(isShown(map), 'map with quote in name linked to visible image should be shown');
166+
167+
document.body.removeChild(img);
168+
document.body.removeChild(map);
169+
});
170+
127171
QUnit.test('element with nested block level element shown', function (assert) {
128172
assert.ok(isShown(byId('containsNestedBlock')));
129173
});

javascript/atoms/typescript/get-attribute.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
'readonly': 'readOnly',
2222
};
2323

24-
const BOOLEAN_PROPERTIES: string[] = [
24+
const BOOLEAN_PROPERTIES: Set<string> = new Set([
2525
'allowfullscreen',
2626
'allowpaymentrequest',
2727
'allowusermedia',
@@ -68,7 +68,7 @@
6868
'truespeed',
6969
'typemustmatch',
7070
'willvalidate',
71-
];
71+
]);
7272

7373
function getAttribute(element: Element, attributeName: string): string | null {
7474
return element.getAttribute(attributeName.toLowerCase());
@@ -160,14 +160,14 @@
160160

161161
const propName = PROPERTY_ALIASES[name] || attribute;
162162

163-
if (BOOLEAN_PROPERTIES.indexOf(name) !== -1) {
164-
const hasAttr = getAttribute(element, attribute) !== null;
163+
if (BOOLEAN_PROPERTIES.has(name)) {
164+
const hasAttr = element.getAttribute(name) !== null;
165165
const propValue = getProperty(element, propName);
166166
return hasAttr || !!propValue ? 'true' : null;
167167
}
168168

169169
if (name === 'value' && isElement(element, 'LI')) {
170-
const attrValue = getAttribute(element, attribute);
170+
const attrValue = element.getAttribute(name);
171171
return attrValue != null ? attrValue : null;
172172
}
173173

@@ -179,7 +179,7 @@
179179
}
180180

181181
if (property == null || isObject(property)) {
182-
const attrValue = getAttribute(element, attribute);
182+
const attrValue = element.getAttribute(name);
183183
return attrValue != null ? attrValue : null;
184184
}
185185

javascript/atoms/typescript/is-displayed.ts

Lines changed: 60 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,13 @@ interface Coordinate {
4545
// Guards against form children that shadow tagName (e.g. <form><input name="tagName">).
4646
var tagNameDescriptor = Object.getOwnPropertyDescriptor(Element.prototype, 'tagName');
4747

48+
// Per-call memoisation caches. Scoped here so they live only for the
49+
// duration of this synchronous call — no stale-data risk, no GC pressure
50+
// between invocations.
51+
var computedStyleCache = new Map<Element, CSSStyleDeclaration | null>();
52+
var clientRectCache = new Map<Element, Rect>();
53+
var displayedCache = new Map<Node, boolean>();
54+
4855
function toUpperCaseTag(tagName?: string): string | undefined {
4956
return tagName ? tagName.toUpperCase() : undefined;
5057
}
@@ -81,11 +88,12 @@ interface Coordinate {
8188
}
8289

8390
function getEffectiveStyle(elem: Element, propertyName: string): string | null {
84-
var win = elem.ownerDocument.defaultView;
85-
if (!win) {
86-
return null;
91+
var computed = computedStyleCache.get(elem);
92+
if (computed === undefined) {
93+
var win = elem.ownerDocument.defaultView;
94+
computed = win ? (win.getComputedStyle(elem) || null) : null;
95+
computedStyleCache.set(elem, computed);
8796
}
88-
var computed = win.getComputedStyle(elem);
8997
if (!computed) {
9098
return null;
9199
}
@@ -124,33 +132,42 @@ interface Coordinate {
124132
}
125133

126134
function getClientRect(elem: Element): Rect {
135+
var cachedRect = clientRectCache.get(elem);
136+
if (cachedRect) {
137+
return cachedRect;
138+
}
139+
140+
var rect: Rect;
127141
var imageMap = maybeFindImageMap(elem);
128142
if (imageMap) {
129-
return imageMap.rect;
130-
}
131-
132-
var elemTagName = typeof (elem as Element).tagName === 'string' ? (elem as Element).tagName : '';
133-
if (elemTagName.toUpperCase() === 'HTML') {
134-
var doc = (elem as Element).ownerDocument;
135-
// In quirks mode (no DOCTYPE), viewport dimensions come from document.body;
136-
// documentElement.clientWidth/Height is unreliable and can be 0.
137-
var sizeElem = doc.compatMode === 'CSS1Compat' ? doc.documentElement : (doc.body || doc.documentElement);
138-
return createRect(0, 0, sizeElem.clientWidth, sizeElem.clientHeight);
139-
}
140-
141-
try {
142-
var nativeRect = (elem as Element).getBoundingClientRect();
143-
return {
144-
left: nativeRect.left,
145-
top: nativeRect.top,
146-
right: nativeRect.right,
147-
bottom: nativeRect.bottom,
148-
width: nativeRect.right - nativeRect.left,
149-
height: nativeRect.bottom - nativeRect.top,
150-
};
151-
} catch (_error) {
152-
return createRect(0, 0, 0, 0);
143+
rect = imageMap.rect;
144+
} else {
145+
var elemTagName = typeof (elem as Element).tagName === 'string' ? (elem as Element).tagName : '';
146+
if (elemTagName.toUpperCase() === 'HTML') {
147+
var doc = (elem as Element).ownerDocument;
148+
// In quirks mode (no DOCTYPE), viewport dimensions come from document.body;
149+
// documentElement.clientWidth/Height is unreliable and can be 0.
150+
var sizeElem = doc.compatMode === 'CSS1Compat' ? doc.documentElement : (doc.body || doc.documentElement);
151+
rect = createRect(0, 0, sizeElem.clientWidth, sizeElem.clientHeight);
152+
} else {
153+
try {
154+
var nativeRect = (elem as Element).getBoundingClientRect();
155+
rect = {
156+
left: nativeRect.left,
157+
top: nativeRect.top,
158+
right: nativeRect.right,
159+
bottom: nativeRect.bottom,
160+
width: nativeRect.right - nativeRect.left,
161+
height: nativeRect.bottom - nativeRect.top,
162+
};
163+
} catch (_error) {
164+
rect = createRect(0, 0, 0, 0);
165+
}
166+
}
153167
}
168+
169+
clientRectCache.set(elem, rect);
170+
return rect;
154171
}
155172

156173
function getAreaRelativeRect(area: HTMLAreaElement): Rect {
@@ -187,14 +204,9 @@ interface Coordinate {
187204
}
188205

189206
function findImageUsingMap(mapName: string, doc: Document): Element | null {
190-
var elements = doc.getElementsByTagName('*');
191-
for (var index = 0; index < elements.length; index += 1) {
192-
var useMap = elements[index].getAttribute('usemap');
193-
if (useMap === '#' + mapName) {
194-
return elements[index];
195-
}
196-
}
197-
return null;
207+
// Use querySelector instead of a full-DOM scan; escape the map name so
208+
// special characters in the CSS attribute value string are handled safely.
209+
return doc.querySelector('[usemap="#' + mapName.replace(/\\/g, '\\\\').replace(/"/g, '\\"') + '"]');
198210
}
199211

200212
function maybeFindImageMap(elem: Element): ImageMapResult | null {
@@ -459,31 +471,42 @@ interface Coordinate {
459471
}
460472

461473
function displayed(node: Node): boolean {
474+
var cached = displayedCache.get(node);
475+
if (cached !== undefined) {
476+
return cached;
477+
}
478+
462479
if (isElement(node)) {
463480
var display = getEffectiveStyle(node, 'display');
464481
var contentVisibility = getEffectiveStyle(node, 'content-visibility');
465482
if (display === 'none' || contentVisibility === 'hidden') {
483+
displayedCache.set(node, false);
466484
return false;
467485
}
468486
}
469487

470488
var parent = getParentNodeInComposedDom(node);
471489
if (typeof ShadowRoot === 'function' && parent instanceof ShadowRoot) {
472490
if (parent.host.shadowRoot && parent.host.shadowRoot !== parent) {
491+
displayedCache.set(node, false);
473492
return false;
474493
}
475494
parent = parent.host;
476495
}
477496

478497
if (parent && (parent.nodeType === Node.DOCUMENT_NODE || parent.nodeType === Node.DOCUMENT_FRAGMENT_NODE)) {
498+
displayedCache.set(node, true);
479499
return true;
480500
}
481501

482502
if (isElement(parent, 'DETAILS') && !(<HTMLDetailsElement>parent).open && !isElement(node, 'SUMMARY')) {
503+
displayedCache.set(node, false);
483504
return false;
484505
}
485506

486-
return !!parent && displayed(parent);
507+
var result = !!parent && displayed(parent);
508+
displayedCache.set(node, result);
509+
return result;
487510
}
488511

489512
return isShownInternal(elem, !!optIgnoreOpacity, displayed);

0 commit comments

Comments
 (0)