From 355e150061a0971759ca31b72539e10bf940c099 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Wed, 21 Apr 2021 17:03:02 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A5=20remove=20mutation=20freeze=20log?= =?UTF-8?q?ic?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This logic was introduced to allow RRWeb users to "pause" the recording by using the public method `freezePage`. This method has been removed in PR #727, and it appears that the `freeze` logic is now unneeded: since the "fullsnapshot" logic is synchronous, no DOM mutation can happen or be processed while the document snapshot is taken. --- .../src/domain/rrweb/mutation.spec.ts | 4 +- .../rum-recorder/src/domain/rrweb/mutation.ts | 35 ++++---------- .../rum-recorder/src/domain/rrweb/record.ts | 48 ++++++------------- 3 files changed, 24 insertions(+), 63 deletions(-) diff --git a/packages/rum-recorder/src/domain/rrweb/mutation.spec.ts b/packages/rum-recorder/src/domain/rrweb/mutation.spec.ts index 5dda6e15a6..2e5c98466e 100644 --- a/packages/rum-recorder/src/domain/rrweb/mutation.spec.ts +++ b/packages/rum-recorder/src/domain/rrweb/mutation.spec.ts @@ -69,12 +69,12 @@ describe('MutationObserverWrapper', () => { expect(mutationCallbackSpy).not.toHaveBeenCalled() }) - it('emits buffered mutation records on freeze', () => { + it('emits buffered mutation records on flush', () => { serializeNodeWithId(sandbox, DEFAULT_OPTIONS) MockMutationObserver.storeRecords([createMutationRecord()]) expect(mutationCallbackSpy).toHaveBeenCalledTimes(0) - mutationController.freeze() + mutationController.flush() expect(mutationCallbackSpy).toHaveBeenCalledTimes(1) }) diff --git a/packages/rum-recorder/src/domain/rrweb/mutation.ts b/packages/rum-recorder/src/domain/rrweb/mutation.ts index 420fa40b77..fc8af7bf4f 100644 --- a/packages/rum-recorder/src/domain/rrweb/mutation.ts +++ b/packages/rum-recorder/src/domain/rrweb/mutation.ts @@ -107,33 +107,17 @@ class DoubleLinkedList { const moveKey = (id: number, parentId: number) => `${id}@${parentId}` /** - * Controls how mutations are processed, allowing to temporarily freeze the mutations process. + * Controls how mutations are processed, allowing to flush pending mutations. */ export class MutationController { - private frozen = false - private unfreezeListener?: () => void - private freezeListener?: () => void + private flushListener?: () => void - public freeze() { - this.freezeListener?.() - this.frozen = true + public flush() { + this.flushListener?.() } - public unfreeze() { - this.unfreezeListener?.() - this.frozen = false - } - - public isFrozen() { - return this.frozen - } - - public onFreeze(listener: () => void) { - this.freezeListener = listener - } - - public onUnfreeze(listener: () => void) { - this.unfreezeListener = listener + public onFlush(listener: () => void) { + this.flushListener = listener } } @@ -180,8 +164,7 @@ export class MutationObserverWrapper { childList: true, subtree: true, }) - this.controller.onFreeze(() => this.processMutations(this.observer.takeRecords())) - this.controller.onUnfreeze(this.emit) + this.controller.onFlush(() => this.processMutations(this.observer.takeRecords())) } public stop() { @@ -190,9 +173,7 @@ export class MutationObserverWrapper { private processMutations = (mutations: MutationRecord[]) => { mutations.forEach(this.processMutation) - if (!this.controller.isFrozen()) { - this.emit() - } + this.emit() } private emit = () => { diff --git a/packages/rum-recorder/src/domain/rrweb/record.ts b/packages/rum-recorder/src/domain/rrweb/record.ts index 403df6cce0..c104f6d4bb 100644 --- a/packages/rum-recorder/src/domain/rrweb/record.ts +++ b/packages/rum-recorder/src/domain/rrweb/record.ts @@ -1,13 +1,11 @@ import { runOnReadyState } from '@datadog/browser-core' import { snapshot } from '../rrweb-snapshot' -import { RawRecord, RecordType } from '../../types' +import { RecordType } from '../../types' import { initObservers } from './observer' import { IncrementalSource, ListenerHandler, RecordAPI, RecordOptions } from './types' import { getWindowHeight, getWindowWidth } from './utils' import { MutationController } from './mutation' -let wrappedEmit!: (record: RawRecord, isCheckout?: boolean) => void - export function record(options: RecordOptions = {}): RecordAPI { const { emit } = options // runtime checks for user options @@ -17,25 +15,10 @@ export function record(options: RecordOptions = {}): RecordAPI { const mutationController = new MutationController() - wrappedEmit = (record, isCheckout) => { - if ( - mutationController.isFrozen() && - record.type !== RecordType.FullSnapshot && - !(record.type === RecordType.IncrementalSnapshot && record.data.source === IncrementalSource.Mutation) - ) { - // we've got a user initiated record so first we need to apply - // all DOM changes that have been buffering during paused state - mutationController.unfreeze() - } - - emit(record, isCheckout) - } - const takeFullSnapshot = (isCheckout = false) => { - const wasFrozen = mutationController.isFrozen() - mutationController.freeze() // don't allow any node to be serialized or mutation to be emitted during snapshooting + mutationController.flush() // process any pending mutation before taking a full snapshot - wrappedEmit( + emit( { data: { height: getWindowHeight(), @@ -47,7 +30,7 @@ export function record(options: RecordOptions = {}): RecordAPI { isCheckout ) - wrappedEmit( + emit( { data: { has_focus: document.hasFocus(), @@ -63,7 +46,7 @@ export function record(options: RecordOptions = {}): RecordAPI { return console.warn('Failed to snapshot the document') } - wrappedEmit({ + emit({ data: { node, initialOffset: { @@ -85,9 +68,6 @@ export function record(options: RecordOptions = {}): RecordAPI { }, type: RecordType.FullSnapshot, }) - if (!wasFrozen) { - mutationController.unfreeze() - } } const handlers: ListenerHandler[] = [] @@ -98,7 +78,7 @@ export function record(options: RecordOptions = {}): RecordAPI { initObservers({ mutationController, inputCb: (v) => - wrappedEmit({ + emit({ data: { source: IncrementalSource.Input, ...v, @@ -106,7 +86,7 @@ export function record(options: RecordOptions = {}): RecordAPI { type: RecordType.IncrementalSnapshot, }), mediaInteractionCb: (p) => - wrappedEmit({ + emit({ data: { source: IncrementalSource.MediaInteraction, ...p, @@ -114,7 +94,7 @@ export function record(options: RecordOptions = {}): RecordAPI { type: RecordType.IncrementalSnapshot, }), mouseInteractionCb: (d) => - wrappedEmit({ + emit({ data: { source: IncrementalSource.MouseInteraction, ...d, @@ -122,7 +102,7 @@ export function record(options: RecordOptions = {}): RecordAPI { type: RecordType.IncrementalSnapshot, }), mousemoveCb: (positions, source) => - wrappedEmit({ + emit({ data: { positions, source, @@ -130,7 +110,7 @@ export function record(options: RecordOptions = {}): RecordAPI { type: RecordType.IncrementalSnapshot, }), mutationCb: (m) => - wrappedEmit({ + emit({ data: { source: IncrementalSource.Mutation, ...m, @@ -138,7 +118,7 @@ export function record(options: RecordOptions = {}): RecordAPI { type: RecordType.IncrementalSnapshot, }), scrollCb: (p) => - wrappedEmit({ + emit({ data: { source: IncrementalSource.Scroll, ...p, @@ -146,7 +126,7 @@ export function record(options: RecordOptions = {}): RecordAPI { type: RecordType.IncrementalSnapshot, }), styleSheetRuleCb: (r) => - wrappedEmit({ + emit({ data: { source: IncrementalSource.StyleSheetRule, ...r, @@ -154,7 +134,7 @@ export function record(options: RecordOptions = {}): RecordAPI { type: RecordType.IncrementalSnapshot, }), viewportResizeCb: (d) => - wrappedEmit({ + emit({ data: { source: IncrementalSource.ViewportResize, ...d, @@ -162,7 +142,7 @@ export function record(options: RecordOptions = {}): RecordAPI { type: RecordType.IncrementalSnapshot, }), focusCb: (data) => - wrappedEmit({ + emit({ type: RecordType.Focus, data, }),