Avoid fetching infinite DOM subtrees to prevent crashing [devtools/devtools-frontend : main]

0 views
Skip to first unread message

Philip Pfaffe (Gerrit)

unread,
Nov 3, 2025, 7:06:19 AM (6 days ago) Nov 3
to Alex Rudenko, Devtools-frontend LUCI CQ, devtools-rev...@chromium.org
Attention needed from Alex Rudenko

Philip Pfaffe voted Auto-Submit+1

Auto-Submit+1
Open in Gerrit

Related details

Attention is currently required from:
  • Alex Rudenko
Submit Requirements:
  • requirement satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: devtools/devtools-frontend
Gerrit-Branch: main
Gerrit-Change-Id: I41f6c991a1162be49fbefab5be82c2d66dfe4097
Gerrit-Change-Number: 7105642
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Pfaffe <pfa...@chromium.org>
Gerrit-Reviewer: Alex Rudenko <alexr...@chromium.org>
Gerrit-Reviewer: Philip Pfaffe <pfa...@chromium.org>
Gerrit-CC: Devtools-frontend LUCI CQ <devtools-fro...@luci-project-accounts.iam.gserviceaccount.com>
Gerrit-Attention: Alex Rudenko <alexr...@chromium.org>
Gerrit-Comment-Date: Mon, 03 Nov 2025 12:06:14 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Alex Rudenko (Gerrit)

unread,
Nov 3, 2025, 9:22:14 AM (6 days ago) Nov 3
to Philip Pfaffe, Devtools-frontend LUCI CQ, devtools-rev...@chromium.org
Attention needed from Philip Pfaffe

Alex Rudenko added 1 comment

File front_end/panels/elements/ElementsTreeElement.ts
Line 738, Patchset 1 (Latest): await this.nodeInternal.getSubtree(100, true);
Alex Rudenko . unresolved

for understanding: would it only expand up to 100 levels deep? can we still expand the whole subtree but perhaps by doing multiple getSubtree calls?

Open in Gerrit

Related details

Attention is currently required from:
  • Philip Pfaffe
Submit Requirements:
    • requirement satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: devtools/devtools-frontend
    Gerrit-Branch: main
    Gerrit-Change-Id: I41f6c991a1162be49fbefab5be82c2d66dfe4097
    Gerrit-Change-Number: 7105642
    Gerrit-PatchSet: 1
    Gerrit-Owner: Philip Pfaffe <pfa...@chromium.org>
    Gerrit-Reviewer: Alex Rudenko <alexr...@chromium.org>
    Gerrit-Reviewer: Philip Pfaffe <pfa...@chromium.org>
    Gerrit-CC: Devtools-frontend LUCI CQ <devtools-fro...@luci-project-accounts.iam.gserviceaccount.com>
    Gerrit-Attention: Philip Pfaffe <pfa...@chromium.org>
    Gerrit-Comment-Date: Mon, 03 Nov 2025 14:22:10 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Philip Pfaffe (Gerrit)

    unread,
    Nov 3, 2025, 1:33:07 PM (6 days ago) Nov 3
    to Alex Rudenko, Devtools-frontend LUCI CQ, devtools-rev...@chromium.org
    Attention needed from Alex Rudenko

    Philip Pfaffe added 1 comment

    File front_end/panels/elements/ElementsTreeElement.ts
    Line 738, Patchset 1 (Latest): await this.nodeInternal.getSubtree(100, true);
    Alex Rudenko . unresolved

    for understanding: would it only expand up to 100 levels deep? can we still expand the whole subtree but perhaps by doing multiple getSubtree calls?

    Philip Pfaffe

    It expands the whole subtree, since the actual expansion is implemented just by way of expand: https://source.chromium.org/chromium/chromium/src/+/main:third_party/devtools-frontend/src/front_end/ui/legacy/Treeoutline.ts;drc=35d26d364efb57d0386b98312ba739f7f65ae97e;l=1070

    This means that we'll fetch the node contents for up to 100 children en bloc, and one by one for the following ones.

    Nonetheless the fix is still somewhat best effort. if the nodes have a non-trivial amount of attributes we may still exceed cbor limits and get the same crash. What we really need is a way to cleanly drop notifications if possible. In this instance that would work but not generally.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Alex Rudenko
    Submit Requirements:
    • requirement satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: devtools/devtools-frontend
    Gerrit-Branch: main
    Gerrit-Change-Id: I41f6c991a1162be49fbefab5be82c2d66dfe4097
    Gerrit-Change-Number: 7105642
    Gerrit-PatchSet: 1
    Gerrit-Owner: Philip Pfaffe <pfa...@chromium.org>
    Gerrit-Reviewer: Alex Rudenko <alexr...@chromium.org>
    Gerrit-Reviewer: Philip Pfaffe <pfa...@chromium.org>
    Gerrit-CC: Devtools-frontend LUCI CQ <devtools-fro...@luci-project-accounts.iam.gserviceaccount.com>
    Gerrit-Attention: Alex Rudenko <alexr...@chromium.org>
    Gerrit-Comment-Date: Mon, 03 Nov 2025 18:33:02 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Alex Rudenko <alexr...@chromium.org>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Alex Rudenko (Gerrit)

    unread,
    Nov 4, 2025, 1:48:53 AM (5 days ago) Nov 4
    to Philip Pfaffe, Devtools-frontend LUCI CQ, devtools-rev...@chromium.org
    Attention needed from Philip Pfaffe

    Alex Rudenko added 1 comment

    File front_end/panels/elements/ElementsTreeElement.ts
    Line 738, Patchset 1 (Latest): await this.nodeInternal.getSubtree(100, true);
    Alex Rudenko . resolved

    for understanding: would it only expand up to 100 levels deep? can we still expand the whole subtree but perhaps by doing multiple getSubtree calls?

    Philip Pfaffe

    It expands the whole subtree, since the actual expansion is implemented just by way of expand: https://source.chromium.org/chromium/chromium/src/+/main:third_party/devtools-frontend/src/front_end/ui/legacy/Treeoutline.ts;drc=35d26d364efb57d0386b98312ba739f7f65ae97e;l=1070

    This means that we'll fetch the node contents for up to 100 children en bloc, and one by one for the following ones.

    Nonetheless the fix is still somewhat best effort. if the nodes have a non-trivial amount of attributes we may still exceed cbor limits and get the same crash. What we really need is a way to cleanly drop notifications if possible. In this instance that would work but not generally.

    Alex Rudenko

    Acknowledged

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Philip Pfaffe
    Submit Requirements:
      • requirement satisfiedCode-Owners
      • requirement is not satisfiedCode-Review
      • requirement is not satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: devtools/devtools-frontend
      Gerrit-Branch: main
      Gerrit-Change-Id: I41f6c991a1162be49fbefab5be82c2d66dfe4097
      Gerrit-Change-Number: 7105642
      Gerrit-PatchSet: 1
      Gerrit-Owner: Philip Pfaffe <pfa...@chromium.org>
      Gerrit-Reviewer: Alex Rudenko <alexr...@chromium.org>
      Gerrit-Reviewer: Philip Pfaffe <pfa...@chromium.org>
      Gerrit-CC: Devtools-frontend LUCI CQ <devtools-fro...@luci-project-accounts.iam.gserviceaccount.com>
      Gerrit-Attention: Philip Pfaffe <pfa...@chromium.org>
      Gerrit-Comment-Date: Tue, 04 Nov 2025 06:48:49 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Philip Pfaffe <pfa...@chromium.org>
      Comment-In-Reply-To: Alex Rudenko <alexr...@chromium.org>
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Alex Rudenko (Gerrit)

      unread,
      Nov 4, 2025, 1:48:58 AM (5 days ago) Nov 4
      to Philip Pfaffe, Devtools-frontend LUCI CQ, devtools-rev...@chromium.org
      Attention needed from Philip Pfaffe

      Alex Rudenko voted

      Code-Review+1
      Commit-Queue+2
      Open in Gerrit

      Related details

      Attention is currently required from:
      • Philip Pfaffe
      Submit Requirements:
      • requirement satisfiedCode-Owners
      • requirement satisfiedCode-Review
      • requirement satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: devtools/devtools-frontend
      Gerrit-Branch: main
      Gerrit-Change-Id: I41f6c991a1162be49fbefab5be82c2d66dfe4097
      Gerrit-Change-Number: 7105642
      Gerrit-PatchSet: 1
      Gerrit-Owner: Philip Pfaffe <pfa...@chromium.org>
      Gerrit-Reviewer: Alex Rudenko <alexr...@chromium.org>
      Gerrit-Reviewer: Philip Pfaffe <pfa...@chromium.org>
      Gerrit-CC: Devtools-frontend LUCI CQ <devtools-fro...@luci-project-accounts.iam.gserviceaccount.com>
      Gerrit-Attention: Philip Pfaffe <pfa...@chromium.org>
      Gerrit-Comment-Date: Tue, 04 Nov 2025 06:48:54 +0000
      Gerrit-HasComments: No
      Gerrit-Has-Labels: Yes
      satisfied_requirement
      open
      diffy

      Devtools-frontend LUCI CQ (Gerrit)

      unread,
      Nov 4, 2025, 1:51:15 AM (5 days ago) Nov 4
      to Philip Pfaffe, Alex Rudenko, devtools-rev...@chromium.org

      Devtools-frontend LUCI CQ submitted the change

      Change information

      Commit message:
      Avoid fetching infinite DOM subtrees to prevent crashing
      Fixed: 451651659
      Change-Id: I41f6c991a1162be49fbefab5be82c2d66dfe4097
      Reviewed-by: Alex Rudenko <alexr...@chromium.org>
      Commit-Queue: Alex Rudenko <alexr...@chromium.org>
      Auto-Submit: Philip Pfaffe <pfa...@chromium.org>
      Files:
      • M front_end/core/sdk/DOMModel.ts
      • M front_end/panels/elements/ElementsTreeElement.ts
      Change size: XS
      Delta: 2 files changed, 2 insertions(+), 1 deletion(-)
      Branch: refs/heads/main
      Submit Requirements:
      • requirement satisfiedCode-Review: +1 by Alex Rudenko
      Open in Gerrit
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: merged
      Gerrit-Project: devtools/devtools-frontend
      Gerrit-Branch: main
      Gerrit-Change-Id: I41f6c991a1162be49fbefab5be82c2d66dfe4097
      Gerrit-Change-Number: 7105642
      Gerrit-PatchSet: 2
      Gerrit-Owner: Philip Pfaffe <pfa...@chromium.org>
      Gerrit-Reviewer: Alex Rudenko <alexr...@chromium.org>
      Gerrit-Reviewer: Devtools-frontend LUCI CQ <devtools-fro...@luci-project-accounts.iam.gserviceaccount.com>
      Gerrit-Reviewer: Philip Pfaffe <pfa...@chromium.org>
      open
      diffy
      satisfied_requirement
      Reply all
      Reply to author
      Forward
      0 new messages