[go: nahoru, domu]

Deduplicate named tasks within a single render frame

This could replace ThrottledWidget in many cases.

Bug:  1448011
Change-Id: Ied8c18f2162e793626c33fb950b2fe9662a9781a
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4559870
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Auto-Submit: Danil Somsikov <dsv@chromium.org>
Commit-Queue: Danil Somsikov <dsv@chromium.org>
diff --git a/front_end/panels/elements/AccessibilityTreeUtils.ts b/front_end/panels/elements/AccessibilityTreeUtils.ts
index 10d7320..8f6ed9e 100644
--- a/front_end/panels/elements/AccessibilityTreeUtils.ts
+++ b/front_end/panels/elements/AccessibilityTreeUtils.ts
@@ -118,7 +118,8 @@
   const role = sdkNode.role()?.value || '';
   const properties = sdkNode.properties() || [];
   const ignored = sdkNode.ignored();
-  return LitHtml.html`<${tag} .data=${{name, role, ignored, properties} as Data}></${tag}>`;
+  const id = getNodeId(sdkNode);
+  return LitHtml.html`<${tag} .data=${{name, role, ignored, properties, id} as Data}></${tag}>`;
 }
 
 export function getNodeId(node: SDK.AccessibilityModel.AccessibilityNode): string {
diff --git a/front_end/panels/elements/components/AccessibilityTreeNode.ts b/front_end/panels/elements/components/AccessibilityTreeNode.ts
index fda0999..57f0927 100644
--- a/front_end/panels/elements/components/AccessibilityTreeNode.ts
+++ b/front_end/panels/elements/components/AccessibilityTreeNode.ts
@@ -49,6 +49,7 @@
   name: string;
   role: string;
   properties: Protocol.Accessibility.AXProperty[];
+  id: string;
 }
 
 export class AccessibilityTreeNode extends HTMLElement {
@@ -59,12 +60,14 @@
   #name = '';
   #role = '';
   #properties: Protocol.Accessibility.AXProperty[] = [];
+  #id = '';
 
   set data(data: AccessibilityTreeNodeData) {
     this.#ignored = data.ignored;
     this.#name = data.name;
     this.#role = data.role;
     this.#properties = data.properties;
+    this.#id = data.id;
     void this.#render();
   }
 
@@ -82,14 +85,15 @@
             LitHtml.nothing);
     const content = this.#ignored ? LitHtml.html`<span>${i18nString(UIStrings.ignored)}</span>` :
                                     LitHtml.html`${role}&nbsp;${name}${properties}`;
-    await Coordinator.RenderCoordinator.RenderCoordinator.instance().write('Accessibility node render', () => {
-      // clang-format off
+    await Coordinator.RenderCoordinator.RenderCoordinator.instance().write(
+        `Accessibility node ${this.#id} render`, () => {
+          // clang-format off
       LitHtml.render(
         LitHtml.html`<div class='container'>${content}</div>`,
         this.#shadow,
         {host: this});
-      // clang-format on
-    });
+          // clang-format on
+        });
   }
 }
 
diff --git a/front_end/ui/components/render_coordinator/RenderCoordinator.ts b/front_end/ui/components/render_coordinator/RenderCoordinator.ts
index afba64b..104a664 100644
--- a/front_end/ui/components/render_coordinator/RenderCoordinator.ts
+++ b/front_end/ui/components/render_coordinator/RenderCoordinator.ts
@@ -167,6 +167,7 @@
   }
 
   #enqueueHandler<T = unknown>(callback: CoordinatorCallback, action: ACTION, label: string): Promise<T> {
+    const hasName = ![UNNAMED_READ, UNNAMED_WRITE, UNNAMED_SCROLL].includes(label);
     label = `${action === ACTION.READ ? '[Read]' : '[Write]'}: ${label}`;
     if (this.#pendingWorkFrames.length === 0) {
       this.#pendingWorkFrames.push({
@@ -194,15 +195,23 @@
         throw new Error(`Unknown action: ${action}`);
     }
 
-    const newWorkItem = {label, handler: callback} as WorkItem;
-    newWorkItem.promise = (new Promise<void>((resolve, reject) => {
-                            newWorkItem.trigger = resolve;
-                            newWorkItem.cancel = reject;
-                          })).then(() => newWorkItem.handler());
-    workItems.push(newWorkItem);
+    let workItem = hasName ? workItems.find(w => w.label === label) : null;
+    if (!workItem) {
+      const newWorkItem = {label} as WorkItem;
+      newWorkItem.promise = (new Promise<void>((resolve, reject) => {
+                              newWorkItem.trigger = resolve;
+                              newWorkItem.cancel = reject;
+                            })).then(() => newWorkItem.handler());
+      workItem = newWorkItem;
+      workItems.push(workItem);
+    }
+    // We are always using the latest handler, so that we don't end up with a
+    // stale results. We are reusing the promise to avoid blocking the first invocation, when
+    // it is being "overridden" by another one.
+    workItem.handler = callback;
 
     this.#scheduleWork();
-    return newWorkItem.promise as Promise<T>;
+    return workItem.promise as Promise<T>;
   }
 
   #scheduleWork(): void {
diff --git a/test/unittests/front_end/panels/elements/components/AccessibilityTreeNode_test.ts b/test/unittests/front_end/panels/elements/components/AccessibilityTreeNode_test.ts
index 4425fc4..78c5cf6 100644
--- a/test/unittests/front_end/panels/elements/components/AccessibilityTreeNode_test.ts
+++ b/test/unittests/front_end/panels/elements/components/AccessibilityTreeNode_test.ts
@@ -19,6 +19,7 @@
       role: 'NodeRole',
       ignored: false,
       properties: [],
+      id: 'NodeId',
     };
 
     await coordinator.done();
@@ -36,6 +37,7 @@
       role: 'NodeRole',
       ignored: true,
       properties: [],
+      id: 'NodeId',
     };
     await coordinator.done();
 
diff --git a/test/unittests/front_end/ui/components/render_coordinator/render_coordinator_test.ts b/test/unittests/front_end/ui/components/render_coordinator/render_coordinator_test.ts
index cfed14d..b6491af 100644
--- a/test/unittests/front_end/ui/components/render_coordinator/render_coordinator_test.ts
+++ b/test/unittests/front_end/ui/components/render_coordinator/render_coordinator_test.ts
@@ -38,6 +38,26 @@
     await validateRecords(expected);
   });
 
+  it('deduplicates named tasks', async () => {
+    const expected = [
+      '[New frame]',
+      '[Read]: Named Read',
+      '[Write]: Unnamed write',
+      '[Write]: Named Write',
+      '[Write]: Unnamed write',
+      '[Queue empty]',
+    ];
+
+    coordinator.observeOnlyNamed = false;
+    void coordinator.read('Named Read', () => {});
+    void coordinator.write(() => {});
+    void coordinator.write('Named Write', () => {});
+    void coordinator.write(() => {});
+    void coordinator.write('Named Write', () => {});
+
+    await validateRecords(expected);
+  });
+
   it('handles nested reads and writes', async () => {
     const expected = [
       '[New frame]',
@@ -190,11 +210,14 @@
     });
 
     it('tracks only the last 100 items', async () => {
-      const expected = new Array(99).fill('[Read]: Named read');
+      const expected = [];
+      for (let i = 51; i < 150; i++) {
+        expected.push(`[Read]: Named read ${i}`);
+      }
       expected.push('[Queue empty]');
 
       for (let i = 0; i < 150; i++) {
-        void coordinator.read('Named read', () => {});
+        void coordinator.read(`Named read ${i}`, () => {});
       }
 
       await validateRecords(expected);
@@ -202,11 +225,14 @@
 
     it('supports different log sizes', async () => {
       coordinator.recordStorageLimit = 10;
-      const expected = new Array(9).fill('[Write]: Named write');
+      const expected = [];
+      for (let i = 41; i < 50; i++) {
+        expected.push(`[Write]: Named write ${i}`);
+      }
       expected.push('[Queue empty]');
 
       for (let i = 0; i < 50; i++) {
-        void coordinator.write('Named write', () => {});
+        void coordinator.write(`Named write ${i}`, () => {});
       }
 
       await validateRecords(expected);