- Revision
- 233897
- Author
- [email protected]
- Date
- 2018-07-17 14:50:09 -0700 (Tue, 17 Jul 2018)
Log Message
Turn on PSON in WebKitTestRunner
https://bugs.webkit.org/show_bug.cgi?id=186542
Reviewed by Brady Eidson.
Source/WebKit:
Fix leaking of pre-warmed WebContent processes which became obvious when turning
on process-swap-on-navigation by default in WebKitTestRunner. The issue was that
the WebProcessPool holds a strong reference to its WebProcessProxy objects via
m_processes data members. In turn, WebProcessProxy objects hold a strong reference
to their WebProcessPool via their m_processPool data member. This reference cycle
normally gets broken by calling WebProcessProxy::shutDown() which removes the
WebProcessProxy objects from its WebProcessPool's m_processes container.
WebProcessProxy::shutDown() normally gets called when a WebProcessProxy no longer
has any WebPageProxy objects. However, in the case of unused pre-warmed
WebProcessProxy objects, those never get any page and therefore we'd never break
the reference cycle. To address the issue, pre-warmed WebProcessProxy objects
now only hold a Weak reference to their process pool, only regular WebProcessProxy
objects hold a strong reference to their pool.
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::~WebProcessPool):
(WebKit::WebProcessPool::tryTakePrewarmedProcess):
* UIProcess/WebProcessPool.h:
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::WebProcessProxy):
(WebKit::WebProcessProxy::getLaunchOptions):
(WebKit::WebProcessProxy::markIsNoLongerInPrewarmedPool):
* UIProcess/WebProcessProxy.h:
(WebKit::WebProcessProxy::processPool):
(WebKit::WebProcessProxy::WeakOrStrongPtr::WeakOrStrongPtr):
(WebKit::WebProcessProxy::WeakOrStrongPtr::setIsWeak):
(WebKit::WebProcessProxy::WeakOrStrongPtr::get const):
(WebKit::WebProcessProxy::WeakOrStrongPtr::operator-> const):
(WebKit::WebProcessProxy::WeakOrStrongPtr::operator* const):
(WebKit::WebProcessProxy::WeakOrStrongPtr::operator bool const):
(WebKit::WebProcessProxy::WeakOrStrongPtr::updateStrongReference):
Tools:
Turn on PSON by default in WebKitTestRunner.
* WebKitTestRunner/TestOptions.h:
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (233896 => 233897)
--- trunk/Source/WebKit/ChangeLog 2018-07-17 21:00:45 UTC (rev 233896)
+++ trunk/Source/WebKit/ChangeLog 2018-07-17 21:50:09 UTC (rev 233897)
@@ -1,3 +1,42 @@
+2018-07-17 Chris Dumez <[email protected]>
+
+ Turn on PSON in WebKitTestRunner
+ https://bugs.webkit.org/show_bug.cgi?id=186542
+
+ Reviewed by Brady Eidson.
+
+ Fix leaking of pre-warmed WebContent processes which became obvious when turning
+ on process-swap-on-navigation by default in WebKitTestRunner. The issue was that
+ the WebProcessPool holds a strong reference to its WebProcessProxy objects via
+ m_processes data members. In turn, WebProcessProxy objects hold a strong reference
+ to their WebProcessPool via their m_processPool data member. This reference cycle
+ normally gets broken by calling WebProcessProxy::shutDown() which removes the
+ WebProcessProxy objects from its WebProcessPool's m_processes container.
+ WebProcessProxy::shutDown() normally gets called when a WebProcessProxy no longer
+ has any WebPageProxy objects. However, in the case of unused pre-warmed
+ WebProcessProxy objects, those never get any page and therefore we'd never break
+ the reference cycle. To address the issue, pre-warmed WebProcessProxy objects
+ now only hold a Weak reference to their process pool, only regular WebProcessProxy
+ objects hold a strong reference to their pool.
+
+ * UIProcess/WebProcessPool.cpp:
+ (WebKit::WebProcessPool::~WebProcessPool):
+ (WebKit::WebProcessPool::tryTakePrewarmedProcess):
+ * UIProcess/WebProcessPool.h:
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::WebProcessProxy):
+ (WebKit::WebProcessProxy::getLaunchOptions):
+ (WebKit::WebProcessProxy::markIsNoLongerInPrewarmedPool):
+ * UIProcess/WebProcessProxy.h:
+ (WebKit::WebProcessProxy::processPool):
+ (WebKit::WebProcessProxy::WeakOrStrongPtr::WeakOrStrongPtr):
+ (WebKit::WebProcessProxy::WeakOrStrongPtr::setIsWeak):
+ (WebKit::WebProcessProxy::WeakOrStrongPtr::get const):
+ (WebKit::WebProcessProxy::WeakOrStrongPtr::operator-> const):
+ (WebKit::WebProcessProxy::WeakOrStrongPtr::operator* const):
+ (WebKit::WebProcessProxy::WeakOrStrongPtr::operator bool const):
+ (WebKit::WebProcessProxy::WeakOrStrongPtr::updateStrongReference):
+
2018-07-17 Wenson Hsieh <[email protected]>
Add an SPI hook to allow clients to yield document parsing and script execution
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (233896 => 233897)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2018-07-17 21:00:45 UTC (rev 233896)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2018-07-17 21:50:09 UTC (rev 233897)
@@ -333,6 +333,18 @@
if (!m_processesUsingGamepads.isEmpty())
UIGamepadProvider::singleton().processPoolStoppedUsingGamepads(*this);
#endif
+
+ // Only remaining processes should be pre-warmed ones as other keep the process pool alive.
+ while (!m_processes.isEmpty()) {
+ auto& process = m_processes.first();
+
+ ASSERT(process->isInPrewarmedPool());
+ // We need to be the only one holding a reference to the pre-warmed process so that it gets destroyed.
+ // WebProcessProxies currently always expect to have a WebProcessPool.
+ ASSERT(process->hasOneRef());
+
+ process->shutDown();
+ }
}
void WebProcessPool::initializeClient(const WKContextClientBase* client)
@@ -784,7 +796,7 @@
for (const auto& process : m_processes) {
if (process->isInPrewarmedPool()) {
--m_prewarmedProcessCount;
- process->setIsInPrewarmedPool(false);
+ process->markIsNoLongerInPrewarmedPool();
if (&process->websiteDataStore() != &websiteDataStore)
process->send(Messages::WebProcess::AddWebsiteDataStore(websiteDataStore.parameters()), 0);
return process.get();
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.h (233896 => 233897)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.h 2018-07-17 21:00:45 UTC (rev 233896)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.h 2018-07-17 21:50:09 UTC (rev 233897)
@@ -110,7 +110,7 @@
int webProcessThroughputQOS();
#endif
-class WebProcessPool final : public API::ObjectImpl<API::Object::Type::ProcessPool>, private IPC::MessageReceiver {
+class WebProcessPool final : public API::ObjectImpl<API::Object::Type::ProcessPool>, public CanMakeWeakPtr<WebProcessPool>, private IPC::MessageReceiver {
public:
static Ref<WebProcessPool> create(API::ProcessPoolConfiguration&);
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp (233896 => 233897)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2018-07-17 21:00:45 UTC (rev 233896)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2018-07-17 21:50:09 UTC (rev 233897)
@@ -116,7 +116,7 @@
: ChildProcessProxy(processPool.alwaysRunsAtBackgroundPriority())
, m_responsivenessTimer(*this)
, m_backgroundResponsivenessTimer(*this)
- , m_processPool(processPool)
+ , m_processPool(processPool, isInPrewarmedPool == IsInPrewarmedPool::Yes ? IsWeak::Yes : IsWeak::No)
, m_mayHaveUniversalFileReadSandboxExtension(false)
, m_numberOfTimesSuddenTerminationWasDisabled(0)
, m_throttler(*this, processPool.shouldTakeUIBackgroundAssertion())
@@ -162,7 +162,7 @@
ChildProcessProxy::getLaunchOptions(launchOptions);
- if (WebKit::isInspectorProcessPool(m_processPool))
+ if (WebKit::isInspectorProcessPool(processPool()))
launchOptions.extraInitializationData.add("inspector-process"_s, "1"_s);
auto overrideLanguages = m_processPool->configuration().overrideLanguages();
@@ -432,6 +432,15 @@
maybeShutDown();
}
+void WebProcessProxy::markIsNoLongerInPrewarmedPool()
+{
+ ASSERT(m_isInPrewarmedPool);
+
+ m_isInPrewarmedPool = false;
+ RELEASE_ASSERT(m_processPool);
+ m_processPool.setIsWeak(IsWeak::No);
+}
+
void WebProcessProxy::removeWebPage(WebPageProxy& webPage, uint64_t pageID)
{
auto* removedPage = m_pageMap.take(pageID);
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.h (233896 => 233897)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2018-07-17 21:00:45 UTC (rev 233896)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2018-07-17 21:50:09 UTC (rev 233897)
@@ -107,7 +107,7 @@
WebConnection* webConnection() const { return m_webConnection.get(); }
- WebProcessPool& processPool() { return m_processPool; }
+ WebProcessPool& processPool() { ASSERT(m_processPool); return *m_processPool.get(); }
// FIXME: WebsiteDataStores should be made per-WebPageProxy throughout WebKit2
WebsiteDataStore& websiteDataStore() const { return m_websiteDataStore.get(); }
@@ -219,7 +219,7 @@
#endif
bool isInPrewarmedPool() const { return m_isInPrewarmedPool; }
- void setIsInPrewarmedPool(bool isInPrewarmedPool) { m_isInPrewarmedPool = isInPrewarmedPool; }
+ void markIsNoLongerInPrewarmedPool();
#if PLATFORM(COCOA)
Vector<String> mediaMIMETypes();
@@ -226,6 +226,10 @@
void cacheMediaMIMETypes(const Vector<String>&);
#endif
+ // Called when the web process has crashed or we know that it will terminate soon.
+ // Will potentially cause the WebProcessProxy object to be freed.
+ void shutDown();
+
protected:
static uint64_t generatePageID();
WebProcessProxy(WebProcessPool&, WebsiteDataStore&, IsInPrewarmedPool);
@@ -243,9 +247,6 @@
#endif
private:
- // Called when the web process has crashed or we know that it will terminate soon.
- // Will potentially cause the WebProcessProxy object to be freed.
- void shutDown();
void maybeShutDown();
// IPC message handlers.
@@ -313,11 +314,43 @@
void logDiagnosticMessageForResourceLimitTermination(const String& limitKey);
+ enum class IsWeak { No, Yes };
+ template<typename T> class WeakOrStrongPtr {
+ public:
+ WeakOrStrongPtr(T& object, IsWeak isWeak)
+ : m_isWeak(isWeak)
+ , m_weakObject(makeWeakPtr(object))
+ {
+ updateStrongReference();
+ }
+
+ void setIsWeak(IsWeak isWeak)
+ {
+ m_isWeak = isWeak;
+ updateStrongReference();
+ }
+
+ T* get() const { return m_weakObject.get(); }
+ T* operator->() const { return m_weakObject.get(); }
+ T& operator*() const { return *m_weakObject; }
+ explicit operator bool() const { return !!m_weakObject; }
+
+ private:
+ void updateStrongReference()
+ {
+ m_strongObject = m_isWeak == IsWeak::Yes ? nullptr : m_weakObject.get();
+ }
+
+ IsWeak m_isWeak;
+ WeakPtr<T> m_weakObject;
+ RefPtr<T> m_strongObject;
+ };
+
ResponsivenessTimer m_responsivenessTimer;
BackgroundProcessResponsivenessTimer m_backgroundResponsivenessTimer;
RefPtr<WebConnectionToWebProcess> m_webConnection;
- Ref<WebProcessPool> m_processPool;
+ WeakOrStrongPtr<WebProcessPool> m_processPool; // Pre-warmed processes do not hold a strong reference to their pool.
bool m_mayHaveUniversalFileReadSandboxExtension; // True if a read extension for "/" was ever granted - we don't track whether WebProcess still has it.
HashSet<String> m_localPathsWithAssumedReadAccess;
Modified: trunk/Tools/ChangeLog (233896 => 233897)
--- trunk/Tools/ChangeLog 2018-07-17 21:00:45 UTC (rev 233896)
+++ trunk/Tools/ChangeLog 2018-07-17 21:50:09 UTC (rev 233897)
@@ -1,3 +1,14 @@
+2018-07-17 Chris Dumez <[email protected]>
+
+ Turn on PSON in WebKitTestRunner
+ https://bugs.webkit.org/show_bug.cgi?id=186542
+
+ Reviewed by Brady Eidson.
+
+ Turn on PSON by default in WebKitTestRunner.
+
+ * WebKitTestRunner/TestOptions.h:
+
2018-07-17 Wenson Hsieh <[email protected]>
Add an SPI hook to allow clients to yield document parsing and script execution
Modified: trunk/Tools/WebKitTestRunner/TestOptions.h (233896 => 233897)
--- trunk/Tools/WebKitTestRunner/TestOptions.h 2018-07-17 21:00:45 UTC (rev 233896)
+++ trunk/Tools/WebKitTestRunner/TestOptions.h 2018-07-17 21:50:09 UTC (rev 233897)
@@ -56,7 +56,7 @@
bool dumpJSConsoleLogInStdErr { false };
bool allowCrossOriginSubresourcesToAskForCredentials { false };
bool enableWebAnimationsCSSIntegration { false };
- bool enableProcessSwapOnNavigation { false };
+ bool enableProcessSwapOnNavigation { true };
bool enableProcessSwapOnWindowOpen { false };
bool enableColorFilter { false };
bool punchOutWhiteBackgroundsInDarkMode { false };
