Title: [198685] branches/safari-601.1.46-branch
- Revision
- 198685
- Author
- [email protected]
- Date
- 2016-03-25 12:50:13 -0700 (Fri, 25 Mar 2016)
Log Message
Merge r197856. rdar://problem/25152411
Modified Paths
Added Paths
Diff
Modified: branches/safari-601.1.46-branch/LayoutTests/ChangeLog (198684 => 198685)
--- branches/safari-601.1.46-branch/LayoutTests/ChangeLog 2016-03-25 19:32:53 UTC (rev 198684)
+++ branches/safari-601.1.46-branch/LayoutTests/ChangeLog 2016-03-25 19:50:13 UTC (rev 198685)
@@ -1,3 +1,29 @@
+2016-03-25 Matthew Hanson <[email protected]>
+
+ Merge r197856. rdar://problem/25152411
+
+ 2016-03-09 Andreas Kling <[email protected]>
+
+ ImageDocuments leak their world.
+ <https://webkit.org/b/155167>
+ <rdar://problem/24987363>
+
+ Reviewed by Antti Koivisto.
+
+ Made a little test that loads an image into an <iframe> 10 times and then
+ triggers a garbage collection and checks that all the documents got destroyed.
+
+ Prior to this change, all 10 ImageDocuments would remain alive at the end.
+
+ This got rolled out the first time because it failed on bots. It failed due
+ to expecting a specific number of documents to be live at the start of the
+ test, which was not reliable on bots since we appear to have more leaks(!)
+
+ Tweaked the test to check the delta in live document count instead.
+
+ * fast/dom/ImageDocument-world-leak-expected.txt: Added.
+ * fast/dom/ImageDocument-world-leak.html: Added.
+
2016-03-24 Matthew Hanson <[email protected]>
Merge r198592. rdar://problem/25271136
Added: branches/safari-601.1.46-branch/LayoutTests/fast/dom/ImageDocument-world-leak-expected.txt (0 => 198685)
--- branches/safari-601.1.46-branch/LayoutTests/fast/dom/ImageDocument-world-leak-expected.txt (rev 0)
+++ branches/safari-601.1.46-branch/LayoutTests/fast/dom/ImageDocument-world-leak-expected.txt 2016-03-25 19:50:13 UTC (rev 198685)
@@ -0,0 +1,4 @@
+Test that ImageDocuments don't leak their world.
+
+PASS: Less than 10 new documents since test began.
+
Added: branches/safari-601.1.46-branch/LayoutTests/fast/dom/ImageDocument-world-leak.html (0 => 198685)
--- branches/safari-601.1.46-branch/LayoutTests/fast/dom/ImageDocument-world-leak.html (rev 0)
+++ branches/safari-601.1.46-branch/LayoutTests/fast/dom/ImageDocument-world-leak.html 2016-03-25 19:50:13 UTC (rev 198685)
@@ -0,0 +1,56 @@
+<head>
+<script>
+function gc() {
+ if (window.GCController)
+ GCController.collect();
+}
+
+function numberOfLiveDocuments() {
+ if (window.internals)
+ return window.internals.numberOfLiveDocuments();
+ return 1;
+}
+
+gc();
+
+iterationsToRun = 10;
+iterationsRemaining = iterationsToRun;
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+}
+
+function test() {
+ var f = document.getElementById("frame");
+ f._onload_ = function() {
+ f.contentDocument.open();
+ f.contentDocument.close();
+ --iterationsRemaining;
+ if (iterationsRemaining)
+ setTimeout("test()", 0);
+ else {
+ gc();
+ var numberOfLiveDocumentsDelta = numberOfLiveDocuments() - numberOfLiveDocumentsAtStart;
+ var out = document.getElementById("out");
+ if (numberOfLiveDocumentsDelta < iterationsToRun)
+ out.innerText += "PASS: Less than " + iterationsToRun + " new documents since test began.";
+ else
+ out.innerText += "FAIL: Too many new documents since test began: " + numberOfLiveDocumentsDelta;
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }
+ }
+ f.setAttribute("src", "resources/apple.gif");
+}
+
+function begin() {
+ numberOfLiveDocumentsAtStart = numberOfLiveDocuments();
+ test();
+}
+</script>
+</head>
+<body _onload_="begin()">
+ <p>Test that ImageDocuments don't leak their world. </p>
+ <pre id="out"></pre>
+ <iframe id="frame"></iframe>
+</body>
Modified: branches/safari-601.1.46-branch/Source/WebCore/ChangeLog (198684 => 198685)
--- branches/safari-601.1.46-branch/Source/WebCore/ChangeLog 2016-03-25 19:32:53 UTC (rev 198684)
+++ branches/safari-601.1.46-branch/Source/WebCore/ChangeLog 2016-03-25 19:50:13 UTC (rev 198685)
@@ -1,3 +1,40 @@
+2016-03-25 Matthew Hanson <[email protected]>
+
+ Merge r197856. rdar://problem/25152411
+
+ 2016-03-09 Andreas Kling <[email protected]>
+
+ ImageDocuments leak their world.
+ <https://webkit.org/b/155167>
+ <rdar://problem/24987363>
+
+ Reviewed by Antti Koivisto.
+
+ ImageDocument uses a special code path in ImageLoader in order to manually
+ control how the image is loaded. It has to do this because the ImageDocument
+ is really just a synthetic wrapper around a main resource that's an image.
+
+ This custom loading code had a bug where it would create a new CachedImage
+ and neglect to set its CachedResource::m_state flag to Pending (which is
+ normally set by CachedResource::load(), but we don't call that for these.)
+
+ This meant that when ImageDocument called CachedImage::finishLoading() to
+ trigger the notifyFinished() callback path, the image would look at its
+ loading state and see that it was Unknown (not Pending), and conclude that
+ it hadn't loaded yet. So we never got the notifyFinished() signal.
+
+ The world leaks here because ImageLoader slaps a ref on its <img> element
+ while it waits for the loading operation to complete. Once finished, whether
+ successfully or with an error, it derefs the <img>.
+
+ Since we never fired notifyFinished(), we ended up with an extra ref on
+ these <img> forever, and then the element kept its document alive too.
+
+ Test: fast/dom/ImageDocument-world-leak.html
+
+ * loader/ImageLoader.cpp:
+ (WebCore::ImageLoader::updateFromElement):
+
2016-03-24 Matthew Hanson <[email protected]>
Merge custom patch. rdar://problem/25152415.
Modified: branches/safari-601.1.46-branch/Source/WebCore/loader/ImageLoader.cpp (198684 => 198685)
--- branches/safari-601.1.46-branch/Source/WebCore/loader/ImageLoader.cpp 2016-03-25 19:32:53 UTC (rev 198684)
+++ branches/safari-601.1.46-branch/Source/WebCore/loader/ImageLoader.cpp 2016-03-25 19:50:13 UTC (rev 198685)
@@ -188,6 +188,7 @@
bool autoLoadOtherImages = document.cachedResourceLoader().autoLoadImages();
document.cachedResourceLoader().setAutoLoadImages(false);
newImage = new CachedImage(request.resourceRequest(), m_element.document().page()->sessionID());
+ newImage->setStatus(CachedResource::Pending);
newImage->setLoading(true);
newImage->setOwningCachedResourceLoader(&document.cachedResourceLoader());
document.cachedResourceLoader().m_documentResources.set(newImage->url(), newImage.get());
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
