Title: [186410] releases/WebKitGTK/webkit-2.8/Source
Revision
186410
Author
[email protected]
Date
2015-07-07 00:28:18 -0700 (Tue, 07 Jul 2015)

Log Message

Merge r185542 - [WK2] API::Navigation objects are leaked on history navigation to HistoryItems in PageCache
https://bugs.webkit.org/show_bug.cgi?id=145948

Reviewed by Darin Adler.

Source/WebCore:

API::Navigation objects were leaked on history navigation to
HistoryItems in PageCache. In such case, we would create 2 Navigation
objects instead of 1 and the first one would be leaked. The reason
we create the second one is because we fail to pass along the
navigationID from the UIProcess to the WebProcess and then back to the
UIProcess. On the IPC back to the UIProcess, the navigationID ends up
being 0 so the UIProcess creates a new Navigation object, thinking that
the load was triggered by the WebContent process.

We now pass along the navigationID, even if the HistoryItem is in the
PageCache and we end up reusing the cached DocumentLoader, instead of
creating a new one. A new updateCachedDocumentLoader() delegate is
added to the FrameLoaderClient, similarly to the pre-existing
createDocumentLoader() but for the case where the DocumentLoader gets
reused.

* loader/EmptyClients.h:
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::loadDifferentDocumentItem):
* loader/FrameLoaderClient.h:

Source/WebKit/mac:

Add empty implementation for new
FrameLoaderClient::updatedCachedDocumentLoader().

* WebCoreSupport/WebFrameLoaderClient.h:

Source/WebKit/win:

Add empty implementation for new
FrameLoaderClient::updatedCachedDocumentLoader().

* WebCoreSupport/WebFrameLoaderClient.h:

Source/WebKit2:

API::Navigation objects were leaked on history navigation to
HistoryItems in PageCache. In such case, we would create 2 Navigation
objects instead of 1 and the first one would be leaked. The reason
we create the second one is because we fail to pass along the
navigationID from the UIProcess to the WebProcess and then back to the
UIProcess. On the IPC back to the UIProcess, the navigationID ends up
being 0 so the UIProcess creates a new Navigation object, thinking that
the load was triggered by the WebContent process.

We now pass along the navigationID, even if the HistoryItem is in the
PageCache and we end up reusing the cached DocumentLoader, instead of
creating a new one. A new updateCachedDocumentLoader() delegate is
added to the FrameLoaderClient, similarly to the pre-existing
createDocumentLoader() but for the case where the DocumentLoader gets
reused.

* WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::updateCachedDocumentLoader):
* WebProcess/WebCoreSupport/WebFrameLoaderClient.h:
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::goForward):
(WebKit::WebPage::goBack):
(WebKit::WebPage::goToBackForwardItem):
(WebKit::WebPage::updateCachedDocumentLoader):
* WebProcess/WebPage/WebPage.h:

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog (186409 => 186410)


--- releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog	2015-07-07 07:21:38 UTC (rev 186409)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog	2015-07-07 07:28:18 UTC (rev 186410)
@@ -1,3 +1,31 @@
+2015-06-13  Chris Dumez  <[email protected]>
+
+        [WK2] API::Navigation objects are leaked on history navigation to HistoryItems in PageCache
+        https://bugs.webkit.org/show_bug.cgi?id=145948
+
+        Reviewed by Darin Adler.
+
+        API::Navigation objects were leaked on history navigation to
+        HistoryItems in PageCache. In such case, we would create 2 Navigation
+        objects instead of 1 and the first one would be leaked. The reason
+        we create the second one is because we fail to pass along the
+        navigationID from the UIProcess to the WebProcess and then back to the
+        UIProcess. On the IPC back to the UIProcess, the navigationID ends up
+        being 0 so the UIProcess creates a new Navigation object, thinking that
+        the load was triggered by the WebContent process.
+
+        We now pass along the navigationID, even if the HistoryItem is in the
+        PageCache and we end up reusing the cached DocumentLoader, instead of
+        creating a new one. A new updateCachedDocumentLoader() delegate is
+        added to the FrameLoaderClient, similarly to the pre-existing
+        createDocumentLoader() but for the case where the DocumentLoader gets
+        reused.
+
+        * loader/EmptyClients.h:
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::loadDifferentDocumentItem):
+        * loader/FrameLoaderClient.h:
+
 2015-06-11  Zalan Bujtas  <[email protected]>
 
         Do not crash when the descendant frame tree is destroyed during layout.

Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/loader/EmptyClients.h (186409 => 186410)


--- releases/WebKitGTK/webkit-2.8/Source/WebCore/loader/EmptyClients.h	2015-07-07 07:21:38 UTC (rev 186409)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/loader/EmptyClients.h	2015-07-07 07:28:18 UTC (rev 186410)
@@ -348,6 +348,7 @@
     virtual void prepareForDataSourceReplacement() override { }
 
     virtual PassRefPtr<DocumentLoader> createDocumentLoader(const ResourceRequest&, const SubstituteData&) override;
+    virtual void updateCachedDocumentLoader(DocumentLoader&) override { }
     virtual void setTitle(const StringWithDirection&, const URL&) override { }
 
     virtual String userAgent(const URL&) override { return ""; }

Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/loader/FrameLoader.cpp (186409 => 186410)


--- releases/WebKitGTK/webkit-2.8/Source/WebCore/loader/FrameLoader.cpp	2015-07-07 07:21:38 UTC (rev 186409)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/loader/FrameLoader.cpp	2015-07-07 07:28:18 UTC (rev 186410)
@@ -3153,6 +3153,7 @@
 
     if (CachedPage* cachedPage = PageCache::singleton().get(item, m_frame.page())) {
         auto documentLoader = cachedPage->documentLoader();
+        m_client.updateCachedDocumentLoader(*documentLoader);
         documentLoader->setTriggeringAction(NavigationAction(documentLoader->request(), loadType, false));
         documentLoader->setLastCheckedRequest(ResourceRequest());
         loadWithDocumentLoader(documentLoader, loadType, 0, AllowNavigationToInvalidURL::Yes);

Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/loader/FrameLoaderClient.h (186409 => 186410)


--- releases/WebKitGTK/webkit-2.8/Source/WebCore/loader/FrameLoaderClient.h	2015-07-07 07:21:38 UTC (rev 186409)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/loader/FrameLoaderClient.h	2015-07-07 07:28:18 UTC (rev 186410)
@@ -246,6 +246,7 @@
         virtual void prepareForDataSourceReplacement() = 0;
 
         virtual PassRefPtr<DocumentLoader> createDocumentLoader(const ResourceRequest&, const SubstituteData&) = 0;
+        virtual void updateCachedDocumentLoader(DocumentLoader&) = 0;
         virtual void setTitle(const StringWithDirection&, const URL&) = 0;
 
         virtual String userAgent(const URL&) = 0;

Modified: releases/WebKitGTK/webkit-2.8/Source/WebKit/mac/ChangeLog (186409 => 186410)


--- releases/WebKitGTK/webkit-2.8/Source/WebKit/mac/ChangeLog	2015-07-07 07:21:38 UTC (rev 186409)
+++ releases/WebKitGTK/webkit-2.8/Source/WebKit/mac/ChangeLog	2015-07-07 07:28:18 UTC (rev 186410)
@@ -1,3 +1,15 @@
+2015-06-13  Chris Dumez  <[email protected]>
+
+        [WK2] API::Navigation objects are leaked on history navigation to HistoryItems in PageCache
+        https://bugs.webkit.org/show_bug.cgi?id=145948
+
+        Reviewed by Darin Adler.
+
+        Add empty implementation for new
+        FrameLoaderClient::updatedCachedDocumentLoader().
+
+        * WebCoreSupport/WebFrameLoaderClient.h:
+
 2015-05-19  Brady Eidson  <[email protected]>
 
         X-Frame-Options headers not respected when loading from application cache.

Modified: releases/WebKitGTK/webkit-2.8/Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.h (186409 => 186410)


--- releases/WebKitGTK/webkit-2.8/Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.h	2015-07-07 07:21:38 UTC (rev 186409)
+++ releases/WebKitGTK/webkit-2.8/Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.h	2015-07-07 07:28:18 UTC (rev 186410)
@@ -190,6 +190,7 @@
     virtual void didFinishLoad() override;
     virtual void prepareForDataSourceReplacement() override;
     virtual PassRefPtr<WebCore::DocumentLoader> createDocumentLoader(const WebCore::ResourceRequest&, const WebCore::SubstituteData&) override;
+    virtual void updateCachedDocumentLoader(WebCore::DocumentLoader&) override { }
 
     virtual void setTitle(const WebCore::StringWithDirection&, const WebCore::URL&) override;
 

Modified: releases/WebKitGTK/webkit-2.8/Source/WebKit/win/ChangeLog (186409 => 186410)


--- releases/WebKitGTK/webkit-2.8/Source/WebKit/win/ChangeLog	2015-07-07 07:21:38 UTC (rev 186409)
+++ releases/WebKitGTK/webkit-2.8/Source/WebKit/win/ChangeLog	2015-07-07 07:28:18 UTC (rev 186410)
@@ -1,3 +1,15 @@
+2015-06-13  Chris Dumez  <[email protected]>
+
+        [WK2] API::Navigation objects are leaked on history navigation to HistoryItems in PageCache
+        https://bugs.webkit.org/show_bug.cgi?id=145948
+
+        Reviewed by Darin Adler.
+
+        Add empty implementation for new
+        FrameLoaderClient::updatedCachedDocumentLoader().
+
+        * WebCoreSupport/WebFrameLoaderClient.h:
+
 2015-05-19  Brady Eidson  <[email protected]>
 
         X-Frame-Options headers not respected when loading from application cache.

Modified: releases/WebKitGTK/webkit-2.8/Source/WebKit/win/WebCoreSupport/WebFrameLoaderClient.h (186409 => 186410)


--- releases/WebKitGTK/webkit-2.8/Source/WebKit/win/WebCoreSupport/WebFrameLoaderClient.h	2015-07-07 07:21:38 UTC (rev 186409)
+++ releases/WebKitGTK/webkit-2.8/Source/WebKit/win/WebCoreSupport/WebFrameLoaderClient.h	2015-07-07 07:28:18 UTC (rev 186410)
@@ -149,6 +149,8 @@
     virtual WTF::String userAgent(const WebCore::URL&) override;
 
     virtual PassRefPtr<WebCore::DocumentLoader> createDocumentLoader(const WebCore::ResourceRequest&, const WebCore::SubstituteData&);
+    virtual void updateCachedDocumentLoader(WebCore::DocumentLoader&) override { }
+
     virtual void setTitle(const WebCore::StringWithDirection&, const WebCore::URL&);
 
     virtual void savePlatformDataToCachedFrame(WebCore::CachedFrame*) override;

Modified: releases/WebKitGTK/webkit-2.8/Source/WebKit2/ChangeLog (186409 => 186410)


--- releases/WebKitGTK/webkit-2.8/Source/WebKit2/ChangeLog	2015-07-07 07:21:38 UTC (rev 186409)
+++ releases/WebKitGTK/webkit-2.8/Source/WebKit2/ChangeLog	2015-07-07 07:28:18 UTC (rev 186410)
@@ -1,3 +1,36 @@
+2015-06-13  Chris Dumez  <[email protected]>
+
+        [WK2] API::Navigation objects are leaked on history navigation to HistoryItems in PageCache
+        https://bugs.webkit.org/show_bug.cgi?id=145948
+
+        Reviewed by Darin Adler.
+
+        API::Navigation objects were leaked on history navigation to
+        HistoryItems in PageCache. In such case, we would create 2 Navigation
+        objects instead of 1 and the first one would be leaked. The reason
+        we create the second one is because we fail to pass along the
+        navigationID from the UIProcess to the WebProcess and then back to the
+        UIProcess. On the IPC back to the UIProcess, the navigationID ends up
+        being 0 so the UIProcess creates a new Navigation object, thinking that
+        the load was triggered by the WebContent process.
+
+        We now pass along the navigationID, even if the HistoryItem is in the
+        PageCache and we end up reusing the cached DocumentLoader, instead of
+        creating a new one. A new updateCachedDocumentLoader() delegate is
+        added to the FrameLoaderClient, similarly to the pre-existing
+        createDocumentLoader() but for the case where the DocumentLoader gets
+        reused.
+
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+        (WebKit::WebFrameLoaderClient::updateCachedDocumentLoader):
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.h:
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::goForward):
+        (WebKit::WebPage::goBack):
+        (WebKit::WebPage::goToBackForwardItem):
+        (WebKit::WebPage::updateCachedDocumentLoader):
+        * WebProcess/WebPage/WebPage.h:
+
 2015-05-29  Joseph Pecoraro  <[email protected]>
 
         Web Inspector: Crash closing a related tab with Web Inspector open while page is refreshing

Modified: releases/WebKitGTK/webkit-2.8/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp (186409 => 186410)


--- releases/WebKitGTK/webkit-2.8/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp	2015-07-07 07:21:38 UTC (rev 186409)
+++ releases/WebKitGTK/webkit-2.8/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp	2015-07-07 07:28:18 UTC (rev 186410)
@@ -1221,6 +1221,11 @@
     return m_frame->page()->createDocumentLoader(*m_frame->coreFrame(), request, substituteData);
 }
 
+void WebFrameLoaderClient::updateCachedDocumentLoader(WebCore::DocumentLoader& loader)
+{
+    m_frame->page()->updateCachedDocumentLoader(static_cast<WebDocumentLoader&>(loader), *m_frame->coreFrame());
+}
+
 void WebFrameLoaderClient::setTitle(const StringWithDirection& title, const URL& url)
 {
     WebPage* webPage = m_frame->page();

Modified: releases/WebKitGTK/webkit-2.8/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.h (186409 => 186410)


--- releases/WebKitGTK/webkit-2.8/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.h	2015-07-07 07:21:38 UTC (rev 186409)
+++ releases/WebKitGTK/webkit-2.8/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.h	2015-07-07 07:28:18 UTC (rev 186410)
@@ -160,6 +160,8 @@
     virtual void prepareForDataSourceReplacement() override;
     
     virtual PassRefPtr<WebCore::DocumentLoader> createDocumentLoader(const WebCore::ResourceRequest&, const WebCore::SubstituteData&) override;
+    virtual void updateCachedDocumentLoader(WebCore::DocumentLoader&) override;
+
     virtual void setTitle(const WebCore::StringWithDirection&, const WebCore::URL&) override;
     
     virtual String userAgent(const WebCore::URL&) override;

Modified: releases/WebKitGTK/webkit-2.8/Source/WebKit2/WebProcess/WebPage/WebPage.cpp (186409 => 186410)


--- releases/WebKitGTK/webkit-2.8/Source/WebKit2/WebProcess/WebPage/WebPage.cpp	2015-07-07 07:21:38 UTC (rev 186409)
+++ releases/WebKitGTK/webkit-2.8/Source/WebKit2/WebProcess/WebPage/WebPage.cpp	2015-07-07 07:28:18 UTC (rev 186410)
@@ -1214,8 +1214,7 @@
         return;
 
     ASSERT(!m_pendingNavigationID);
-    if (!item->isInPageCache())
-        m_pendingNavigationID = navigationID;
+    m_pendingNavigationID = navigationID;
 
     m_page->goToItem(*item, FrameLoadType::Forward);
 }
@@ -1230,8 +1229,7 @@
         return;
 
     ASSERT(!m_pendingNavigationID);
-    if (!item->isInPageCache())
-        m_pendingNavigationID = navigationID;
+    m_pendingNavigationID = navigationID;
 
     m_page->goToItem(*item, FrameLoadType::Back);
 }
@@ -1246,8 +1244,7 @@
         return;
 
     ASSERT(!m_pendingNavigationID);
-    if (!item->isInPageCache())
-        m_pendingNavigationID = navigationID;
+    m_pendingNavigationID = navigationID;
 
     m_page->goToItem(*item, FrameLoadType::IndexedBackForward);
 }
@@ -4825,6 +4822,14 @@
     return documentLoader.release();
 }
 
+void WebPage::updateCachedDocumentLoader(WebDocumentLoader& documentLoader, Frame& frame)
+{
+    if (m_pendingNavigationID && frame.isMainFrame()) {
+        documentLoader.setNavigationID(m_pendingNavigationID);
+        m_pendingNavigationID = 0;
+    }
+}
+
 void WebPage::getBytecodeProfile(uint64_t callbackID)
 {
     ASSERT(JSDOMWindow::commonVM().m_perBytecodeProfiler);

Modified: releases/WebKitGTK/webkit-2.8/Source/WebKit2/WebProcess/WebPage/WebPage.h (186409 => 186410)


--- releases/WebKitGTK/webkit-2.8/Source/WebKit2/WebProcess/WebPage/WebPage.h	2015-07-07 07:21:38 UTC (rev 186409)
+++ releases/WebKitGTK/webkit-2.8/Source/WebKit2/WebProcess/WebPage/WebPage.h	2015-07-07 07:28:18 UTC (rev 186410)
@@ -152,6 +152,7 @@
 class WebColorChooser;
 class WebContextMenu;
 class WebContextMenuItemData;
+class WebDocumentLoader;
 class WebEvent;
 class WebFrame;
 class WebFullScreenManager;
@@ -844,6 +845,7 @@
     void setScrollPinningBehavior(uint32_t /* WebCore::ScrollPinningBehavior */ pinning);
 
     PassRefPtr<WebCore::DocumentLoader> createDocumentLoader(WebCore::Frame&, const WebCore::ResourceRequest&, const WebCore::SubstituteData&);
+    void updateCachedDocumentLoader(WebDocumentLoader&, WebCore::Frame&);
 
     void getBytecodeProfile(uint64_t callbackID);
     
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to