Bug 88470 - MHTMLArchives are leaked when a MHTML file is loaded
Summary: MHTMLArchives are leaked when a MHTML file is loaded
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jay Civelli
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-06 16:51 PDT by Jay Civelli
Modified: 2012-06-25 16:04 PDT (History)
5 users (show)

See Also:


Attachments
Patch (5.34 KB, patch)
2012-06-06 16:55 PDT, Jay Civelli
no flags Details | Formatted Diff | Diff
Patch (5.35 KB, patch)
2012-06-07 17:07 PDT, Jay Civelli
no flags Details | Formatted Diff | Diff
Patch for landing (5.35 KB, patch)
2012-06-07 17:47 PDT, Jay Civelli
no flags Details | Formatted Diff | Diff
Patch for landing (5.35 KB, patch)
2012-06-25 11:20 PDT, Jay Civelli
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jay Civelli 2012-06-06 16:51:22 PDT
When we load a MHTML document, the MHTMLArchives and resources are leaked.
This is caused by circular references of the MHTMLArchives, which are ref counted.

The problem comes from the fact that MHTML is a flat format that lists frames and resources, when the WebKit Archive class has a tree structure.
In order to have any frame have access to any resources, we add all archives and resources to all other archives. That causes the circular references, preventing the archives from going away.
Comment 1 Jay Civelli 2012-06-06 16:55:57 PDT
Created attachment 146147 [details]
Patch
Comment 2 Adam Barth 2012-06-07 15:22:52 PDT
Comment on attachment 146147 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=146147&action=review

> Source/WebCore/loader/archive/Archive.cpp:46
> +    for (Vector<RefPtr<Archive> >::iterator it = m_subframeArchives.begin(); it != m_subframeArchives.end(); ++it)

This loop should have { } around its body.
Comment 3 Jay Civelli 2012-06-07 17:07:38 PDT
Created attachment 146422 [details]
Patch
Comment 4 Jay Civelli 2012-06-07 17:47:01 PDT
Created attachment 146432 [details]
Patch for landing
Comment 5 WebKit Review Bot 2012-06-07 19:21:11 PDT
Comment on attachment 146432 [details]
Patch for landing

Rejecting attachment 146432 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: /mnt/git/webkit-commit-queue/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/12909471
Comment 6 Jay Civelli 2012-06-08 09:37:32 PDT
Adam,
Could you R+ CQ+?
I tried to land but somehow the reviewer in the log did not get set, sorry.

Thanks!
Comment 7 Jay Civelli 2012-06-25 11:20:21 PDT
Created attachment 149330 [details]
Patch for landing
Comment 8 WebKit Review Bot 2012-06-25 16:03:46 PDT
Comment on attachment 149330 [details]
Patch for landing

Clearing flags on attachment: 149330

Committed r121191: <http://trac.webkit.org/changeset/121191>
Comment 9 WebKit Review Bot 2012-06-25 16:04:09 PDT
All reviewed patches have been landed.  Closing bug.