Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(523)

Issue 23180002: Revert 156078 "Fix XMLHttpRequest leak document when send() is c..." (Closed)

Created:
7 years, 4 months ago by tasak
Modified:
7 years, 4 months ago
Reviewers:
kouhei (in TOK)
CC:
blink-reviews
Visibility:
Public.

Description

Revert 156078 "Fix XMLHttpRequest leak document when send() is c..." > Fix XMLHttpRequest leak document when send() is called multiple times. > > XMLHttpRequest creates a ThreadableLoader which may call XHR async, so it setPendingActivity() to avoid being destroyed. However, before this patch, unsetPendingActivity() was called asynchronously after ThreadableLoader was destroyed, so it lead to multiple problems: > a) When next m_loader was set in send() with pending unsetPendingActivity(), the pendingActivity may be dropped even when there exists new m_loader need protection. > b) pendingActivity may be set multiple times from pending unsetPendingActivity(), but dropProtectionSoon() only decrements m_pendingActivityCount by one, leading to a leak. > > This patch fix the above problems by unsetPendingActivity() synchronously with m_loader destruction where possible. XMLHttpRequest::stop() still uses asynchronous unsetPendingActivity() to workaround issues mentioned in r152266. > > The file "leak-check.js" was moved from fast/dom to fast/js to enable access from http tests. > > BUG=270000 > > Review URL: https://chromiumcodereview.appspot.com/23005006 TBR=kouhei@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=156082

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -122 lines) Patch
M trunk/LayoutTests/editing/context-menu-leak-document.html View 1 chunk +1 line, -1 line 0 comments Download
M trunk/LayoutTests/editing/selection/leak-document-with-selection-inside.html View 1 chunk +1 line, -1 line 0 comments Download
M trunk/LayoutTests/fast/dom/NodeIterator/NodeIterator-leak-document.html View 1 chunk +1 line, -1 line 0 comments Download
M trunk/LayoutTests/fast/dom/TreeWalker/TreeWalker-leak-document.html View 1 chunk +1 line, -1 line 0 comments Download
A + trunk/LayoutTests/fast/dom/resources/leak-check.js View 0 chunks +-1 lines, --1 lines 0 comments Download
M trunk/LayoutTests/fast/events/event-sender-context-click-leak-document.html View 1 chunk +1 line, -1 line 0 comments Download
D trunk/LayoutTests/fast/js/resources/leak-check.js View 1 chunk +0 lines, -74 lines 0 comments Download
D trunk/LayoutTests/http/tests/xmlhttprequest/resources/multiple-send.html View 1 chunk +0 lines, -9 lines 0 comments Download
D trunk/LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-leaks-on-multiple-send.html View 1 chunk +0 lines, -12 lines 0 comments Download
D trunk/LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-leaks-on-multiple-send-expected.txt View 1 chunk +0 lines, -5 lines 0 comments Download
M trunk/Source/core/xml/XMLHttpRequest.h View 2 chunks +1 line, -6 lines 0 comments Download
M trunk/Source/core/xml/XMLHttpRequest.cpp View 4 chunks +4 lines, -12 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
tasak
7 years, 4 months ago (2013-08-14 08:38:07 UTC) #1
tasak
7 years, 4 months ago (2013-08-14 08:38:53 UTC) #2
Message was sent while issue was closed.
Committed patchset #1 manually as r156082.

Powered by Google App Engine
This is Rietveld 408576698