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

Issue 1368623003: Pending scripts can cause disposed documents to leak. (Closed)

Created:
5 years, 3 months ago by jb
Modified:
5 years, 3 months ago
Reviewers:
sof
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Pending scripts can cause disposed documents to leak. The script runner of a document may still be holding pending scripts when the document is disposed (e.g. due to navigation). The script runner, and thus the pending scripts, is cleared in the destructor of the document. However, the pending scripts keep a reference to the script element they belong to. The script element in turn, keeps a guard ref to the document, so the document will never be deleted. Clearing the script runner in dispose clears the pending scripts and breaks the reference cycle so that document can be deleted. BUG=534844 R=sigbjornf@opera.com Committed: https://crrev.com/da753c3cf6f742b968a9b72d27fdc9baaf8d739b Cr-Commit-Position: refs/heads/master@{#350664}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+20 lines, -2 lines) Patch
A third_party/WebKit/LayoutTests/http/tests/loading/pending-script-leak.html View 1 chunk +17 lines, -0 lines 0 comments Download
A + third_party/WebKit/LayoutTests/http/tests/loading/pending-script-leak-expected.txt View 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/dom/Document.cpp View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 6 (1 generated)
jb
New post blink merge CL of 1358583005. PTAL.
5 years, 3 months ago (2015-09-24 12:42:08 UTC) #1
sof
lgtm
5 years, 3 months ago (2015-09-24 19:52:01 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1368623003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1368623003/1
5 years, 3 months ago (2015-09-24 19:53:31 UTC) #4
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 3 months ago (2015-09-24 21:14:27 UTC) #5
commit-bot: I haz the power
5 years, 3 months ago (2015-09-24 21:15:31 UTC) #6
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/da753c3cf6f742b968a9b72d27fdc9baaf8d739b
Cr-Commit-Position: refs/heads/master@{#350664}

Powered by Google App Engine
This is Rietveld 408576698