Changeset 76163 in webkit

Timestamp:

Jan 19, 2011 2:26:45 PM (13 years ago)

Author:

jberlin@webkit.org

Message:

Crash in WebDatabaseManagerProxy::getDatabaseOrigins when called after the WebProcess has
died at least once
​https://bugs.webkit.org/show_bug.cgi?id=52730

Reviewed by Darin Adler.

WebDatabaseManagerProxy::invalidate was setting m_webContext to 0, and invalidate gets
called in WebContext::processDidClose. However, m_webContext is only set in the
constructor, which is only called from the constructor of WebContext, so attempting to send
a message to any new WebProcess after the first one died was causing a null deref.

This patch moves setting m_webcontext into clearContext and clearContext is only called in
the WebContext destructor.

This patch also adds checks for a valid WebProcessProxy before attempting to send messages to
the WebProcessProxy so that if the WebProcess has died and has not been revived, it does not
attempt to dereference a null WebProcessProxy.

• UIProcess/WebContext.cpp:

(WebKit::WebContext::~WebContext):
Call WebDatabaseManagerProxy::clearContext.

• UIProcess/WebContext.h:

(WebKit::WebContext::hasValidProcess):
Make this method public so that it can be called from WebDatabaseManagerProxy.

• UIProcess/WebDatabaseManagerProxy.cpp:

(WebKit::WebDatabaseManagerProxy::getDatabasesByOrigin):
If there isn't a valid process, invalidate the callback and return early.
(WebKit::WebDatabaseManagerProxy::getDatabaseOrigins):
Ditto.
(WebKit::WebDatabaseManagerProxy::deleteDatabaseWithNameForOrigin):
If tehre isn't a valid process return early.
(WebKit::WebDatabaseManagerProxy::deleteDatabasesForOrigin):
Ditto.
(WebKit::WebDatabaseManagerProxy::deleteAllDatabases):
Ditto.
(WebKit::WebDatabaseManagerProxy::setQuotaForOrigin):
Ditto.
(WebKit::WebDatabaseManagerProxy::invalidate):
Move setting m_webContext to 0 from here ...

• UIProcess/WebDatabaseManagerProxy.h:

(WebKit::WebDatabaseManagerProxy::clearContext):
... to here.

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/WebContext.cpp (modified) (1 diff)
• UIProcess/WebContext.h (modified) (2 diffs)
• UIProcess/WebDatabaseManagerProxy.cpp (modified) (7 diffs)
• UIProcess/WebDatabaseManagerProxy.h (modified) (1 diff)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2011-01-19  Jessie Berlin  <jberlin@apple.com>
+
+        Reviewed by Darin Adler.
+
+        Crash in WebDatabaseManagerProxy::getDatabaseOrigins when called after the WebProcess has
+        died at least once
+        https://bugs.webkit.org/show_bug.cgi?id=52730
+
+        WebDatabaseManagerProxy::invalidate was setting m_webContext to 0, and invalidate gets
+        called in WebContext::processDidClose. However, m_webContext is only set in the
+        constructor, which is only called from the constructor of WebContext, so attempting to send
+        a message to any new WebProcess after the first one died was causing a null deref.
+
+        This patch moves setting m_webcontext into clearContext and clearContext is only called in
+        the WebContext destructor.
+
+        This patch also adds checks for a valid WebProcessProxy before attempting to send messages to
+        the WebProcessProxy so that if the WebProcess has died and has not been revived, it does not
+        attempt to dereference a null WebProcessProxy.
+
+        * UIProcess/WebContext.cpp:
+        (WebKit::WebContext::~WebContext):
+        Call WebDatabaseManagerProxy::clearContext.
+        * UIProcess/WebContext.h:
+        (WebKit::WebContext::hasValidProcess):
+        Make this method public so that it can be called from WebDatabaseManagerProxy.
+
+        * UIProcess/WebDatabaseManagerProxy.cpp:
+        (WebKit::WebDatabaseManagerProxy::getDatabasesByOrigin):
+        If there isn't a valid process, invalidate the callback and return early.
+        (WebKit::WebDatabaseManagerProxy::getDatabaseOrigins):
+        Ditto.
+        (WebKit::WebDatabaseManagerProxy::deleteDatabaseWithNameForOrigin):
+        If tehre isn't a valid process return early.
+        (WebKit::WebDatabaseManagerProxy::deleteDatabasesForOrigin):
+        Ditto.
+        (WebKit::WebDatabaseManagerProxy::deleteAllDatabases):
+        Ditto.
+        (WebKit::WebDatabaseManagerProxy::setQuotaForOrigin):
+        Ditto.
+        (WebKit::WebDatabaseManagerProxy::invalidate):
+        Move setting m_webContext to 0 from here ...
+        * UIProcess/WebDatabaseManagerProxy.h:
+        (WebKit::WebDatabaseManagerProxy::clearContext):
+        ... to here.
+
 2011-01-19  Anders Carlsson  <andersca@apple.com>
 

trunk/Source/WebKit2/UIProcess/WebContext.cpp

@@ -115 +115 @@
     m_geolocationManagerProxy->clearContext();
 
+    m_databaseManagerProxy->invalidate();
+    m_databaseManagerProxy->clearContext();
+
 #ifndef NDEBUG
     webContextCounter.decrement();

trunk/Source/WebKit2/UIProcess/WebContext.h

@@ -68 +68 @@
     ProcessModel processModel() const { return m_processModel; }
     WebProcessProxy* process() const { return m_process.get(); }
+    bool hasValidProcess() const { return m_process && m_process->isValid(); }
 
     void processDidFinishLaunching(WebProcessProxy*);
@@ -143 +144 @@
 
     void ensureWebProcess();
-    bool hasValidProcess() const { return m_process && m_process->isValid(); }
     void platformInitializeWebProcess(WebProcessCreationParameters&);
 

trunk/Source/WebKit2/UIProcess/WebDatabaseManagerProxy.cpp

@@ -101 +101 @@
 {
     invalidateCallbackMap(m_arrayCallbacks);
-
-    m_webContext = 0;
 }
 
@@ -113 +111 @@
 {
     RefPtr<ArrayCallback> callback = prpCallback;
+    if (!m_webContext->hasValidProcess()) {
+        callback->invalidate();
+        return;
+    }
     uint64_t callbackID = callback->callbackID();
     m_arrayCallbacks.set(callbackID, callback.release());
@@ -165 +167 @@
 {
     RefPtr<ArrayCallback> callback = prpCallback;
+    if (!m_webContext->hasValidProcess()) {
+        callback->invalidate();
+        return;
+    }
     uint64_t callbackID = callback->callbackID();
     m_arrayCallbacks.set(callbackID, callback.release());
@@ -189 +195 @@
 void WebDatabaseManagerProxy::deleteDatabaseWithNameForOrigin(const String& databaseIdentifier, WebSecurityOrigin* origin)
 {
+    if (!m_webContext->hasValidProcess())
+        return;
     m_webContext->process()->send(Messages::WebDatabaseManager::DeleteDatabaseWithNameForOrigin(databaseIdentifier, origin->databaseIdentifier()), 0);
 }
@@ -194 +202 @@
 void WebDatabaseManagerProxy::deleteDatabasesForOrigin(WebSecurityOrigin* origin)
 {
+    if (!m_webContext->hasValidProcess())
+        return;
     m_webContext->process()->send(Messages::WebDatabaseManager::DeleteDatabasesForOrigin(origin->databaseIdentifier()), 0);
 }
@@ -199 +209 @@
 void WebDatabaseManagerProxy::deleteAllDatabases()
 {
+    if (!m_webContext->hasValidProcess())
+        return;
     m_webContext->process()->send(Messages::WebDatabaseManager::DeleteAllDatabases(), 0);
 }
@@ -204 +216 @@
 void WebDatabaseManagerProxy::setQuotaForOrigin(WebSecurityOrigin* origin, uint64_t quota)
 {
+    if (!m_webContext->hasValidProcess())
+        return;
     m_webContext->process()->send(Messages::WebDatabaseManager::SetQuotaForOrigin(origin->databaseIdentifier(), quota), 0);
 }

trunk/Source/WebKit2/UIProcess/WebDatabaseManagerProxy.h

@@ -56 +56 @@
 
     void invalidate();
+    void clearContext() { m_webContext = 0; }
 
     void initializeClient(const WKDatabaseManagerClient*);