Bug 51062

Summary: [chromium] AssociatedURLLoader leaks m_realLoader to its WebURLLoaderClient.
Product: WebKit Reporter: Darin Fisher (:fishd, Google) <fishd>
Component: WebKit Misc.Assignee: Darin Fisher (:fishd, Google) <fishd>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, michaeln
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
v1 patch dglazkov: review+, dglazkov: commit-queue+

Description Darin Fisher (:fishd, Google) 2010-12-14 15:00:44 PST
[chromium] AssociatedURLLoader leaks m_realLoader to its WebURLLoaderClient.

AssociatedURLLoader should implement WebURLLoaderClient so that it can intercept
client callbacks and modify the WebURLLoader parameter to be |this|.
Comment 1 Darin Fisher (:fishd, Google) 2010-12-14 15:02:12 PST
Created attachment 76577 [details]
v1 patch
Comment 2 Eric Seidel (no email) 2010-12-14 15:30:31 PST
Am I understanding correctly that the WebKit API layer was leaking an internal WebKit (or WebCore?) object?
Comment 3 Eric Seidel (no email) 2010-12-14 15:30:46 PST
Leaking in the sense of "providing a pointer to".
Comment 4 Darin Fisher (:fishd, Google) 2010-12-14 20:47:56 PST
(In reply to comment #2)
> Am I understanding correctly that the WebKit API layer was leaking an internal WebKit (or WebCore?) object?

Nope.  AssociatedURLLoader is a WebURLLoader implementation.  It is implemented in terms of the stock WebURLLoader accessible from WebKitClient::createURLLoader().  That internal WebURLLoader was being exposed via WebURLLoaderClient methods.

For background, AssociatedURLLoader is allocated via WebFrame::createAssociatedURLLoader().  That method provides a WebURLLoader implementation that can be used to load an URL that should be treated like a subresource of the frame (e.g., it should be subject to cancellation when window.stop() is called, it should be subject to appcache, etc.).  The present implementation does not do all of these things yet.  I plan on changing AssociatedURLLoader to be based on WebCore::ResourceLoader so that it can.
Comment 5 Darin Fisher (:fishd, Google) 2010-12-15 16:13:06 PST
Landed as http://trac.webkit.org/changeset/74161