Fix for a memory leak with MHTMLArchives.

MHTML files present a flat list of frames and resources but the WebKit Archive
has a tree strcture. So the MHTMLArchive class make sures that every frame
knows about any other frames and resources.
Because these objects are ref counted, that would introduce circular references
preventing the entire Archive from being deleted.
This fixes this by:
- making sure the top-frame (which appears as the first entry in the MHTML) is
  not referenced by the other frames.
- when the main frame is deleted it traverse the entire subarchive (sub-frames)
  graph and makes sure they clear all their references to other subarchives.

https://bugs.webkit.org/show_bug.cgi?id=88470

Reviewed by Adam Barth.

* loader/archive/Archive.cpp:
(WebCore::Archive::clearAllSubframeArchives):
(WebCore):
(WebCore::Archive::clearAllSubframeArchivesImpl):
* loader/archive/Archive.h:
(Archive):
* loader/archive/mhtml/MHTMLArchive.cpp:
(WebCore::MHTMLArchive::~MHTMLArchive):
(WebCore):
(WebCore::MHTMLArchive::create):
* loader/archive/mhtml/MHTMLArchive.h:
(MHTMLArchive):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@121191 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 6961277..093d2bd 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,35 @@
+2012-06-25  Jay Civelli  <jcivelli@chromium.org>
+
+        Fix for a memory leak with MHTMLArchives.
+
+        MHTML files present a flat list of frames and resources but the WebKit Archive
+        has a tree strcture. So the MHTMLArchive class make sures that every frame
+        knows about any other frames and resources.
+        Because these objects are ref counted, that would introduce circular references
+        preventing the entire Archive from being deleted.
+        This fixes this by:
+        - making sure the top-frame (which appears as the first entry in the MHTML) is
+          not referenced by the other frames.
+        - when the main frame is deleted it traverse the entire subarchive (sub-frames)
+          graph and makes sure they clear all their references to other subarchives.
+
+        https://bugs.webkit.org/show_bug.cgi?id=88470
+
+        Reviewed by Adam Barth.
+
+        * loader/archive/Archive.cpp:
+        (WebCore::Archive::clearAllSubframeArchives):
+        (WebCore):
+        (WebCore::Archive::clearAllSubframeArchivesImpl):
+        * loader/archive/Archive.h:
+        (Archive):
+        * loader/archive/mhtml/MHTMLArchive.cpp:
+        (WebCore::MHTMLArchive::~MHTMLArchive):
+        (WebCore):
+        (WebCore::MHTMLArchive::create):
+        * loader/archive/mhtml/MHTMLArchive.h:
+        (MHTMLArchive):
+
 2012-06-25  Alpha Lam  <hclam@chromium.org>
 
         Unreviewed, rolling out r121178.

diff --git a/Source/WebCore/loader/archive/Archive.cpp b/Source/WebCore/loader/archive/Archive.cpp
index 8f45fbd..ea76a11 100644 (file)
--- a/Source/WebCore/loader/archive/Archive.cpp
+++ b/Source/WebCore/loader/archive/Archive.cpp
@@ -35,4 +35,21 @@ Archive::~Archive()
 {
 }
 
+void Archive::clearAllSubframeArchives()
+{
+    Vector<RefPtr<Archive> > clearedArchives;
+    clearAllSubframeArchivesImpl(&clearedArchives);
+}
+
+void Archive::clearAllSubframeArchivesImpl(Vector<RefPtr<Archive> >* clearedArchives)
+{
+    for (Vector<RefPtr<Archive> >::iterator it = m_subframeArchives.begin(); it != m_subframeArchives.end(); ++it) {
+        if (!clearedArchives->contains(*it)) {
+            clearedArchives->append(*it);
+            (*it)->clearAllSubframeArchivesImpl(clearedArchives);
+        }
+    }
+    m_subframeArchives.clear();
+}
+
 }

diff --git a/Source/WebCore/loader/archive/Archive.h b/Source/WebCore/loader/archive/Archive.h
index d41c608..b7656e5 100644 (file)
--- a/Source/WebCore/loader/archive/Archive.h
+++ b/Source/WebCore/loader/archive/Archive.h
@@ -56,8 +56,12 @@ protected:
     void setMainResource(PassRefPtr<ArchiveResource> mainResource) { m_mainResource = mainResource; }
     void addSubresource(PassRefPtr<ArchiveResource> subResource) { m_subresources.append(subResource); }
     void addSubframeArchive(PassRefPtr<Archive> subframeArchive) { m_subframeArchives.append(subframeArchive); }
-    
+
+    void clearAllSubframeArchives();
+
 private:
+    void clearAllSubframeArchivesImpl(Vector<RefPtr<Archive> >* clearedArchives);
+
     RefPtr<ArchiveResource> m_mainResource;
     Vector<RefPtr<ArchiveResource> > m_subresources;
     Vector<RefPtr<Archive> > m_subframeArchives;

diff --git a/Source/WebCore/loader/archive/mhtml/MHTMLArchive.cpp b/Source/WebCore/loader/archive/mhtml/MHTMLArchive.cpp
index b996f2d..cb9c7e5 100644 (file)
--- a/Source/WebCore/loader/archive/mhtml/MHTMLArchive.cpp
+++ b/Source/WebCore/loader/archive/mhtml/MHTMLArchive.cpp
@@ -96,6 +96,12 @@ MHTMLArchive::MHTMLArchive()
 {
 }
 
+MHTMLArchive::~MHTMLArchive()
+{
+    // Because all frames know about each other we need to perform a deep clearing of the archives graph.
+    clearAllSubframeArchives();
+}
+
 PassRefPtr<MHTMLArchive> MHTMLArchive::create()
 {
     return adoptRef(new MHTMLArchive);
@@ -115,7 +121,7 @@ PassRefPtr<MHTMLArchive> MHTMLArchive::create(const KURL& url, SharedBuffer* dat
     // Since MHTML is a flat format, we need to make all frames aware of all resources.
     for (size_t i = 0; i < parser.frameCount(); ++i) {
         RefPtr<MHTMLArchive> archive = parser.frameAt(i);
-        for (size_t j = 0; j < parser.frameCount(); ++j) {
+        for (size_t j = 1; j < parser.frameCount(); ++j) {
             if (i != j)
                 archive->addSubframeArchive(parser.frameAt(j));
         }

diff --git a/Source/WebCore/loader/archive/mhtml/MHTMLArchive.h b/Source/WebCore/loader/archive/mhtml/MHTMLArchive.h
index 0371536..dd14d99 100644 (file)
--- a/Source/WebCore/loader/archive/mhtml/MHTMLArchive.h
+++ b/Source/WebCore/loader/archive/mhtml/MHTMLArchive.h
@@ -52,6 +52,8 @@ public:
     // Binary encoding results in smaller MHTML files but they might not work in other browsers.
     static PassRefPtr<SharedBuffer> generateMHTMLDataUsingBinaryEncoding(Page*);
 
+    virtual ~MHTMLArchive();
+
 private:
     static PassRefPtr<SharedBuffer> generateMHTMLData(Page*, bool useBinaryEncoding);