New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 516153 link

Starred by 12 users

Issue metadata

Status: WontFix
Owner: ----
Closed: Oct 2015
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocked on:
issue 544731



Sign in to add a comment

Typing into an input and deleting the content creates a constant Node count increase

Reported by samcc...@gmail.com, Aug 1 2015

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.89 Safari/537.36

Steps to reproduce the problem:
To manually reproduce:

Create a html page that only contains a single input.

Take a heapsnapshot and then start a timeline profile

Type a single letter into the input, then hit backspace

repeat this 10 times.

Force a garbage collection via the dev tools trashcan.

Stop the timeline profiler and take another heap snapshot.

You will notice the heap size drops back to the starting point, however the node count does not drop at all. This seems to be incorrect.

There is a reduced automated test case located here
https://github.com/samccone/drool/commit/c0d264cc60052da669440f4dc7292cdd08bfa7a4

------

I have tested this also in canary and the problem is still present.

Thanks for your time!

What is the expected behavior?
The node count should drop back to the starting level.

What went wrong?
It looks like, something internally is retaining a reference to the text that is typed, ideally when a GC has been forced the node count should drop.

Did this work before? N/A 

Chrome version: 44.0.2403.89  Channel: n/a
OS Version: OS X 10.10.0
Flash Version: Shockwave Flash 18.0 r0
 
Screen Shot 2015-08-01 at 11.16.46 AM.png
42.7 KB View Download

Comment 1 by addyo@chromium.org, Aug 1 2015

Labels: Performance-Memory

Comment 2 by tkent@chromium.org, Aug 3 2015

Labels: Cr-Blink-Forms Cr-Blink-Forms-Text Cr-Blink-Editing

Comment 3 by tkent@chromium.org, Aug 3 2015

I suspect Undo buffer.

Comment 4 by tkent@chromium.org, Aug 3 2015

Cc: tkent@chromium.org
Labels: -Cr-Blink-Forms -Cr-Blink-Forms-Text
Status: Untriaged
Confirmed this was due to UndoStack.cpp.

Cc: haraken@chromium.org bashi@chromium.org

Comment 6 by yosin@chromium.org, Aug 11 2015

Status: Available
I think this is due to the input not removing the textnode from the user-agent shadowroot when the value transitions back to "".

Attached is a gif of this effect in 47.0.2517.0 canary (64-bit)
input-leak.gif
382 KB View Download

Comment 8 by yu...@chromium.org, Oct 14 2015

Cc: yu...@chromium.org alph@chromium.org aerotwist@chromium.org
 Issue 543203  has been merged into this issue.

Comment 9 by yu...@chromium.org, Oct 14 2015

Any progress on this? This is a clear memory leak in the platform code, still the issue has no owner. The test case is as simple as a page with just one input element (see attached file).
input.html
45 bytes View Download

Comment 10 by yosin@chromium.org, Oct 15 2015

Labels: Needs-Feedback
Owner: yu...@chromium.org
Status: Assigned
>#7 I think this is due to the input not removing the textnode from the user-agent shadowroot when the value transitions back to "".

HTMLTextFormControlElement::setInnerEditorValue(const String& value)
calls innerEditor->setInnerText(value, ASSERT_NO_EXCEPTION);

HTMLElement::setInnerText() remove the node if value is empty:

    if (!text.contains('\n') && !text.contains('\r')) {
        if (text.isEmpty()) {
            removeChildren();
            return;
        }
        replaceChildrenWithText(this, text, exceptionState);
        return;
    }

Changes for INPUT/TEXTAREA are logged into Undo stack. Contents
before setting empty value are in Undo stack.

>#9 yurs@,
I'm not sure what memory leak yield by you sample: <input placeholder="What needs to be done?">
Running layout test with --enable-leak-detection doesn't report leaks.

Sample command line:
Command line: D:\src\w\cr\src\out\Debug\content_shell.exe --run-layout-test --enable-direct-write --enable-crash
r --crash-dumps-dir=D:\src\w\cr\src\out\Debug\crash-dumps --enable-leak-detection -

Could you tell me how you find memory leak from your sample HTML?


Comment 11 by yu...@chromium.org, Oct 15 2015

@yosin: I did the following:

1. Open input.html from #9.
2. Opend DevTools, start recording Timeline with Memory checkbox ticked.
3. Type 'w' and delete it.
4. Repeat step 3 several times.
5. Stop recording Timeline.

Result number of Nodes increased by the number of times step 3 was repeated. See attached screenshot.

Expected: node count stays stable.


It might not be a leak from the leak detector standpoint if it only detects leaks after unloading document.
Screenshot from 2015-10-15 11:56:29.png
88.7 KB View Download

Comment 12 by yu...@chromium.org, Oct 15 2015

Owner: yosin@chromium.org
Using attached patch I found that the node that leaks is created in Text::createEditingText.
text-leak.patch
14.2 KB Download

Comment 13 by yosin@chromium.org, Oct 16 2015

Labels: -OS-Mac OS-All
Owner: ----
Status: WontFix
yurus@, Thanks for steps. These text nodes are hold in undo stack.

When I get rid of pushing into UnoStep into UndoStack, there are no node leak
as attached file, I repeat type "w" + hit Delete key


--- a/third_party/WebKit/Source/core/editing/commands/UndoStack.cpp
+++ b/third_party/WebKit/Source/core/editing/commands/UndoStack.cpp
@@ -58,12 +58,12 @@ void UndoStack::registerUndoStep(PassRefPtrWillBeRawPtr<UndoStep> step)
         m_undoStack.removeFirst(); // drop oldest item off the far end
     if (!m_inRedo)
         m_redoStack.clear();
-    m_undoStack.append(step);
+    //m_undoStack.append(step);
 }

 void UndoStack::registerRedoStep(PassRefPtrWillBeRawPtr<UndoStep> step)
 {
-    m_redoStack.append(step);
+    //m_redoStack.append(step);
 }


P.S. When I click checkbox in Timeline screen, e.g. JS Profile at left of Memory, the inspector
attempt to call Range::create() with null start/end containers. I remember I saw this issue
in past, but I don't remember issue number.

cr516153.png
74.3 KB View Download
Ok, is there a way we can can not count this as a node then? This is a super misleading thing for users when they are profiling because, most people would never think that the undo stack would be considered to be a node per entry.
Agreed with Sam (#14). Initially the issue was not about the memory leak (as it was in discussions before), but about the unexpected increasing of the node count in the timeline. Consider this documentation

https://developer.chrome.com/devtools/docs/javascript-memory-profiling#proving-a-problem-exists 

According to the article, the timeline can be used to detect the node leaks, but with the such behaviour the timeline cann't be used for node leaks detecting. 
I ran into this exact issue earlier this week, too, and I can't realistically recommend to developers that they use DevTools when they can't trust its data here.

Comment 17 by yu...@chromium.org, Oct 16 2015

Re #13:
> P.S. When I click checkbox in Timeline screen, e.g. JS Profile at left of Memory, the inspector
> attempt to call Range::create() with null start/end containers. I remember I saw this issue
> in past, but I don't remember issue number.

The issue is  crbug.com/510734 . Chrome with release asserts keeps crashing with that assert in Range.h Can you help find an owner for that bug?

Comment 18 by yu...@chromium.org, Oct 19 2015

Blockedon: chromium:544731
DevTools-related discussion forked into https://code.google.com/p/chromium/issues/detail?id=544731

Sign in to add a comment