Diff
Modified: trunk/Source/WebKit/ChangeLog (235939 => 235940)
--- trunk/Source/WebKit/ChangeLog 2018-09-12 16:34:09 UTC (rev 235939)
+++ trunk/Source/WebKit/ChangeLog 2018-09-12 16:36:55 UTC (rev 235940)
@@ -1,3 +1,35 @@
+2018-09-12 Chris Dumez <[email protected]>
+
+ Clean up SuspendedPageProxy
+ https://bugs.webkit.org/show_bug.cgi?id=189517
+
+ Reviewed by Alex Christensen.
+
+ Clean up SuspendedPageProxy:
+ 1. SuspendedPageProxy does not need to be RefCounted. It is even dangerous given that WebPageProxy
+ owns the SuspendedPageProxy and SuspendedPageProxy has a WebPageProxy& data member. We definitely
+ do not want it to outlive its WebPageProxy.
+ 2. The SuspendedPageProxy destructor does not need to be virtual.
+ 3. Have WebBackForwardListItem keep a WeakPtr<SuspendedPageProxy> instead of a SuspendedPageProxy*.
+ This is safer and avoid having to explicitly clear the pointer.
+ 4. m_finishedSuspending data member does not need a getter and is only needed if !LOG_DISABLED.
+
+ * Shared/WebBackForwardListItem.cpp:
+ (WebKit::WebBackForwardListItem::setSuspendedPage):
+ * Shared/WebBackForwardListItem.h:
+ (WebKit::WebBackForwardListItem::suspendedPage const):
+ * UIProcess/SuspendedPageProxy.cpp:
+ (WebKit::SuspendedPageProxy::SuspendedPageProxy):
+ (WebKit::SuspendedPageProxy::~SuspendedPageProxy):
+ (WebKit::SuspendedPageProxy::webProcessDidClose):
+ (WebKit::SuspendedPageProxy::didFinishLoad):
+ * UIProcess/SuspendedPageProxy.h:
+ (WebKit::SuspendedPageProxy::process const):
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::maybeCreateSuspendedPage):
+ (WebKit::WebPageProxy::reattachToWebProcess):
+ * UIProcess/WebPageProxy.h:
+
2018-09-12 Guillaume Emont <[email protected]>
Add IGNORE_WARNING_.* macros
Modified: trunk/Source/WebKit/Shared/WebBackForwardListItem.cpp (235939 => 235940)
--- trunk/Source/WebKit/Shared/WebBackForwardListItem.cpp 2018-09-12 16:34:09 UTC (rev 235939)
+++ trunk/Source/WebKit/Shared/WebBackForwardListItem.cpp 2018-09-12 16:36:55 UTC (rev 235940)
@@ -26,6 +26,7 @@
#include "config.h"
#include "WebBackForwardListItem.h"
+#include "SuspendedPageProxy.h"
#include <WebCore/URL.h>
#include <wtf/DebugUtilities.h>
@@ -112,10 +113,10 @@
return documentTreesAreEqual(mainFrameState, otherMainFrameState);
}
-void WebBackForwardListItem::setSuspendedPage(SuspendedPageProxy* page)
+void WebBackForwardListItem::setSuspendedPage(SuspendedPageProxy& page)
{
- ASSERT(!m_suspendedPage || page == nullptr);
- m_suspendedPage = page;
+ ASSERT(!m_suspendedPage);
+ m_suspendedPage = makeWeakPtr(page);
}
#if !LOG_DISABLED
Modified: trunk/Source/WebKit/Shared/WebBackForwardListItem.h (235939 => 235940)
--- trunk/Source/WebKit/Shared/WebBackForwardListItem.h 2018-09-12 16:34:09 UTC (rev 235939)
+++ trunk/Source/WebKit/Shared/WebBackForwardListItem.h 2018-09-12 16:36:55 UTC (rev 235940)
@@ -28,6 +28,7 @@
#include "APIObject.h"
#include "SessionState.h"
#include <wtf/Ref.h>
+#include <wtf/WeakPtr.h>
#include <wtf/text/WTFString.h>
namespace API {
@@ -68,8 +69,8 @@
ViewSnapshot* snapshot() const { return m_itemState.snapshot.get(); }
void setSnapshot(RefPtr<ViewSnapshot>&& snapshot) { m_itemState.snapshot = WTFMove(snapshot); }
#endif
- void setSuspendedPage(SuspendedPageProxy*);
- SuspendedPageProxy* suspendedPage() const { return m_suspendedPage; }
+ void setSuspendedPage(SuspendedPageProxy&);
+ SuspendedPageProxy* suspendedPage() const { return m_suspendedPage.get(); }
#if !LOG_DISABLED
const char* loggingString();
@@ -80,7 +81,7 @@
BackForwardListItemState m_itemState;
uint64_t m_pageID;
- SuspendedPageProxy* m_suspendedPage { nullptr };
+ WeakPtr<SuspendedPageProxy> m_suspendedPage;
};
typedef Vector<Ref<WebBackForwardListItem>> BackForwardListItemVector;
Modified: trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp (235939 => 235940)
--- trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp 2018-09-12 16:34:09 UTC (rev 235939)
+++ trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp 2018-09-12 16:36:55 UTC (rev 235940)
@@ -76,10 +76,9 @@
SuspendedPageProxy::SuspendedPageProxy(WebPageProxy& page, WebProcessProxy& process, WebBackForwardListItem& item)
: m_page(page)
, m_process(&process)
- , m_backForwardListItem(item)
, m_origin(SecurityOriginData::fromURL({ ParsedURLString, item.url() }))
{
- m_backForwardListItem->setSuspendedPage(this);
+ item.setSuspendedPage(*this);
m_process->processPool().registerSuspendedPageProxy(*this);
m_process->send(Messages::WebPage::SetIsSuspended(true), m_page.pageID());
}
@@ -91,8 +90,6 @@
process->suspendedPageWasDestroyed(*this);
process->processPool().unregisterSuspendedPageProxy(*this);
}
-
- m_backForwardListItem->setSuspendedPage(nullptr);
}
void SuspendedPageProxy::webProcessDidClose(WebProcessProxy& process)
@@ -99,12 +96,11 @@
{
ASSERT_UNUSED(process, &process == m_process);
- auto protectedThis = makeRef(*this);
m_process->processPool().unregisterSuspendedPageProxy(*this);
m_process = nullptr;
+ // This will destroy |this|.
m_page.suspendedPageClosed(*this);
- m_backForwardListItem->setSuspendedPage(nullptr);
}
void SuspendedPageProxy::destroyWebPageInWebProcess()
@@ -118,7 +114,9 @@
ASSERT(m_process);
LOG(ProcessSwapping, "SuspendedPageProxy %s from process %i finished transition to suspended", loggingString(), m_process->processIdentifier());
+#if !LOG_DISABLED
m_finishedSuspending = true;
+#endif
m_process->send(Messages::WebProcess::UpdateActivePages(), 0);
}
Modified: trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h (235939 => 235940)
--- trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h 2018-09-12 16:34:09 UTC (rev 235939)
+++ trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h 2018-09-12 16:36:55 UTC (rev 235940)
@@ -29,6 +29,7 @@
#include "WebBackForwardListItem.h"
#include <WebCore/SecurityOriginData.h>
#include <wtf/RefCounted.h>
+#include <wtf/WeakPtr.h>
namespace WebKit {
@@ -35,24 +36,17 @@
class WebPageProxy;
class WebProcessProxy;
-class SuspendedPageProxy : public RefCounted<SuspendedPageProxy> {
+class SuspendedPageProxy : public CanMakeWeakPtr<SuspendedPageProxy> {
public:
- static Ref<SuspendedPageProxy> create(WebPageProxy& page, WebProcessProxy& process, WebBackForwardListItem& item)
- {
- return adoptRef(*new SuspendedPageProxy(page, process, item));
- }
+ SuspendedPageProxy(WebPageProxy&, WebProcessProxy&, WebBackForwardListItem&);
+ ~SuspendedPageProxy();
- virtual ~SuspendedPageProxy();
-
void didReceiveMessage(IPC::Connection&, IPC::Decoder&);
WebPageProxy& page() const { return m_page; }
WebProcessProxy* process() const { return m_process; }
- WebBackForwardListItem& item() const { return m_backForwardListItem; }
const WebCore::SecurityOriginData& origin() const { return m_origin; }
- bool finishedSuspending() const { return m_finishedSuspending; }
-
void webProcessDidClose(WebProcessProxy&);
void destroyWebPageInWebProcess();
@@ -61,16 +55,15 @@
#endif
private:
- SuspendedPageProxy(WebPageProxy&, WebProcessProxy&, WebBackForwardListItem&);
-
void didFinishLoad();
WebPageProxy& m_page;
WebProcessProxy* m_process;
- Ref<WebBackForwardListItem> m_backForwardListItem;
WebCore::SecurityOriginData m_origin;
+#if !LOG_DISABLED
bool m_finishedSuspending { false };
+#endif
};
} // namespace WebKit
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (235939 => 235940)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2018-09-12 16:34:09 UTC (rev 235939)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2018-09-12 16:36:55 UTC (rev 235940)
@@ -718,7 +718,7 @@
return nullptr;
}
- m_suspendedPage = SuspendedPageProxy::create(*this, process, *currentItem);
+ m_suspendedPage = std::make_unique<SuspendedPageProxy>(*this, process, *currentItem);
LOG(ProcessSwapping, "WebPageProxy %" PRIu64 " created suspended page %s for process pid %i, back/forward item %s" PRIu64, pageID(), m_suspendedPage->loggingString(), process.processIdentifier(), currentItem->itemID().logString());
@@ -740,12 +740,12 @@
// If the process we're attaching to is kept alive solely by our current suspended page,
// we need to maintain that by temporarily keeping the suspended page alive.
- RefPtr<SuspendedPageProxy> currentSuspendedPage;
+ std::unique_ptr<SuspendedPageProxy> currentSuspendedPage;
if (!navigation) {
m_process->removeWebPage(*this, m_pageID);
m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID);
} else {
- currentSuspendedPage = m_suspendedPage;
+ currentSuspendedPage = WTFMove(m_suspendedPage);
m_process->suspendWebPageProxy(*this, *navigation);
}
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.h (235939 => 235940)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.h 2018-09-12 16:34:09 UTC (rev 235939)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.h 2018-09-12 16:36:55 UTC (rev 235940)
@@ -2234,7 +2234,7 @@
// FIXME: Support more than one suspended page per WebPageProxy,
// and have a global collection of them per process pool
// (e.g. for that process pool's page cache)
- RefPtr<SuspendedPageProxy> m_suspendedPage;
+ std::unique_ptr<SuspendedPageProxy> m_suspendedPage;
RunLoop::Timer<WebPageProxy> m_resetRecentCrashCountTimer;
unsigned m_recentCrashCount { 0 };
