New issue
Advanced search Search tips

Issue 693975 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 388681
issue 698661



Sign in to add a comment

DOMSelection should dispose temoprary Range object if it is not exposed to script

Project Member Reported by yosin@chromium.org, Feb 19 2017

Issue description

We should dispose Range object created by Selection API, which exposed via getRangeAt(), otherwise DOM mutation operations can be slow down due by unnecessary relocation of Range objects until GC collects them.

Following sample makes 999 unused Range objects in document's Range list:

for (let count = 0; count < 1000; ++count)
  getSelection.collapse(document.body(), 0);

Two options to avoid unused 
 1. Postpone Range object creation until Selection#getRangeAt()
 2. Call Range::dispose() in SelectionEditor::clearDocumentCachedRange() when cached Range object isn't exposed to JS.
 
Summary: DOMSelection should dispose temoprary Range object if it is not exposed to script (was: Selection should dispose Range object if it is exposed to script )
Blocking: 388681
Blocking: 698661
Owner: yoichio@chromium.org
Status: Started (was: Available)
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9608ef8b7480c2d5d86afde8726c0bfbcfb47c7b

commit 9608ef8b7480c2d5d86afde8726c0bfbcfb47c7b
Author: yoichio <yoichio@chromium.org>
Date: Wed Mar 22 04:13:41 2017

Dispose temporary Range in Selection::collapse

That is neither cached nor exposed to js.

BUG=693975

Review-Url: https://codereview.chromium.org/2760273002
Cr-Commit-Position: refs/heads/master@{#458622}

[modify] https://crrev.com/9608ef8b7480c2d5d86afde8726c0bfbcfb47c7b/third_party/WebKit/Source/core/editing/DOMSelection.cpp

Project Member

Comment 7 by bugdroid1@chromium.org, Mar 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ec7293736547e92e7f86ab52edebb469b9a5b93c

commit ec7293736547e92e7f86ab52edebb469b9a5b93c
Author: yoichio <yoichio@chromium.org>
Date: Tue Mar 28 09:20:51 2017

Return EphemeralRange instead of Range in DOMSelection::createRangeFromSelectionEditor

BUG=693975
TEST=No change in behavior

Review-Url: https://codereview.chromium.org/2778963002
Cr-Commit-Position: refs/heads/master@{#460047}

[modify] https://crrev.com/ec7293736547e92e7f86ab52edebb469b9a5b93c/third_party/WebKit/Source/core/editing/DOMSelection.cpp
[modify] https://crrev.com/ec7293736547e92e7f86ab52edebb469b9a5b93c/third_party/WebKit/Source/core/editing/DOMSelection.h

Other redundant Range creation in DOMSelection are
 getRangeAt() calling on shadow DOM.

Functions anchorNode, anchorOffset, focusNodelfocusOffset, isCollapsed,
  removeRange and addRange use getRangeAt() to create temporary Range.
We might have something like:
class TemporaryRange{
 TemporaryRange(range)
 ~TemporaryRange() {
  if (!isSelectionOfDocument())
    range->dispose();
 }
}

if (TemporaryRange temp(primaryRangeOrNull()))
    return temp.range->endContainer();


Labels: -Pri-2 Pri-3
Status: Assigned (was: Started)
Owner: ----
Status: Available (was: Assigned)

Sign in to add a comment