Simon Fraser mentioned in bug 185712, comment 4 that NavigationAction retains a DOM Event, which ultimately keeps the document associated with the DOM Event alive. It is sufficient for NavigationAction to own details of the initiating DOM Event as opposed to the DOM Event itself.
<rdar://problem/40531539>
Created attachment 341221 [details] Patch This patch depends on the patch for bug #185712.
Created attachment 341356 [details] Patch
Comment on attachment 341356 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=341356&action=review > Source/WebCore/loader/NavigationAction.cpp:96 > NavigationAction::NavigationAction(Document& requester, const ResourceRequest& resourceRequest, InitiatedByMainFrame initiatedByMainFrame, NavigationType type, ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy, Event* event, const AtomicString& downloadAttribute) Maybe add a comment in the header file to say that this class shouldn't hold on to any objects from the originating document. > Source/WebCore/loader/NavigationAction.h:93 > + bool buttonDown; > + unsigned short button; > + unsigned short syntheticClickType; > + LayoutPoint absoluteLocation; > + FloatPoint locationInRootViewCoordinates; This would pack better with the larger types first.
Comment on attachment 341356 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=341356&action=review >> Source/WebCore/loader/NavigationAction.cpp:96 >> NavigationAction::NavigationAction(Document& requester, const ResourceRequest& resourceRequest, InitiatedByMainFrame initiatedByMainFrame, NavigationType type, ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy, Event* event, const AtomicString& downloadAttribute) > > Maybe add a comment in the header file to say that this class shouldn't hold on to any objects from the originating document. Will add the following comment above the class definition in the header before landing as formatted: // NavigationAction should never hold a strong reference to the originating document either directly // or indirectly as doing so prevents its destruction even after navigating away from it because // DocumentLoader keeps around the NavigationAction for the last navigation. Will also add the following comment below the "private" section in the header before landing as formatted: // Do not add a strong reference to the originating document or a subobject that holds the // originating document. See comment above the class for more details. >> Source/WebCore/loader/NavigationAction.h:93 >> + FloatPoint locationInRootViewCoordinates; > > This would pack better with the larger types first. Will order these fields as follows: locationInRootViewCoordinates, absoluteLocation, button, syntheticClickType, buttonDown.
Committed r232316: <https://trac.webkit.org/changeset/232316>